-
Notifications
You must be signed in to change notification settings - Fork 77
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
[MJAR-62] Set Build-Jdk according to used toolchain #73
Conversation
src/main/java/org/apache/maven/plugins/jar/ToolchainsJdkSpecification.java
Outdated
Show resolved
Hide resolved
src/test/java/org/apache/maven/plugins/jar/ToolchainsJdkVersionTest.java
Outdated
Show resolved
Hide resolved
4cf84bb
to
691e0a1
Compare
src/main/java/org/apache/maven/plugins/jar/ToolchainsJdkSpecification.java
Outdated
Show resolved
Hide resolved
691e0a1
to
68ac9fc
Compare
68ac9fc
to
ce7ba4f
Compare
if (version.isEmpty()) { | ||
version = err.getOutput().trim(); | ||
} | ||
if (version.startsWith("javac ")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems somewhat prone to unexpected results from javac -v, either in a different JDK than normal or in a future JDK version. Can you add a a catch for IndexOutOfBoundsException and perhaps return null in that case?
return null; | ||
} | ||
} catch (CommandLineException e) { | ||
throw new UncheckedCommandLineException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're already returning null in line 93, it might make sense to return null here as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is idea .... but we should log a warning about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, no exception - only warn about something wrong as it is not critical build should not be broken
ce7ba4f
to
5269ec0
Compare
logger.warn("Unrecognized output form " + path + " -version - " + version); | ||
return null; | ||
} | ||
} catch (CommandLineException e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommandLineException | IndexOutOfBoundsException
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
5269ec0
to
3002a3a
Compare
### What changes were proposed in this pull request? ### Why are the changes needed? - `exec-maven-plugin` from `3.1.0` to `3.2.0` https://github.com/mojohaus/exec-maven-plugin/releases/tag/3.2.0 https://github.com/mojohaus/exec-maven-plugin/releases/tag/3.1.1 Bug Fixes: 1.Fix mojohaus/exec-maven-plugin#158 - Fix non ascii character handling (mojohaus/exec-maven-plugin#372) 2.[mojohaus/exec-maven-plugin#323] exec arguments missing (mojohaus/exec-maven-plugin#324) - `build-helper-maven-plugin` from `3.4.0` to `3.5.0` https://github.com/mojohaus/build-helper-maven-plugin/releases/tag/3.5.0 - `maven-compiler-plugin` from `3.12.1` to `3.13.0` https://github.com/apache/maven-compiler-plugin/releases/tag/maven-compiler-plugin-3.13.0 - `maven-jar-plugin` from `3.3.0` to `3.4.0` https://github.com/apache/maven-jar-plugin/releases/tag/maven-jar-plugin-3.4.0 [[MJAR-62]](https://issues.apache.org/jira/browse/MJAR-62) - Set Build-Jdk according to used toolchain (apache/maven-jar-plugin#73) - `maven-source-plugin` from `3.3.0` to `3.3.1` https://github.com/apache/maven-source-plugin/releases/tag/maven-source-plugin-3.3.1 - `maven-assembly-plugin` from `3.6.0` to `3.7.1` https://github.com/apache/maven-assembly-plugin/releases/tag/maven-assembly-plugin-3.7.1 https://github.com/apache/maven-assembly-plugin/releases/tag/maven-assembly-plugin-3.7.0 Bug Fixes: 1.[[MASSEMBLY-967](https://issues.apache.org/jira/browse/MASSEMBLY-967)] - maven-assembly-plugin doesn't add target/class artifacts in generated jarfat but META-INF/MANIFEST.MF seems to be correct 2.[[MASSEMBLY-994](https://issues.apache.org/jira/browse/MASSEMBLY-994)] - Items from unpacked dependency are not refreshed 3.[[MASSEMBLY-998](https://issues.apache.org/jira/browse/MASSEMBLY-998)] - Transitive dependencies are not properly excluded as of 3.1.1 4.[[MASSEMBLY-1008](https://issues.apache.org/jira/browse/MASSEMBLY-1008)] - Assembly plugin handles scopes wrongly 5.[[MASSEMBLY-1020](https://issues.apache.org/jira/browse/MASSEMBLY-1020)] - Cannot invoke "java.io.File.isFile()" because "this.inputFile" is null 6.[[MASSEMBLY-1021](https://issues.apache.org/jira/browse/MASSEMBLY-1021)] - Nullpointer in assembly:single when upgrading to 3.7.0 7.[[MASSEMBLY-1022](https://issues.apache.org/jira/browse/MASSEMBLY-1022)] - Unresolved artifacts should be not processed - `cyclonedx-maven-plugin` from `2.7.9` to `2.8.0` https://github.com/CycloneDX/cyclonedx-maven-plugin/releases/tag/cyclonedx-maven-plugin-2.8.0 https://github.com/CycloneDX/cyclonedx-maven-plugin/releases/tag/cyclonedx-maven-plugin-2.7.11 https://github.com/CycloneDX/cyclonedx-maven-plugin/releases/tag/cyclonedx-maven-plugin-2.7.10 Bug Fixes: 1.check if configured schemaVersion is supported (CycloneDX/cyclonedx-maven-plugin#479) 2.ignore bomGenerator.generate() call (CycloneDX/cyclonedx-maven-plugin#376) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46043 from panbingkun/update_maven_plugins. Authored-by: panbingkun <panbingkun@baidu.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
https://issues.apache.org/jira/browse/MJAR-62