-
Notifications
You must be signed in to change notification settings - Fork 453
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
remove <keep>com.google.api.client.**, fixing packaging #434
Conversation
Before this commit, packaging is broken: the google oauth and api libraries depend on this library repackaging guava in a specific way. Parts of the jarjar configuration (above the <keep> declaration) do just that. However, for some reason, not all of the guava classes seem to be included: for instance, oauth will complain that the following class is missing: com/google/api/client/repackaged/com/google/common/base/Platform. What gives? guava-20 certainly contains that class. Even more strange, there _are_ things in that directory, just not _all_ of the classes expected. So, _some_ guava things are obviously being repackaged. Well, the inclusion of the keep rule removed in this commit somehow seems to either: - Repackage a very old version of guava that does not have all the classes that newer code expects - Remove some classes I suspect the former. Either way, this change makes all three libraries `mvn clean install` without a problem, and does not seem to have a negative impact (e.g., the existence of com.google.api.client classes in the jar does not seem predicated on this keep being present - confirmed with "jar tf targets/etc/etc").
The keep reduces the amount of repackaging to only classes that are used. (As mentioned in the last paragraph of jarjar's readme). Oauth should not be directly referencing the repackaged classes. I searched oauth and api, and there were no references to "repackaged". So I expect that for some reason jarjar is failing to include a class it should. The question is why doesn't it detect the class is necessary. None of http/oauth/api reference Platform. Can you provide the full error message? It should likely make it clear what is using Platform. |
This fixes a packaging problem described in googleapis#434. It is a cleaner fix than the fix applied in that PR.
Superseded by #435 |
For posterity, the error was:
This is with Guava 20 (IIRC). The Paltform reference in Strings is pretty explicit; no real cause for it to be handled improperly. So the thought was it was a jarjar bug. Upgrading the jarjar-maven-plugin fixed the problem. We also discovered that jarjar-maven-plugin forked jarjar :''( |
This fixes a packaging problem described in #434. It is a cleaner fix than the fix applied in that PR.
Before this commit, packaging is broken: the google oauth and api libraries depend on this library repackaging guava in a specific way. Parts of the jarjar configuration (above the keep declaration) do just that. However, for some reason, not all of the guava classes seem to be included: for instance, oauth will complain that the following class is missing: com/google/api/client/repackaged/com/google/common/base/Platform.
What gives? guava-20 certainly contains that class. Even more strange, there are things in that directory, just not all of the classes expected. So, some guava things are obviously being repackaged.
edit:
Well, it turns out the keep rule is intended to keep everything listed and all transitive dependencies. As in, methods and functions used in the included path, and those used in its dependencies, and so on. Anything not apparently used is discarded.
Unfortunately, jarjar 1.5 does not traverse the tree properly, and incorrectly discards things that are used, like the aforementioned Platform.class.
This PR removes the keep rule, stopping it from discarding any files. This PR is superseded by #434, which just bumps the version of the plugin to one that traverses the tree properly.