You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/07/12 18:25:40 UTC
[GitHub] [maven] kwart opened a new pull request, #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
kwart opened a new pull request, #767:
URL: https://github.com/apache/maven/pull/767
JIRA: https://issues.apache.org/jira/browse/MNG-7511
This PR ensures the `degreeOfConcurrency` is always a positive number (defaulting to `1`). It resolves issues when the computed value is less than zero.
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [x] Each commit in the pull request should have a meaningful subject line and body.
- [x] Format the pull request title like `[MNG-XXX] SUMMARY`, where you replace `MNG-XXX`
and `SUMMARY` with the appropriate JIRA issue. Best practice is to use the JIRA issue
title in the pull request title and in the first line of the commit message.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
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
[GitHub] [maven] kwart commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
kwart commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1187228526
@michael-o Thanks for merging.
WRT 3.9 backport: Should I care about the exception type and the `-TC<float>` format, or should it be rather the 1:1 backport?
--
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
[GitHub] [maven] kwart commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
kwart commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1182239385
> I am not convinced by this. It should throw an IAE here. I would expect to fix the source.
To me, it seems like the safest approach.
Another place to put the fix would be the [MavenCli.calculateDegreeOfConcurrencyWithCoreMultiplier(...)](https://github.com/apache/maven/blob/maven-3.8.6/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java#L1613-L1617) method, but I like the protection in the config object more as it handles more cases without doing harm.
Still, if you request the change, I can move the fix to the `calculateDegreeOfConcurrencyWithCoreMultiplier(...)` method.
--
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
[GitHub] [maven] kwart commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
kwart commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1183632685
The C allowed only at the end would break the backward compatibility, see the [MavenCliTest](https://github.com/apache/maven/blob/maven-3.8.6/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliTest.java#L82-L85).
So I've added a new commit with additional checks in the `MavenCli.calculateDegreeOfConcurrencyWithCoreMultiplier(String)` method.
--
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
[GitHub] [maven] kwart commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
kwart commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1186983324
:+1: LGTM
Thanks for jumping on it.
--
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
[GitHub] [maven] michael-o commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1187242346
> @michael-o Thanks for merging.
>
> WRT 3.9 backport: Should I care about the exception type and the `-TC<float>` format, or should it be rather the 1:1 backport?
Back port 1:1 because:
* `-TC<float>` was never documented
* This is a class with a `main()` not really intended for code use, but both exceptions are still IAE.
--
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
[GitHub] [maven] asfgit merged pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
asfgit merged PR #767:
URL: https://github.com/apache/maven/pull/767
--
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
[GitHub] [maven] michael-o commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1183459036
I think I have a better idea for this. First of all the arg need to checked whether it is a really an int or float. The C must be at the end. It must be larger than 0. The IAE still stands. When doing with float we could either use ceiling or floor. And then here set to one when smaller than 1.
--
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
[GitHub] [maven] michael-o commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1186597152
Picking this up...A breaking change in Maven 4 is fully acceptable.
--
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
[GitHub] [maven] michael-o commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1186618300
@kwart Added a commit on top:
* Using `C` not at the end was never documented, so it is undefined behavior. Thus, don't rely on
* Change of exception: Well, it is CLI, not for embedding into an IDE or so. For the user on the CLI it does not matter.
WDYT?
--
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
[GitHub] [maven] michael-o commented on pull request #767: [MNG-7511] Ensure the degreeOfConcurrency is a positive number in MavenExecutionRequest
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #767:
URL: https://github.com/apache/maven/pull/767#issuecomment-1187166702
@kwart If you want this in Maven 3.9 please provide a followup PR.
--
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