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

Document strategy for shading dependencies and merging indices #100

Closed
vlsi opened this issue Jan 5, 2021 · 6 comments
Closed

Document strategy for shading dependencies and merging indices #100

vlsi opened this issue Jan 5, 2021 · 6 comments
Assignees
Milestone

Comments

@vlsi
Copy link
Contributor

vlsi commented Jan 5, 2021

AFAIK the current "standard" is to include index as META-INF/index.idx.
However, that would result in collisions when dependencies are shaded, so it looks like jandex is not ready for adding indices to Java libraries yet :(

I think it would be helpful if there was a documented procedure to merge indices (and remap classnames) for shading dependencies (e.g. Resource Transformers for maven-shade-plugin, Content Merging in com.github.johnrengelman.shadow Gradle plugin, and so on).

@vlsi vlsi changed the title Document a strategy for shading dependencies and merging indices Document strategy for shading dependencies and merging indices Jan 5, 2021
@n1hility
Copy link
Contributor

Hey visi. Some good points here. Although, I think the cleanest approach is that you just reindex the shaded jar. The overhead is about the same, if you merge we'd have to create a merge plugin tool, and that would throw away the existing indexes since names are radix trees. Further there might be other transformations that are done and this could mean an index might contain other data that is now invalidated by the shading process. If you just repeat the indexing phase everything is correct and accurate.

@vlsi
Copy link
Contributor Author

vlsi commented Jan 11, 2021

Are there configurations like "ignore private methods" / "ignore certain packages"?

If reindexing, then the configurations would have to be repeated for the reindex part :-/

@Ladicek
Copy link
Collaborator

Ladicek commented May 11, 2022

For reasons stated above, I'm inclined to just reject this. The best approach is to reindex the shaded JAR -- especially considering that classes may be relocated during shading, so some level of duplicate configuration is necessary anyway.

What the Maven plugin could and should do is offer a more streamlined configuration for this situation. Currently (in the smallrye branch), the Maven plugin expects that it will index target/classes (and possibly some additional file sets) and put the result into target/classes/META-INF/jandex.idx. The directory is configurable, but in this situation, we want to put the index directly into the JAR. I'm thinking probably a separate goal would be best, something like jar or jandex-jar.

@vlsi
Copy link
Contributor Author

vlsi commented May 11, 2022

considering that classes may be relocated during shading, so some level of duplicate configuration is necessary anyway.

What do you mean by "some level of duplicate"?

Case a

  1. Suppose shadow plugin knows the way to apply "relocation info" just like it currently knows the way to relocate classes
  2. Suppose shadow plugin knows the way to "merge indices" just like it currently knows the way to merge META-INF/SERVICES

Pros:

  • shading/merging jars would just work. There will be no duplication of the config.
  • there will be no need to know the parameters that were used to create the indices for the original dependencies

Cons:

  • Somebody should implement "relocate class" operation for an index
  • Somebody should implement "merge indices" operation

Case b

  1. Suppose shadow plugin knows the way to "reindex the jar" after shading

Pros:

  • The index will be re-created every time, so index corruption will have less chances to happen
  • No need to implement relocate classes and merge index operations

Cons:

  • If the original dependency fine-tuned the index (e.g. they included only a specific subset of classes or something like that), then the newly created index should somehow get that configuration as well. I would say, that the ones who shade dependencies have little to no clue about the index contents. You don't know which classes were supposed to be indexed.

@Ladicek
Copy link
Collaborator

Ladicek commented May 12, 2022

I was solely thinking of a solution in the Jandex Maven plugin. It didn't occur to me that we could provide a resource transformer to be used by people that use the Maven Shade plugin. I'm looking at that API and I think it would be almost sufficient without having to actually merge indices. The resource transformer can be notified of any META-INF/jandex.idx file, read it and remember the names of classes present in the index. Then, when the shaded JAR is being written, the resource transformer would go through the remembered class names, apply relocations and index the relocated classes from the new JAR. Except the new JAR is not available at the moment -- the API only exposes a JarOutputStream to which the newly constructed index would have to be written.

My plan here is to provide a new goal of the Jandex Maven plugin to index a JAR. That goal would take a path to the JAR, set of includes and excludes relative to the root of that JAR (similarly to the existing goal), and would add/replace a META-INF/jandex.idx in that JAR.

@Ladicek
Copy link
Collaborator

Ladicek commented May 12, 2022

Done in #198.

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

No branches or pull requests

3 participants