-
Notifications
You must be signed in to change notification settings - Fork 114
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
[Maven-Runtime] convert projects into standard Maven-projects (#423) #466
Conversation
no idea, are they listed in the target-platform state? Maybe there is another place in PDE where the BUNDLE_ROOT is not considered correctly? Have you tried this out with a current i-build? |
No they where not. But I have now tested it in the 'final form' with an Eclipse launched from my m2e workspace that contains PR #440 and a current I-Build target platform. And with my changes I just suggested in PR #440 it worked better. At least after a clean-full-build projects are listed in the |
I won't expect to have both for a pure maven bundle, is there a reason we need |
The containers are not mixed in one project. I meant that the I suspect that the ClassPath containers have to be adjusted for this case, but I have to investigate it. |
Okay lets try to just do it step-by-step then :-) |
This seems to be fixed by apache/felix-dev#124 ? |
Only from updating to
|
Using the mixed Maven/Eclipse project reactor also failed:
Or did I do something wrong? |
I cannot confirm this. I my test setup I didn't make a difference. Have you checked if the .classpath file is a 'pure' Maven-project classpath and does not contain elements generated before? When switching branches it can happen that the .classpath file is not updated. |
The changes for this are currently not merged to master so only visible if you checkout the PR and build a custom version of tycho. |
🤦🏽♂️ Yes, I noticed that too just in this second. This cannot work here. |
When locally installing eclipse-tycho/tycho#470 and using Tycho 2.6.0-SNAPSHOT the build of commit ce46946) fails:
|
Just wondering is m2e still using a site.xml and update-site packaging? |
When locally installing eclipse-tycho/tycho#470 and using Tycho 2.6.0-SNAPSHOT the build of commit faab02c) fails: Could it be a problem that the packaing of the maven-runtime-bundles is
|
No it is just the (legacy) name. But it shouldn't be a problem to rename it. |
Well at least the build starts ;-) Strange enough the tychobuild on the CI fails for some reason it seems my local state is slightly different but I don't see how... can you try to execute the test project in |
I have no clue either. But there is likely some other (unlreated) problem because only switching from Tycho 2.5.0 to 2.6.0-SNAPSHOT fails the build, with but also without tycho PR 470!
That test setup builds successfully on my computer. |
can you try eclipse-tycho/tycho@1fe29d7 if that works? |
With Tycho at that state the m2e build works. It also works with eclipse-tycho/tycho@4980ba6. I can go the history further up to identify the commit that broke the build. |
Thats good, as this most probably is the next candidate for the release... |
By the way if you could derive a small reproducer of the problem that would be great otherwise I'll try to test with the m2e build... |
I have to correct my statement, because when I build locally even the current tycho-master is working for this branchs commit ce46946 (switch to new Tycho version). But it failed on Jenkins. The only difference between my local build and M2E-Jenkins is that on Jenkins the eclipse-sign profile is active and with it the |
Even when running the p2-meta goal in my local build it passes with Tycho 2.6.0-SNAPSHOT. On Jenkins it still fails even when the From inspecting the console-log I assume the problem is that the The question is, what is the root of this problem. IIRC the artifact replacement can be tricky for long standing PRs because the snapshots then contain more recent versions than the PR produces. But there have not been such change in the meantime. I will try if a version update fixes the problem. |
OK, it looks like the problem is not in the current Tycho snapshot rather than in my change. |
Never mind, the more testing the better. |
@HannesWell the 2.7.0-SNAPSHOT should now contain this feature soon. |
At least it is something strange, but as we do not build the code one wont notice it, in general I think Tycho do not care much about the source, but the new resolver with Tycho 4.0 will do and hopefully better address such missconfigurations. For this specific error I think it is more the |
Yes we do. It is provided by And this PR only changes the way the 'maven-runtime' bundles are build. The generated Manifest will remain unchanged (at least the relevant entries). |
I still think it might be good to not embeds the jars but having them as maven-target components that are fragments to the runtime bundle and export their additional packages using a customization in the target. But I have not tried if this works any better... (most probably we would need some hacks like extensible api header for PDE then).... but |
One big problem that I see is that, since OSGi is a dynamic environment and we cannot completely control which bundle is wired to which other bundle. Therefore it could happen that one component from the maven-runtime is actually wired to another jar than the original maven installation, which could lead to subtle differences to the original Maven installation and therefore hard to find errors. Another problem could be if a Service-Loader is used in one of the embedded component. Then we would have to configure the Service-Loader Mediator to make it work. Furthermore I think it would be good to have at lest a few Maven projects so that we have to eat our own dog-food. :)
Is that already available as snapshot? I wouldn't mind to use Tycho 4.0-SNAPSHOT if that helps. |
Fragments can only be attached to their declared hosts. There is no wiring involved.
Fragments are not "components" in that sense and you cannot act uppon them as they have no BundleContext/ClassLoader at all but just extend their host.
For the installation I would just use a p2.inf as swt does: this force p2 to always install all fragments. |
0e769b0
to
48f0e95
Compare
Indeed, I didn't consider that bundles and their fragments share the same class-loader and effectively have a flat shared classpath. And yes the fragment-host's version-range should be made so tight that it only allows exactly one version. Another difficulty is in the test execution during the build because tycho-surefire does not add fragments to the runtime.
What would be the advantage compared to a generated feature? We could then include that feature into the m2e feature, which would also force p2 to install a Plug-in? And I have the impression that such p2.inf has to be maintained manually or at least generated in a script. Anyway, I think I managed to work-around the classpath-validation error by just adding an additional Import-Package The build now succeeds and main and source artefacts are build (I have to check again if their content did not change in a relevant way). In the IDE also the .classpath file is generated properly (on full build) and M2E even managed to find the sources for the embedded jars (but I don't know how^^). |
48f0e95
to
775b5c0
Compare
80a0686
to
2013796
Compare
As soon as #1068 is available in the snapshots (I think that is OK, since M2E devs should always use the latest snapshots and we are close to the next release), this can be considered for merging. The main missing part is to double check that the jars build eventually did not change in a significant way. |
057ebb6
to
aa66a88
Compare
This Maven-project contains elements common to all M2E modules regardless of their packaging-type and serves as their parent.
aa66a88
to
c79a567
Compare
Almost everything looks good now. Only two things are currently not working: @laeubi do you have an idea why this is the case? Do you think https://tycho.eclipseprojects.io/doc/3.0.0/tycho-source-plugin/feature-source-mojo.html#plugins could help? And the other thing is that, sometimes in the IDE the maven-resources-plugin is not executed because the mojo configuration failed and therefore the content of |
113b0ec
to
6603f87
Compare
Thank you @laeubi for the hint (via private-chat) about the The second point I mentioned, the resolution failure of the resources-plugin didn't happen again, so maybe it was a local problem on my computer. When the current build has succeeded I will make some final check of the generated artifacts and then I think this is finally ready for submission. |
6603f87
to
4e47cc9
Compare
I just did the final check of the result jars and the resulting content is equal (in all relevant aspects). So this is finally ready for submission. 🎉 |
This PR has the goal to convert M2E's Maven runtime bundles (back) into full standard Maven projects that don't require special tooling/build as suggested in and to resolve #423.
To check for version regressions and to generate Eclipse-style build qualifiers the corresponding Tycho plug-ins are still used in the standard Maven build. This requires plug-in definitions like in the m2e-core/pom.xml, but those definitions should not be duplicated. At the same time
m2e-maven-runtime/pom.xml
cannot inherit from m2e's root pom.xml any more (because that one depends on the m2e-maven-runtime components) I introduced a m2e-parent Maven-project whose pom.xml contains the configuration shared by m2e-core/pom.xml andm2e-maven-runtime/pom.xml
. Besides the build configuration also the legal elements. If you agree that this is a good idea, the m2e-parent could be introduced in a separate preceding PR?As soon as #439 is merged the
.settings/org.eclipse.pde.core.prefs
and.project
files can be completely removed.With the current state of this PR the build is successful but in the IDE, the Maven-projects seem to not be considered by PDE as actually expected (and I already successfully tested with a small project). The m2e-maven-runtime bundles are always marked as missing in the Manifest's of other m2e plug-ins that require them.
@laeubi if you don't have an immediate idea why this is the case, I will investigate it in the next days.