You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cstamas (via GitHub)" <gi...@apache.org> on 2024/05/31 19:09:00 UTC
[PR] Use Maven Wrapper to build [maven]
cstamas opened a new pull request, #1548:
URL: https://github.com/apache/maven/pull/1548
And do not rely on third party tools as they
may do things that surprise us.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2145825719
Done in #1553
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "laeubi (via GitHub)" <gi...@apache.org>.
laeubi commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143449905
> You are mixing things. Again, we do NOT use setup-maven action, hence the readme you point it is... off topic.
I mentioned [you need to use setup-maven action](https://github.com/apache/maven/pull/1548#issuecomment-2143285471), you replied that
>> We use that action, and the last IT failures made us realize how strange it behaves
so I obviously do not mix up anything here but maybe you have just misread my reply...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2145810928
I will create a new one for 3.x 😄
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143449034
> Well I would call it **strange** to use an action without specify a version ... I mean even [the readme tells you to specify one](https://github.com/stCarolas/setup-maven), i more suspect that without a version nothing is done actually....
You are mixing things. Again, we do NOT use `setup-maven` action, hence the readme you point it is... off topic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2142844431
or we can add:
```
- name: Set up Maven
run:
mvn --errors --batch-mode --show-version org.apache.maven.plugins:maven-wrapper-plugin:3.3.2:wrapper "-Dmaven=3.9.7"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "laeubi (via GitHub)" <gi...@apache.org>.
laeubi commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143285471
There is a setup-maven action:
https://github.com/marketplace/actions/setup-maven-action
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "rmannibucau (via GitHub)" <gi...@apache.org>.
rmannibucau commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143424345
setup-java uses OS maven version which is likely not what we want.
Guess we should reuse our shared action enabling to support setup-maven to have a build matrix (ideally).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "laeubi (via GitHub)" <gi...@apache.org>.
laeubi commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143352303
> and the last IT failures made us realize how strange it behaves: it set up different Maven versions per
> True, we did not ask for specific maven version, but that is not the point.
Well I would call it **strange** to use an action without specify a version ... I mean even [the readme tells you to specify one](https://github.com/stCarolas/setup-maven), i more suspect that without a version nothing is done actually....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas closed pull request #1548: Use Maven Wrapper to build
URL: https://github.com/apache/maven/pull/1548
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2145809544
#1550 is for master ... this one was for 3.x branch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "laeubi (via GitHub)" <gi...@apache.org>.
laeubi commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143422824
> I can't see the use of this action for ITs. We do use actions/setup-java, but not actions/setup-maven. I would think reusing this action in the context of GH action makes more sense.
Maybe it is confused with setup-java having support for maven toolchains and caching option named maven ...
If one wants there is a composite action https://github.com/marketplace/actions/setup-maven-action but I usually found setup-java/maven enough and more flexible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2145748141
I hope it is ready to merge @cstamas @gnodet what do you think
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "laeubi (via GitHub)" <gi...@apache.org>.
laeubi commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143448479
setup-java has nothing to do with installing / "using" maven it just do the following:
- generates `toolchain.xml` from installed jdks
- uses `pom.xml` files as cache keys
beside that I hope that the itest are using the _maven that was build_ anyways (so a matrix des not make much sense here), so whats left is that the _maven version to run the build_ is better specified explicitly to prevent any surprise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143461937
Actually, we're not using it... I thought, but the matrix is used with wrapper
https://github.com/apache/maven-gh-actions-shared/blob/v4/.github/workflows/maven-verify.yml#L196-L198
So I'm fine using wrapper here too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #1548:
URL: https://github.com/apache/maven/pull/1548#discussion_r1623245316
##########
.github/workflows/maven.yml:
##########
@@ -47,7 +47,7 @@ jobs:
cache: 'maven'
- name: Build with Maven
- run: mvn verify -e -B -V -DdistributionFileName=apache-maven
+ run: ./mvnw verify -e -B -V -DdistributionFileName=apache-maven
Review Comment:
before this step we can also add step `Set up Maven` with wrapper
and we not need add wrapper properties and scripts to code
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143346085
> There is a setup-maven action: https://github.com/marketplace/actions/setup-maven
>
> I use it all the time without any issues and you can perfectly control the maven version.
We use that action, and the last IT failures made us realize how strange it behaves: it set up different Maven versions per OS:
* ubuntu 3.8.8
* windows 3.8.7
* macos 3.9.7
True, we did _not_ ask for specific maven version, but that is not the point. We should use wrapper anyway to be able to precisely control which Maven version we want to use (across all OSes) but also for dogfooding purposes.
See
https://github.com/apache/maven/actions/runs/9322027451/job/25662372003?pr=1546
https://github.com/apache/maven/actions/runs/9322027451/job/25662372450?pr=1546
https://github.com/apache/maven/actions/runs/9322027451/job/25662372946?pr=1546
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143420229
> > There is a setup-maven action: https://github.com/marketplace/actions/setup-maven
> > I use it all the time without any issues and you can perfectly control the maven version.
>
> We use that action, and the last IT failures made us realize how strange it behaves: it set up different Maven versions per OS:
I can't see the use of this action for ITs. We do use `actions/setup-java`, but not `actions/setup-maven`. I would think reusing this action in the context of GH action makes more sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2143449297
IMHO we should not use any action to setup Maven but wrapper.... that's my 5 cents (we even have examples of "maven matrix" that is simply doable with wrapper).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use Maven Wrapper to build [maven]
Posted by "cstamas (via GitHub)" <gi...@apache.org>.
cstamas commented on PR #1548:
URL: https://github.com/apache/maven/pull/1548#issuecomment-2145782407
Superseded by https://github.com/apache/maven/pull/1550
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org