[MNG-8685] Better support for CI systems#2254
Conversation
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/cisupport/GenericCIDetector.java
Outdated
Show resolved
Hide resolved
|
Fun fact: all 3 failing ITs tries to "escape" from CI env by setting |
| public static List<CIInfo> detectCI() { | ||
| List<CIDetector> detectors = ServiceLoader.load(CIDetector.class).stream() | ||
| .map(ServiceLoader.Provider::get) | ||
| .toList(); |
There was a problem hiding this comment.
Why not continue with this stream to get List<CIInfo> and get rid of the loop?
api/maven-api-cli/src/main/java/org/apache/maven/api/cli/cisupport/CIInfo.java
Outdated
Show resolved
Hide resolved
...cli/src/main/resources/META-INF/services/org.apache.maven.cling.invoker.cisupport.CIDetector
Outdated
Show resolved
Hide resolved
impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/cisupport/CIDetectorHelperTest.java
Show resolved
Hide resolved
| .map(CIDetector::detectCI) | ||
| .filter(Optional::isPresent) | ||
| .map(Optional::get) | ||
| .toList()); |
There was a problem hiding this comment.
https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Stream.html#toList():
The returned List is unmodifiable
so not sure about removeIf later.
Maybe Collectors.toCollection(ArrayList::new)?
There was a problem hiding this comment.
I think is fine, this happens "on boot", is not some sort of "hot" spot. The reason why I remove "generic" if there are 2 or more is that I assume there is more specific hit (like GH, sets CI=true but also sets GITHUB_RUNNER=true)... we can refine this later anyway.
There was a problem hiding this comment.
Also, to keep things simple I don't want to fiddle with order, as other solution would be to test all, and THEN as last, if no hit, "generic"... but that would assume there is no (and never will be) overlap between all CI systems, which IMO we never know and cannot know (future wise).
|
Hello @cstamas, thank you for your great work on this 🙏. I am not familiar with the release process of Maven, is this change planned for Maven 3.10 and Maven 4, or solely for Maven 4? In case of the later, is it ok if I backport your work on maven-3.x-next (I think this is the one)? |
|
This change went into the master cli module, so will be out as 4.0.0-rc-4 (next planned release). You can try backport it if you want, and I can help with that, but it will not be a simple thing (like just apply patches). The whole cli was redone in 4... |
|
In short, for mvn3 I'd envision some PR that is more close to your original PR (and adds verbose support for those that support it: GH and Travis IIRC?) |
|
Resolve #9622 |
5 participants
Improve CI detection of Maven, make possible to propagate some options from CI to Maven.
Changes:
InvokerRequestcarries new info:Optional<CIInfo>: if present, we run on CI, if empty, not.Parser(creatingInvokerRequest) already inspects env and assembles invoker request, so make it detect CI as wellCIDetectoris a Java Service (this all happens early, no DI yet) that can be extended (like adding CI specific detectors). Core had one implementation that became the "generic" that is what Maven 4 had so far.https://issues.apache.org/jira/browse/MNG-8685