[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest
rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest URL: https://github.com/apache/maven-shade-plugin/pull/38#discussion_r376263328 ## File path: src/main/java/org/apache/maven/plugins/shade/resource/ManifestResourceTransformer.java ## @@ -46,6 +46,8 @@ // Configuration private String mainClass; +private boolean skipRelocators; Review comment: FYI, I updated the code to reflect the proposal 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest
rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest URL: https://github.com/apache/maven-shade-plugin/pull/38#discussion_r376223734 ## File path: src/main/java/org/apache/maven/plugins/shade/resource/ManifestResourceTransformer.java ## @@ -46,6 +46,8 @@ // Configuration private String mainClass; +private boolean skipRelocators; Review comment: Hmm, sounds like an option but current code works well for jakarta relocation too so I'm mixed to make the conf more complex/verbose for end user. Is having the boolean AND dedicated relocators ok for you? Boolean could be reversed and named "inheritDefaultRelocators". 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest
rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest URL: https://github.com/apache/maven-shade-plugin/pull/38#discussion_r376103646 ## File path: src/main/java/org/apache/maven/plugins/shade/resource/ManifestResourceTransformer.java ## @@ -46,6 +46,8 @@ // Configuration private String mainClass; +private boolean skipRelocators; Review comment: Yes and no, they will have Bundle-Version entry in the manifest but most of the modules will be potential bundles but more often used in se/ee. For ex, the test i wrote used an openwebbeans output. It is used in se context, tomee, meecrowave (all not osgi) but also aries-cdi (osgi) or as plain osgi bundle. This is why i introduced this boolean. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest
rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest URL: https://github.com/apache/maven-shade-plugin/pull/38#discussion_r376068006 ## File path: src/main/java/org/apache/maven/plugins/shade/resource/ManifestResourceTransformer.java ## @@ -46,6 +46,8 @@ // Configuration private String mainClass; +private boolean skipRelocators; Review comment: Agree but the algorithm being a bit fragile until we use a full osgi header parsers/rewriter with exception algorithms for the general case, I'd prefer a parameter to disable it for not osgi case. I can enhance the parsing but thought it is not the core of shade plugin so is not worth that logic. If wrong, happy to revise this. Not perfect state but secured - would be sad to break a project for that new behavior IMHO. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest
rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest URL: https://github.com/apache/maven-shade-plugin/pull/38#discussion_r376068006 ## File path: src/main/java/org/apache/maven/plugins/shade/resource/ManifestResourceTransformer.java ## @@ -46,6 +46,8 @@ // Configuration private String mainClass; +private boolean skipRelocators; Review comment: Agree but the algorithm being a bit fragile until we use a full osgi header parsers/rewriter with exception algorithms for the general case, I'd prefer a parameter to disable it for not osgi case. I can enhance the parsing but thought it is not the core of shade plugin so is kot worse that logic. If wrong, happy to revise this. Not perfect state but secured - would be sad to break a project for that new behavior IMHO. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest
rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest URL: https://github.com/apache/maven-shade-plugin/pull/38#discussion_r375940979 ## File path: src/main/java/org/apache/maven/plugins/shade/resource/ManifestResourceTransformer.java ## @@ -51,6 +51,8 @@ // Fields private boolean manifestDiscovered; +private boolean skipRelocators; Review comment: it is set in the pom.xml and is an user config - as mainClass is . Guess I put it in the wrong comment block ;), will move 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-shade-plugin] rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest
rmannibucau commented on a change in pull request #38: [MSHADE-350] enable to rewrite the manifest URL: https://github.com/apache/maven-shade-plugin/pull/38#discussion_r375940979 ## File path: src/main/java/org/apache/maven/plugins/shade/resource/ManifestResourceTransformer.java ## @@ -51,6 +51,8 @@ // Fields private boolean manifestDiscovered; +private boolean skipRelocators; Review comment: it is set in the pom.xml and is an user config. Guess I forgot a setter, will add 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
