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

Maven wrapper added to the parent directory of generated projects #1657

Closed
fbricon opened this issue Mar 23, 2019 · 9 comments
Closed

Maven wrapper added to the parent directory of generated projects #1657

fbricon opened this issue Mar 23, 2019 · 9 comments
Assignees
Labels
area/maven kind/bug Something isn't working
Milestone

Comments

@fbricon
Copy link
Contributor

fbricon commented Mar 23, 2019

Following up on #1498 (comment):

➜  souk mvn io.quarkus:quarkus-maven-plugin:0.12.0:create \
    -DprojectGroupId=foo.bar \
    -DprojectArtifactId=todo-quarked \
    -DclassName="foo.bar.GreetingResource" \
    -Dpath="/hello"
...
[INFO]
[INFO] Maven Wrapper version 0.5.3 has been successfully set up for your project.
[INFO] Using Apache Maven: 3.6.0
[INFO] Repo URL in properties file: https://repo.maven.apache.org/maven2
[INFO]
[INFO]
[INFO] ========================================================================================
[INFO] Your new application has been created in /Users/fbricon/Dev/souk/todo-quarked
[INFO] Navigate into this directory and launch your application with mvn compile quarkus:dev
[INFO] Your application will be accessible on http://localhost:8080
[INFO] ========================================================================================
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  6.927 s
[INFO] Finished at: 2019-03-23T13:18:01-04:00
[INFO] ------------------------------------------------------------------------
➜  souk cd /Users/fbricon/Dev/souk/todo-quarked
➜  todo-quarked ./mvnw quarkus:list-extensions
zsh: no such file or directory: ./mvnw
➜  todo-quarked la
total 16
-rw-r--r--  1 fbricon  staff    53B 23 Mar 13:17 .dockerignore
-rw-r--r--  1 fbricon  staff   3.5K 23 Mar 13:17 pom.xml
drwxr-xr-x  4 fbricon  staff   128B 23 Mar 13:17 src
drwxr-xr-x  6 fbricon  staff   192B 23 Mar 13:19 target

Turns out the maven wrapper has been added to the parent directory instead

@gsmet
Copy link
Member

gsmet commented Mar 23, 2019

@geoand so FYI, the Maven project creation behaves differently if the directory is empty or not:

  • if the directory is empty, it creates a new project in place and that's probably what we all tested;
  • if the directory is not empty, it will create a new project in a new directory and that's where the Maven wrapper should be installed.

@cescoffier I still think we should always create a new directory and have a consistent behavior if it was unclear :)

@gsmet gsmet added area/maven kind/bug Something isn't working labels Mar 23, 2019
@geoand
Copy link
Contributor

geoand commented Mar 23, 2019

I'm cool with whatever behavior we think is appropriate and will gladly submit a PR to implement it.

@cescoffier
Copy link
Member

cescoffier commented Mar 24, 2019

@cescoffier I still think we should always create a new directory and have a consistent behavior if it was unclear :)

Well, it's more complicated. We need to check if there is a pom.xml, if there, check the type, add the sub-module and so on (add the parent, which mean that the template becomes conditional, but we can't really do this anymore as the template engine has been removed). So yes, we need to do it, but it's not a 15 minutes fix.

@geoand
Copy link
Contributor

geoand commented Mar 24, 2019

Why don't we just fail the create process if there is already a pom.xml in the directory?
It seems to me that trying to accommodate cases where the project already exists would be tricky and not obvious to users. From my perspective, when I create a project I start with the idea that it is always a blank slate.

@gsmet
Copy link
Member

gsmet commented Mar 24, 2019

Yes, personally, I would prefer the following behavior:

  • create project: create a new project from scratch in a directory named after the artifact name;
  • if we want to implement creating a submodule, I would make it a separate command. It's definitely not the same thing you are after and the behavior might be totally different (maybe the property quarkus.version should be added to the parent pom for instance). One thing that comes to mind is that I'm really not sure I would like the command to entirely rewrite and reformat my existing pom file (which is what it would do right now).

@geoand for now, I would fix the mvn wrapper addition to take into account where the project is created if it's not too much work. This discussion is a bit orthogonal IMHO.

@geoand
Copy link
Contributor

geoand commented Mar 24, 2019

@gsmet makes sense, I'll look into the wrapper part and see how it can be fixed

geoand added a commit to geoand/quarkus that referenced this issue Mar 24, 2019
When a project is created via the maven plugin, ensure that the maven
wrapper always ends up in the same directory as the generated pom file

Fixes: quarkusio#1657
@cescoffier
Copy link
Member

cescoffier commented Mar 24, 2019 via email

gsmet added a commit that referenced this issue Mar 25, 2019
Ensure the Maven Wrapper is created in the proper directory
@gsmet gsmet added this to the 0.13.0 milestone Mar 25, 2019
@FroMage
Copy link
Member

FroMage commented Apr 8, 2019

Haha, just got bitten by this.

@geoand
Copy link
Contributor

geoand commented Apr 8, 2019

It should be fixed in master :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/maven kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants