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
Fix #462 Delay Pom considered items to the final Target Platform calculation #470
Conversation
@mickaelistria this moves pom dependency metat-data computation out of the "maven-setup-phase" into the "target-resoloution-phase" and thus delays it until the final maven build starts. This will (after some adjustments) fix the long outstanding issue that tycho is not able to consumer "pom-first" bundles together with tycho ones in the same reactor build. |
All integration tests are green now, so from my side this is ready to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to discuss #462 (comment) before merging anything. I'm afraid this could bring regressions.
Are you aware of any project using |
@HannesWell do you want to try this out with m2e build to gather some more experience beyond the simple integration test? |
Yes, I will test it with the m2e build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minimal suggestions inline.
Also, please add documentation to TargetPlatformConfigurationMojo#pomDependencies to explicit how pom dependencies are now expected to participate or not in the resolution.
...bundles/org.eclipse.tycho.embedder.shared/src/main/java/org/eclipse/tycho/PackagingType.java
Outdated
Show resolved
Hide resolved
....tycho.p2.resolver.impl.test/src/test/java/org/eclipse/tycho/p2/resolver/P2ResolverTest.java
Show resolved
Hide resolved
...o.p2.resolver.impl/src/main/java/org/eclipse/tycho/p2/target/PomDependencyCollectorImpl.java
Show resolved
Hide resolved
...o.p2.resolver.impl/src/main/java/org/eclipse/tycho/p2/target/PomDependencyCollectorImpl.java
Outdated
Show resolved
Hide resolved
...o.p2.resolver.impl/src/main/java/org/eclipse/tycho/p2/target/PomDependencyCollectorImpl.java
Show resolved
Hide resolved
...2.resolver.impl/src/main/java/org/eclipse/tycho/p2/target/PreliminaryTargetPlatformImpl.java
Show resolved
Hide resolved
/rebase |
@mickaelistria I rebased this now on your change regarding local file transfers. |
only |
See https://ci.eclipse.org/tycho/job/tycho-github/job/PR-470/28/#showFailuresLink ; ProductMixedVersionTest also fails with the same reason. The Maven artifacts are not visible into the "blackboard" repository used by p2 director. |
okay found it, everything green now. |
...ho.p2.resolver.impl/src/main/java/org/eclipse/tycho/p2/target/TargetPlatformFactoryImpl.java
Outdated
Show resolved
Hide resolved
This looks good to merge soon, can you please squash the commits and add a commit message that describes the goal (delay resolution to allow for interacting with "lazily populated" Maven artifacts from reactor for better integration with eg Felix) and the most important part of the implementation (do not fail preliminary TP build -used for build order- is some requirements are missing most likely because of an artifact not yet available and let build continue further, and fail later if necessary + do not consider pom dependencies in extra p2-based build order computation) ? |
I'll combine this into one commit and check the last TODO if I will do it now or later... |
…Platform - if pom dependencies are considered allow for a partial resolution of the preliminary target platform - don't use pom considered dependencies in computation of build-order anymore - pom considered dependencies are now part of the final target platform calculation These changes allow for other reactor projects to contribute to the OSGi classpath results including for example bnd/felix bundle plugin. Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
Everything is one commit now. |
Thanks! Awesome change! |
Signed-off-by: Christoph Läubrich laeubi@laeubi-soft.de