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

CLI: look for maven/gradle wrapper in parent dirs #20503

Merged

Conversation

TomasHofman
Copy link
Contributor

Resolves #11516

@quarkus-bot quarkus-bot bot added area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins labels Oct 1, 2021
@geoand geoand requested a review from ebullient October 1, 2021 13:54
@TomasHofman
Copy link
Contributor Author

To elaborate more on how this changes the usability, suppose you have a following multi-module project:

project-root/
 - ./mvnw
 - some-subproject/
 - quarkus-app-subproject/

and that you navigate to project-root/quarkus-app-subproject/ and run quarkus ext ls (or other quarkus command that needs to execute maven).

The current behaviour is that the maven wrapper in the project root is not detected, so either a system installation of maven would be used by the quarkus command, or the command would fail if there was no system installation of maven present.

The new behavior would be that mvnw in the project root would be detected and used by the quarkus command. (And the same for gradlew.)

@ebullient
Copy link
Member

I reviewed this a bit on Friday.. my only concern was the discovery backstop (how do you know when you've gone too far up the parent tree)... e.g. if the parent directory isn't a (maven/gradle) project, do you stop traversing?

I do see that you are setting the target directory (in all cases) to the project root, so that is great.

@TomasHofman
Copy link
Contributor Author

@ebullient no, there is no backstop until I meet the file system root ('/' or 'C:'). I agree it's bit simplistic, but I didn't get a better idea that would work :).

Potential complications for smarter logic:

  • not every directory inside a project structure has to necessarily be a module - modules can be nested in directories that are not modules themselves (no pom.xml),
  • in gradle it may not be that easy to locate the gradle file - it's not always called build.gradle.

The proposed solution could break when there is mvnw laying around in some parent dir which in fact doesn't belong to the project, which IMO is rarer than the points I mentioned above.

@TomasHofman
Copy link
Contributor Author

BTW somebody mentioned mdub in the related issue, which does the same thing: https://github.com/dansomething/mdub/blob/master/bin/mw

But I don't want to push it at all cost, if you don't find it reasonable. People have lived with the current functionality for some time...

@ebullient
Copy link
Member

ebullient commented Oct 4, 2021

nono! Not resistant. Just paranoid. ;) I've done dumb things in my own systems looking for "stuff in the parent" .. so I'm just trying to understand when/how we determine we've backed out of the project tree entirely.

mdub stops at root.. is that right? or should you stop as soon as you reach a directory that doesn't have a pom.xml/build.gradle file, or.. ?

@TomasHofman
Copy link
Contributor Author

TomasHofman commented Oct 5, 2021

Yes, mdub stops at the file system root, which is what this PR currently does.

In my opinion stopping at root is the most reasonable option, because it offers more predictable and understandable behavior to developers. Less magic - less potential for surprises. Worst case - CLI uses mvn wrapper it shouldn't have used.

Trying to detect project roots by the presence of pom.xml / build.gradle will fail in some corner cases, that I described:

  • Meeting a directory that doesn't have pom.xml/build.gradle may not mean that it's outside of a project. It could just be non-module directory inside the project, although it's not common.

  • With Gradle, some projects use gradle files that are not called build.gradle but something else. E.g. Hibernate uses convention ${moduleName}.gradle (I know that hibernate is not a quarkus project, just pointing out that gradle allows you to do weird things).

Worst case: CLI will not work in these cases. But it will work for most projects so the corner cases may be no big deal, there will always be some.

I still feel better about the file system root. OTOH detecting pom.xml / build.gradle appears more defensive, so if you would prefer that I can rework :).

@ebullient ebullient merged commit a4e2280 into quarkusio:main Oct 5, 2021
@quarkus-bot quarkus-bot bot added this to the 2.4 - main milestone Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quarkus CLI should look in parent directories to find maven and gradle wrappers
2 participants