You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "slawekjaranowski (via GitHub)" <gi...@apache.org> on 2023/04/23 09:19:33 UTC
[GitHub] [maven-assembly-plugin] slawekjaranowski opened a new pull request, #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
slawekjaranowski opened a new pull request, #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134
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/MASSEMBLY) 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 `[MASSEMBLY-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MASSEMBLY-XXX` 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.
- [ ] 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.
- [x] You have run the integration tests successfully (`mvn -Prun-its clean verify`).
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)
- [x] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest configuration of jar archiver should be respected
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1182168173
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
+
+ if (configuration != null) {
+ archiver.setCompress(configuration.isCompress());
+ archiver.setForced(configuration.isForced());
+ archiver.setIndex(configuration.isIndex());
Review Comment:
As I wrote code was copied from maven-archiver, for this change I will remove 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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174596627
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
yes - exactly
--
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-assembly-plugin] hboutemy commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest configuration of jar archiver should be respected
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1183818546
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
I definitively don't understand when all this is triggered, but it seems inoffensive, let's go...
--
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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174634942
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
The same is in Maven Archiver.
--
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-assembly-plugin] slawekjaranowski merged pull request #134: [MASSEMBLY-986] Minimal default Manifest configuration of jar archiver should be respected
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski merged PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134
--
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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174596978
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
+
+ if (configuration != null) {
+ archiver.setCompress(configuration.isCompress());
+ archiver.setForced(configuration.isForced());
+ archiver.setIndex(configuration.isIndex());
Review Comment:
It comes from
https://github.com/apache/maven-archiver/blob/master/src/main/java/org/apache/maven/archiver/MavenArchiver.java#L576-L593
--
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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174634834
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
+
+ if (configuration != null) {
+ archiver.setCompress(configuration.isCompress());
+ archiver.setForced(configuration.isForced());
+ archiver.setIndex(configuration.isIndex());
Review Comment:
Ok, I did it by the way - can be in separate issue 😄
--
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-assembly-plugin] hboutemy commented on pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#issuecomment-1530913494
IIUC, what you are trying to achieve is to avoid Reproducible Builds output to change when changing `plexus-archiver` version (like proposed by dependabot in #119
--
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-assembly-plugin] michael-o commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174595582
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -293,10 +294,7 @@ protected Archiver createArchiver(
final List<FileSelector> extraSelectors = new ArrayList<>();
final List<ArchiveFinalizer> extraFinalizers = new ArrayList<>();
if (archiver instanceof JarArchiver) {
- if (mergeManifestMode != null) {
- ((JarArchiver) archiver)
- .setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
- }
+ configureJarArchiver((JarArchiver) archiver, mergeManifestMode, configSource.getJarArchiveConfiguration());
Review Comment:
I would expect that this applies to all JAR derived formats? WAR, EAR, CAR, etc?
--
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-assembly-plugin] slawekjaranowski commented on pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#issuecomment-1522954505
@hboutemy, @michael-o - Similar change was done in https://github.com/apache/maven-archiver/commit/7041fcace9210112f6745d880133769f954ef442
Please try to upgrade plexus-archiver - #119 - it is cause of this issue
--
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-assembly-plugin] slawekjaranowski commented on pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#issuecomment-1528704807
@hboutemy can you update your comment - is it more understandable for you now?
--
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-assembly-plugin] slawekjaranowski commented on pull request #134: [MASSEMBLY-986] Minimal default Manifest configuration of jar archiver should be respected
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#issuecomment-1532021383
Now PR contains only required changes to achieve goal.
--
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-assembly-plugin] michael-o commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174595764
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
This will set 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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174597747
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -293,10 +294,7 @@ protected Archiver createArchiver(
final List<FileSelector> extraSelectors = new ArrayList<>();
final List<ArchiveFinalizer> extraFinalizers = new ArrayList<>();
if (archiver instanceof JarArchiver) {
- if (mergeManifestMode != null) {
- ((JarArchiver) archiver)
- .setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
- }
+ configureJarArchiver((JarArchiver) archiver, mergeManifestMode, configSource.getJarArchiveConfiguration());
Review Comment:
yes - currently ear, rar, war
https://codehaus-plexus.github.io/plexus-archiver/apidocs/org/codehaus/plexus/archiver/jar/JarArchiver.html
I would like to extends another types, eg for car we only have a UnArchiver
https://codehaus-plexus.github.io/plexus-archiver/apidocs/org/codehaus/plexus/archiver/car/package-summary.html
--
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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest configuration of jar archiver should be respected
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1182400577
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
default value for `mergeManifestMode` is `skip`
`setMinimalDefaultManifest` has only effect on code: https://github.com/codehaus-plexus/plexus-archiver/blob/40b1b12e849ef62021e9d9787add3db7f34267af/src/main/java/org/codehaus/plexus/archiver/jar/Manifest.java#L767
If we omit `addDefaultEntries` or set it with `true` - maven-archiver will add own Manifest
So IMHO can be hardcoded
--
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-assembly-plugin] hboutemy commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest configuration of jar archiver should be respected
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1182127757
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
what if mergeManifestMode is set to `skip`?
I find it surprising to have `true` hardcoded
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
+
+ if (configuration != null) {
+ archiver.setCompress(configuration.isCompress());
+ archiver.setForced(configuration.isForced());
+ archiver.setIndex(configuration.isIndex());
Review Comment:
does this have to be checked against mergeManifestmode or not?
--
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-assembly-plugin] hboutemy commented on pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "hboutemy (via GitHub)" <gi...@apache.org>.
hboutemy commented on PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#issuecomment-1522795659
I don't understand what issue we're trying to solve
I don't understand the title of the issue
The only aspect I understand in the Jira issue description is that "it may break Reproducible Builds", but I don't see how: I never have RB broken by something like that (because Plexus Archiver component version is reproducible)
please explain what the problem is (theoretical or in real life) that is being tried to solve
--
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-assembly-plugin] michael-o commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174598030
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
+
+ if (configuration != null) {
+ archiver.setCompress(configuration.isCompress());
+ archiver.setForced(configuration.isForced());
+ archiver.setIndex(configuration.isIndex());
Review Comment:
But these aren't related to this change, no? I am still searching for the connection between the issue title and this hunk.
--
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-assembly-plugin] michael-o commented on pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#issuecomment-1519086649
I am still not fully conviced here. For me the gain is questionable or I don't understand it completely. I'd like to hear @hboutemy's opinion on this.
--
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-assembly-plugin] slawekjaranowski commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest configuration of jar archiver should be respected
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1182169784
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
> what if mergeManifestMode is set to `skip`?
I don't test such ...
>I find it surprising to have `true` hardcoded
copied from maven-archived 😄
--
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-assembly-plugin] michael-o commented on pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#issuecomment-1522976312
> @hboutemy, @michael-o - Similar change was done in [apache/maven-archiver@7041fca](https://github.com/apache/maven-archiver/commit/7041fcace9210112f6745d880133769f954ef442)
>
> Please try to upgrade plexus-archiver - #119 - it is cause of this issue
Give me a couple of days to look into. I am currently overloaded.
--
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-assembly-plugin] michael-o commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174595729
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
+
+ if (configuration != null) {
+ archiver.setCompress(configuration.isCompress());
+ archiver.setForced(configuration.isForced());
+ archiver.setIndex(configuration.isIndex());
Review Comment:
I am confused, where do these come from?
--
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-assembly-plugin] michael-o commented on a diff in pull request #134: [MASSEMBLY-986] Minimal default Manifest should be used by jar archiver
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #134:
URL: https://github.com/apache/maven-assembly-plugin/pull/134#discussion_r1174596894
##########
src/main/java/org/apache/maven/plugins/assembly/archive/DefaultAssemblyArchiver.java:
##########
@@ -351,6 +349,23 @@ protected Archiver createArchiver(
return archiver;
}
+ private void configureJarArchiver(
+ JarArchiver archiver, String mergeManifestMode, MavenArchiveConfiguration configuration) {
+
+ if (mergeManifestMode != null) {
+ archiver.setFilesetmanifest(JarArchiver.FilesetManifestConfig.valueOf(mergeManifestMode));
+ }
+
+ archiver.setMinimalDefaultManifest(true);
Review Comment:
So this leaves the user no option to include 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