Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid installing multiple extensions with multiple matches #11098

Merged
merged 1 commit into from
Jul 31, 2020

Conversation

gastaldi
Copy link
Contributor

Unless it's a pattern.

Fixes #11086

@boring-cyborg boring-cyborg bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform labels Jul 30, 2020
@gastaldi gastaldi requested review from ia3andy and maxandersen July 30, 2020 14:43
@gastaldi gastaldi force-pushed the add_extension_fix branch from 2df5814 to 77dac54 Compare July 30, 2020 14:46
@gastaldi gastaldi added this to the 1.8.0 - master milestone Jul 30, 2020
@gastaldi gastaldi force-pushed the add_extension_fix branch 2 times, most recently from edd0d7a to 06a5b5f Compare July 30, 2020 15:11
@gastaldi gastaldi force-pushed the add_extension_fix branch from 06a5b5f to afa209e Compare July 30, 2020 15:13
@gsmet
Copy link
Member

gsmet commented Jul 30, 2020

I thought we had tests for that? I remember we did tests some of this.

@gsmet
Copy link
Member

gsmet commented Jul 30, 2020

@maxandersen can you check that one tomorrow morning? That's something I could include in the CR1.

@maxandersen
Copy link
Member

@ia3andy @gastaldi I assume this code only effects if you enable new registry ?

Since default users are not affected in master where I get:

 mvn quarkus:add-extension -Dextension=jdbc
[INFO] --- quarkus-maven-plugin:1.6.1.Final:add-extension (default-cli) @ test2 ---
❌ Multiple extensions matching 'jdbc'
     * io.quarkus:quarkus-jdbc-derby
     * io.quarkus:quarkus-jdbc-mssql
     * io.quarkus:quarkus-elytron-security-jdbc
     * org.apache.camel.quarkus:camel-quarkus-jdbc
     * io.quarkus:quarkus-jdbc-mysql
     * io.quarkus:quarkus-jdbc-postgresql
     * io.quarkus:quarkus-jdbc-h2
     * io.quarkus:quarkus-jdbc-mariadb
     Be more specific e.g using the exact name or the full GAV.

Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only affects new registry and looks correct thus lgtm.

@gastaldi
Copy link
Contributor Author

gastaldi commented Jul 31, 2020

Thanks, but looking at the output you pasted it seems like you're testing 1.6.1.Final.

FYI the original extension lookup algorithm was copied to the extension registry and that's what's being used in the list and add-extension commands, regardless if the Registry URL is specified or not (it falls-back to the extensions in the platform if the registry URL is not provided). This was a special case that was not caught in the existing tests but hopefully this should be good now

@gastaldi gastaldi merged commit 858851b into quarkusio:master Jul 31, 2020
@gsmet gsmet modified the milestones: 1.8.0 - master, 1.7.0.CR1 Jul 31, 2020
@gastaldi gastaldi deleted the add_extension_fix branch August 21, 2020 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In tooling, do not install any extension when there are multiple matches
3 participants