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

Unable to zap non-.class files #48

Closed
acourtneybrown opened this issue Jan 18, 2024 · 5 comments
Closed

Unable to zap non-.class files #48

acourtneybrown opened this issue Jan 18, 2024 · 5 comments

Comments

@acourtneybrown
Copy link

👋 Hey, we're trying to use jarjar via https://github.com/bazeltools/bazel_jar_jar while migrating a build to Bazel from Gradle. The Gradle shadow plugin that we're trying to replace allows for the removal of any file type/extension as part of the jar shading process. It appears that the ZapProcessor only allows zaping .class files.

public boolean process(EntryStruct struct) throws IOException {
String name = struct.name;
if (name.endsWith(".class"))
return !zap(name.substring(0, name.length() - 6));
return true;
}

I was thinking of removing this constraint to zap, but wanted to see if there were any objections before going too far down that path.

Alternatively, are there other suggestions for how to handle this use-case? We're currently seeing .proto files & native libraries added to the shaded jar, but would like them excluded.

@eed3si9n
Copy link
Owner

I was thinking of removing this constraint to zap, but wanted to see if there were any objections before going too far down that path.

I kind of like the idea of removing the *.class only constraint rather than adding a new vocabulary that we now need to remember to support. @johnynek wdyt?

@johnynek
Copy link
Collaborator

I agree that reusing zap is ideal and removing non-class files from a jar is a real use case.

@acourtneybrown
Copy link
Author

I initially played around with updating zap to allow for other files, but realized that the way that it handled the pattern implicitly assumes that it's going to be operating on Java packages (eg: org.apache.kafka.**) instead of file paths (eg: org/apache/kafka/**/*.proto). That's when I took the route in #49 to separate out then handling of zaping Java packages versus zaping general file paths within the input jarfile.

@patrick-premont
Copy link
Contributor

👋 Hi Eugene, Oscar.

Thanks Adam for your PR. I've prepared PR #53 that generalizes zap.

Since rule does match resources (based only on their path, not their filename) changing zap to mirror this behavior seems appropriate.

@eed3si9n
Copy link
Owner

Thanks for the contribution @patrick-premont! #53 is merged.

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 a pull request may close this issue.

4 participants