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

[m2e] Avoid runtime errors due to API changes between 1.x and 2.x #5429

Merged
merged 3 commits into from
Nov 14, 2022

Conversation

timothyjward
Copy link
Contributor

Several types in M2E have been updated to use Lists rather than arrays. We have to implement the extra interface methods (where needed) and use reflective calls to avoid linking to a method which may not exist. All of this code is ugly, and should be removed once we can use a base of M2E 2.x.

Signed-off-by: Tim Ward [email protected]

Several types in M2E have been updated to use Lists rather than arrays. We have to implement the extra interface methods (where needed) and use reflective calls to avoid linking to a method which may not exist. All of this code is ugly, and should be removed once we can use a base of M2E 2.x.

Signed-off-by: Tim Ward <[email protected]>
@timothyjward
Copy link
Contributor Author

I believe that this fixes #5409

Some types (notably ArtifactKey) have become record types in M2E 2.x - we need to cope transparently with this API change until we increase our base to M2E 2.x. As always, this is ugly code and it should be removed once we can use a base of M2E 2.x

Signed-off-by: Tim Ward <[email protected]>
@timothyjward
Copy link
Contributor Author

I believe that the two commits above make everything work again. It's pretty horrible, and I look forward to being able to remove these hacks once Bndtools 7.0.0 uses M2E v2.x as a base.

@stbischof
Copy link
Contributor

thx @timothyjward

@timothyjward
Copy link
Contributor Author

thx @timothyjward

I'm hoping that this means it works for you? You can build locally with ./gradlew p2 and then install from org.bndtools.p2/generated/p2/

@stbischof
Copy link
Contributor

this means just thank you for taking it. and placing the method there where the error happened.

I will build it later and report

@bjhargrave bjhargrave added the maint-candidate Issues or pull requests that are candidates for a maintenance release label Nov 14, 2022
Copy link
Member

@bjhargrave bjhargrave left a comment

Choose a reason for hiding this comment

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

Once this is verified and merged, I will cherry-pick the commits to next for 6.4.0.

When running in M2E 2.x the  delegates to the array version of the mavenProjectChanged method, however as this is declared by the 1.x interface the JVM fails to link the method call. We therefore re-declare the method in the Abstract class to give the linker something to link to.

Signed-off-by: Tim Ward <[email protected]>
@stbischof
Copy link
Contributor

Screenshot from 2022-11-14 17-20-43

with 2022-9 I do not see bundles in the bndrun view.

but the error is not longer there

An internal error occurred during: "Updating Maven Project".
Receiver class bndtools.m2e.MavenDependenciesRepository does not define or inherit an implementation 
of the resolved method 'abstract void mavenProjectChanged(java.util.List, org.eclipse.core.runtime.IProgressMonitor)' 
of interface org.eclipse.m2e.core.project.IMavenProjectChangedListener.

@timothyjward
Copy link
Contributor Author

Screenshot from 2022-11-14 17-20-43

with 2022-9 I do not see bundles in the bndrun view.

but the error is not longer there

An internal error occurred during: "Updating Maven Project".
Receiver class bndtools.m2e.MavenDependenciesRepository does not define or inherit an implementation 
of the resolved method 'abstract void mavenProjectChanged(java.util.List, org.eclipse.core.runtime.IProgressMonitor)' 
of interface org.eclipse.m2e.core.project.IMavenProjectChangedListener.

It sounds like an improvement then. I found another place that needed a repeat of an abstract method to satisfy the linker in c8ea019 - if that wasn't the commit you build then it may be worth trying. Otherwise I'm not sure how else to fix this.

@stbischof
Copy link
Contributor

i build after c8ea019

@timothyjward
Copy link
Contributor Author

I think that this is ready to merge. I can't guarantee that all the M2E integration is working, but it does appear substantially improved. I am able to build/resolve/run my test workspace in both old and new versions of Eclipse.

@stbischof
Copy link
Contributor

jep this should be merged!
@timothyjward
old: 2022-03
new: 2022-06?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maint-candidate Issues or pull requests that are candidates for a maintenance release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants