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

[export] JPMS module info calculation fails #5258

Merged
merged 1 commit into from May 23, 2022
Merged

Conversation

pkriens
Copy link
Member

@pkriens pkriens commented May 23, 2022

There was a recent change dd2f0c5 that took a shortcut to calculate the embedded packages unconditionally. It used the bundle class path analysis to analyze all the jars in the executable.

However, this significantly slows down Eclipse but worse, it then tries to reinterpret the annotation headers. In my case, I got 3 types on the Bundle-Activator.

The analysis should only be done when it is needed and I'd suggest to do it either in a separate Builder or try to analyze the manifests for speedup.

This patch just makes the analysis conditional but that is a temporary fix to make it compatible with the the previous code.

This needs to be cherry picked for 6.3.0 since it is a very serious regression.

Fixes #5257

Signed-off-by: Peter Kriens Peter.Kriens@aqute.biz

There was a recent change bndtools@dd2f0c5 that took a shortcut to calculate the embedded packages unconditionally. It used the bundle class path analysis to analyze all the jars in the executable. 

However, this significantly slows down Eclipse but worse, it then tries to reinterpret the annotation headers. In my case, I got 3 types on the Bundle-Activator.

The analysis should only be done when it is needed and I'd suggest to do it either in a separate Builder or try to analyze the manifests for speedup.

This patch just makes the analysis conditional but that is a temporary fix to make it compatible with the the previous code.

This needs to be cherry picked for 6.3.0 since it is a very serious regression.

Fixes  bndtools#5257

Signed-off-by: Peter Kriens <Peter.Kriens@aqute.biz>
@pkriens pkriens requested a review from rotty3000 May 23, 2022 09:57
@pkriens
Copy link
Member Author

pkriens commented May 23, 2022

@rotty3000 I will commit this once it builds so I can use 6.4.0-SNAPSHOT again. However, I think we need a better solution since it will fail for your use case as well with the multiple Bundle-Activator types caused by the use of multiple manifest annotation headers. I expect there will be more of those issues since a mechanisms is used that is clearly not intended for the purpose. I think there are faster & better ways.

@pkriens pkriens merged commit ebe59ed into bndtools:master May 23, 2022
10 checks passed
Copy link
Contributor

@rotty3000 rotty3000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record this change is perfectly fine with me. Also note that I do see that the current approach has limitations that should be improved and have some ideas on how to accomplish that. Stay tuned.

@pkriens
Copy link
Member Author

pkriens commented May 23, 2022

has limitations

It will generate errors. Bundle-Activator defined by Header annotations is one since you get the aggregate of all bundlles & jars on the runpath. I also got an error because a bundle had some stuff in the default package.

Best approach imho is to use a new builder.

@rotty3000
Copy link
Contributor

👍

a new builder

is what @bjhargrave and I thought as well.

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.

Gradle Export Task fails
2 participants