-
Notifications
You must be signed in to change notification settings - Fork 193
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
Delay Pom considered items to the final Target Platform calculation #462
Comments
FYI @HannesWell this will finally allow to have a "mixed" setup of plain maven plugins (e.g. felix-bundle-plugin |
…atform calculation Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
That's great! So for eclipse-m2e/m2e-core#466 two builds would not be necessary any more? Thanks for looking into this.
Not sure if this exactly related, but shouldn't the Tycho (Target-Platform) resolution be skipped for clean builds since #166? |
Yep but only if clean in the only phase I think. Just consider the case that there is one project that takes very long (e.g. 30 seconds) to resolve its classpath. If I now issue a With a delaying it to the actual build, while the slow porject resolves the others could be cleaned/build (given there are no inter-dependency relations between them). |
That's right. I also just wanted to mention it, I don't think it does not make this issue irrelevant.
Sounds promising. Do I understand you right, that the classpath resolution will be trigged for each project in a separate (java concurrent Executor?) task in the beginning of the build and when a particular project is next to be build Tycho waits for the cp-resolution task of that project to complete. And if the previous projects took long enough to build that task will already be completed. So if one is lucky and the first projects to build have a classpath to resolve quickly and take long enough to build, the additional total runtime of the cp-resolution can go to zero? |
It then uses the usual maven |
…cord those Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
…cord those Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
…Platform calculation Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
I'm not sure what you mean by "delay", do you mean they're ignored? If I'm not mistaken, the preliminary target platform is used in order to compute the build order; and the pomDependencies can affect the build order, so I don't get how the build order can remain correct if pom dependencies are ignored. |
Moving it out of the phase where we compute build order and dely it until a later stage in the regular maven build.
Can you give any example? As pom dependencies are already there at the pom level there is no need for tycho to take those into account at this stage. |
For example, bundles A and B are in reactor, bundle Z is a Maven dependency defined in pom.xml. A requires-bundle on Z, Z requires-bundle B and reexports. So packages from B must be visible to A, B must be built before A. If pomDependencies are ignored while computing build order, Tycho will lead to incorrect results in some cases, that would be a regression. |
Could you please provide a runnable example (or better integration test) for this? Is this a real use-case or just some kind of academic example? |
I have added an integration test to the PR now that shows a mixed reactor build, the output is as follows:
as one can see the |
Is this case even possible? Because Z is a Maven project (that has a OSGi compliant MANIFEST.MF generated in its final jar) it only uses the Maven mechanism to resolve its dependencies and therefore cannot know B, unless B is also a 'pure' Maven project. I first also had concerns about cases like that, but came to the conclusion that Maven projects cannot depend on Eclipse-Plugin projects because they only have the Maven dependency mechanism, while Eclipse projects have Eclipse (possible extended by Maven) dependencies. So Maven projects can only depend on other Maven-projects while Eclipse-plugin projects can depend on both. In the end Maven projects have to respectively will be (if the pom-dependencies of a Eclipse plug-in are also considered in the Reactor order) always build first. |
It is actually possible, assuming a variation (old version or other provider) of B pre-exists and is used when Z is built. Or a similar example can be considered with Import-Package, with another provider for the packages.
It's more an academic example and the goal is more to discuss the change fully before deploying it in order to anticipate potential breakage it can cause. But I think the value of the change is very high and it can be acceptable to break some cases if there is a consensus they're fine to be broken. |
It is possible as tycho computes the dependencies in an "dynamic" way, I can think of a simpler example. On the other hand P can only have a GAV dependency that matches exactly one version. From Maven POV the Tycho way might seem to "threaten the stability of the build", but that's how it works at the moment.
I think we should support people as much as possible, my experience with pomDependecies was that it was rather limited and hard to guess if something worked or not and finally has always chosen other ways. Thats why I wonder if there are some projects actually using this feature beyond very simple examples (e.g. injecting some additional dependencies that are not available as update-sites).
I think we should even narrow this don more: pomDependencies should only resolve to pom declared dependencies as well, as described by @HannesWell this would make things much more clear and consistent. If one still needs such a case as described above there are some ways to accomplish this:
|
What do you think about we start the 2.6.0 release (as people already have asked for) and then we have more time to find bugs/clean up things for 2.7.x... |
Fixing bugs should happen before a releaser. Given the amount of recent and pending changes, the code is IMO too "fresh" to be released. If you want to complete the work about ignoring pomDependencies in build order resolution, then we should allow at least 1 extra week of testing to consumers after completion to verify it fixes the issues as expected and doesn't cause unexpected regressions. |
I'm also fine with including it in 2.6.0 that's just an idea if we think we should take more time to investigate for this feature. |
…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>
- 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>
Currently pom-dependecies are calculate as part of the
PreliminaryTargetPlatform
and thus in the maven setup phase of Tycho.As pom-dependecies are already part of the model the are not strictly required in the
PreliminaryTargetPlatform
and thus should be delayed until actually required (ReactorRepositoryManagerFacade.computeFinalTargetPlatform
/ReactorRepositoryManagerFacade.getFinalTargetPlatform
) to mitigate the following restrictions:validate
/clean
goals are executed depending on the configuration heavy work will unnecessarily be performedThe text was updated successfully, but these errors were encountered: