Skip to content
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

handle nested jars/zips when stripping jars/zips #52

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

tobias-hammerschmidt
Copy link
Contributor

When jars are produced by eclipse pde build or eclipse tycho build its possible to bundle compiled classes and/or resources within nested jars. When using the reproducible-build-maven-plugin with such a jar the resulting stripped jar won't be reproducible since the plugin doesn't handle these nested jars. This pr adds handling for nested jars/zips.

@Zlika
Copy link
Owner

Zlika commented Sep 28, 2021

Thanks for this PR.
I see two main problems in this PR.
First, the diff is difficult to read because there are lots of cosmetic changes (maybe performed by an automatic formatting tool) unrelated with the feature. Please remove these changes from this PR. If you think these cosmetic changes improve the readability of the code, you can propose a dedicated PR to clean the code.
The second problem I see is that the code now processes and changes every embedded jar/zip files. However, most of the time embedded jar files are just external dependencies that we do not want to change (because it would change their hash/signature and then you cannot check their integrity anymore by comparison with the reference on maven central).
So not handling nested jar/zip files by default is done on purpose.
In theory, nested jars should be stripped before being incorporated inside another jar. Can't you do that in your tycho build? If you cannot do that, then you should add some configuration options in this plugin to explicitly enable the handling of some specific embedded jars, but you cannot blindly change all the nested jar/zip files.

@tobias-hammerschmidt
Copy link
Contributor Author

Let me roll back the cosmectic changes which were introduced by some IDE autosave hook. Right now it is not possible to intercept the bundling within tycho since the nested jar as well as the final jar are packaged in a single step. I get your point regarding potential issues with nested third party dependencies. I'll update the code to handle stripping of nested jars/zips only for resources explicitly chosen by the user as part of the plugin configuration.

@Zlika
Copy link
Owner

Zlika commented Oct 1, 2021

Please rebase and force push to have a single commit with only the required changes.

@Zlika
Copy link
Owner

Zlika commented Oct 1, 2021

It would be nice to add an integration test for this feature... :-)

@tobias-hammerschmidt
Copy link
Contributor Author

Here we go :-)

@Zlika
Copy link
Owner

Zlika commented Oct 8, 2021

Thank you. But I cannot see this library.jar file in the commit. Did you forget to add it?

@tobias-hammerschmidt
Copy link
Contributor Author

The integration test mimics the behavior of a tycho build with nested jar so the library.jar is generated as part of the maven build (assembly plugin is called twice - first call to generate library.jar and second call to bundle the library.jar within the final artifact).

@Zlika
Copy link
Owner

Zlika commented Oct 8, 2021

ok fine, I didn't notice that.

@Zlika Zlika merged commit a9e535d into Zlika:master Oct 8, 2021
@Zlika
Copy link
Owner

Zlika commented Oct 8, 2021

Thanks!

@tobias-hammerschmidt
Copy link
Contributor Author

Thank you! Would you mind doing a release anytime soon? ;-)

@Zlika
Copy link
Owner

Zlika commented Oct 8, 2021

Yes. I would just want to replace the current travis build with github actions before doing a release. Maybe this week-end if I have some free time.

@Zlika
Copy link
Owner

Zlika commented Oct 16, 2021

@tobias-hammerschmidt v0.14 released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants