search

eed3si9n / jarjar-abrams

an experimental Scala extension of Jar Jar Links
27 stars 21 forks source link

Unable to `zap` non-`.class` files #48

Closed acourtneybrown closed 9 months ago

acourtneybrown commented 10 months ago

👋 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.

https://github.com/eed3si9n/jarjar-abrams/blob/9d0eb03caef24f1f04229b77cb2a492625804d80/jarjar/src/main/java/com/eed3si9n/jarjar/ZapProcessor.java#L31-L36

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 commented 10 months ago

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 commented 10 months ago

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

acourtneybrown commented 10 months ago

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 commented 9 months ago

👋 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 commented 9 months ago

Thanks for the contribution @patrick-premont! https://github.com/eed3si9n/jarjar-abrams/pull/53 is merged.