Bugzilla – Full Text Bug Listing
| Summary: | Introduce a target platform object with final metadata | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | Tobias Oberlies <t-oberlies> |
| Component: | Tycho | Assignee: | Igor Fedorenko <igor> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | gunnar, igor |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 353889, 362883, 369316, 412416 | ||
Igor's summary of the Tycho call on 21.11.2011: Target platform changes - TargetPlatformResolver will be renamed to TargetPlatformBuilder and will expose two methods, one that returns "dumb" target platform, i.e. calculated without consideration of project dependencies, and another method that "resolves" the dumb target platform using project dependencies. The same TargetPlatform interface will be used to represent both the dumb and the resolved target platforms. - The dumb target platform contents is a combination of IUs defined by .target files (plural!), all IUs from repositories with layout=p2, IUs from Maven local repository, IUs introduced via pomDependencies=consider and IUs representing reactor projects. - TargetPlatform implementation will be changed to download artifacts lazily. - OsgiBundleProject implementation will be changed to resolve the dumb target platform before calling into Equinox State resolver - Feature 0.0.0 and .qualifier expansion will be reworked to use the dumb target platform to select the latest version, TargetPlatformBuilder "resolve" method to validate that the expanded versions can actually be installed. - Reactor build order OSGi bundle projects will be determined by resolved target platform. Reactor build order for other project types will be determined by shallow matching of provided and required capabilities of reactor projects. Dependency resolution changes - extraRequirements and optionalDependencies configuration parameters will be moved from target-platform-configuration block to tycho-compiler-plugin configuration block - extraRequirements and optionalDependencies will be used by OsgiBundleProject when resolving the dump target platform and will affect project dependencies visible to all maven plugins that operate of the project - optionalDependencies will allow two values, "required", the default, and "ignore" (In reply to comment #1) > The same TargetPlatform interface will be used to > represent both the dumb and the resolved target platforms. "Target platform" shall always denote the "dumb target platform", so we need a different namer for the latter. My original proposal was "ResolvedProjectDependencies" or "ResolvedDependencies", but that name looks confusing in the context of the Equinox resolver used to compute the compile class path. For now, I have chosen "DependencyArtifacts" as new name for the resolved project dependencies. Both "dump" and "resolved" are just collections of artifacts and their metadata. I don't see why we need separate data structures to hold them. (In reply to comment #3) > Both "dump" and "resolved" are just collections of artifacts and their metadata. > I don't see why we need separate data structures to hold them. Right now this is a purely practical consideration: The existing data structures are currently very different, and I am refactoring them in small steps. Unifying their implementations is not my first concern, but this may be the eventual result. As for the interfaces, I have a preference for having different ones for "dumb" and "resolved" for type safety. If for example the 0.0.0 replacement logic only took the "dumb" type, bug 352081 could no longer occur. (In reply to comment #2) > For now, I have chosen "DependencyArtifacts" as new name for the > resolved project dependencies. This change has been pushed to master (f4aa146). It is just a rename - but since it touches a lot of files, we probably wanted to keep this separate from functional changes anyway. I'm currently working on splitting target platform computation from dependency resolution. (In reply to comment #6) > I'm currently working on splitting target platform computation from dependency > resolution. This is mostly done. I still want to review the commit so I haven't pushed it to master yet - but it is available here: https://github.com/oberlies/tycho/tree/364134_resolver_split I reviewed 364134_resolver_split and although I did not like everything I saw, I believe it is a step in the right direction and can be merged to master. Few comments and observations 1. Javadoc. For example, I had to look at call hierarchy of TargetPlatformBuilder.publishAndAddArtifactIfBundleArtifact before I could understand the real purpose of this method. Likewise, looking for workspace references was the only way to understand the need for empty TargetPlatform interface. 2. Names will likely need more work. For example, why TargetPlatformBuilder "computes" the target platform? Verb "build" seems to better reflect what builders do. 3. I am not sure I agree with proposed split of functionality between TargetPlatformResolver and TargetPlatformBuilder. From client point of view, I think it makes more sense to have one interface that provides functionality related to the dumb target platform and another interface that provides resolution logic. 4. I assume the next step after this refactoring is to change DefaultTychoDependencyResolver.resolveProject method implementation to NOT unconditionally invoke resolver.resolveDependencies. I think this resolution logic belongs to AbstractTychoProject.resolveClassPath implementations. Does this match your plans? I've committed (aa176ca) the resolver_split change with a few modifications. @Igor: Thanks for the review. (In reply to comment #8) > 1. Javadoc. For example, I had to look at call hierarchy of > TargetPlatformBuilder.publishAndAddArtifactIfBundleArtifact before I could > understand the real purpose of this method. This wasn't change by the refactoring, but this could be revised. Although it does say quite a lot already about its semanticts - that's at least what I thought... > Likewise, looking for workspace > references was the only way to understand the need for empty TargetPlatform > interface. I've added a comment in the interface and the commit message. > 2. Names will likely need more work. For example, why TargetPlatformBuilder > "computes" the target platform? Verb "build" seems to better reflect what > builders do. I've changed this to buildTargetPlatform. Most of the other names were copied from the previously existing code and will get another review. > 3. I am not sure I agree with proposed split of functionality between > TargetPlatformResolver and TargetPlatformBuilder. From client point of view, I > think it makes more sense to have one interface that provides functionality > related to the dumb target platform and another interface that provides > resolution logic. Like the P2ResolverFactory? Or did you mean to have P2Resolver and TargetPlatformBuilder merged? These interfaces don't have any common methods, so I don't know why you'd want to do this. > 4. I assume the next step after this refactoring is to change > DefaultTychoDependencyResolver.resolveProject method implementation to NOT > unconditionally invoke resolver.resolveDependencies. I think this resolution > logic belongs to AbstractTychoProject.resolveClassPath implementations. Does > this match your plans? Yes, but this is rather further in the future (because this requires changing the "usage in the reactor" detection). Next, I'll probably make use of TargetPlatform when fixing bug 359902. (In reply to comment #9) > (In reply to comment #8) > > 4. I assume the next step after this refactoring is to change > > DefaultTychoDependencyResolver.resolveProject method implementation to NOT > > unconditionally invoke resolver.resolveDependencies. I think this resolution > > logic belongs to AbstractTychoProject.resolveClassPath implementations. Does > > this match your plans? > Yes, but this is rather further in the future (because this requires changing > the "usage in the reactor" detection). Next, I'll probably make use of > TargetPlatform when fixing bug 359902. ... but if you want to implement the proposed change now, feel free to do it. I agree that something like this should be done. On second thoughts: If you were talking about the reactor build order thing, I think there should be a new bug report for this. (In reply to comment #1) > - Reactor build order OSGi bundle projects will be determined by > resolved target platform. Reactor build order for other project types > will be determined by shallow matching of provided and required > capabilities of reactor projects. Sorry for the compile error in tycho-extras. I didn't manage to do it before I had to leave from work - and this morning you had already done it. (In reply to comment #9) > Next, I'll probably make use of TargetPlatform when fixing bug 359902. I figure that this doesn't work because TargetPlatform contains dependency only metadata. I guess that I'll also need an object representing the full/final target platform. I'll see what it takes to implement this in the next couple of days. (In reply to comment #12) > (In reply to comment #9) > > Next, I'll probably make use of TargetPlatform when fixing bug 359902. > I figure that this doesn't work because TargetPlatform contains dependency only > metadata. I guess that I'll also need an object representing the full/final > target platform. I'll see what it takes to implement this in the next couple of > days. What is "full/final target platform"? (In reply to comment #13) > What is "full/final target platform"? It is similar to the current TargetPlatformImpl, but with the full p2 metadata of the referenced reactor projects (as it is currently created by the tycho-p2-mojo) instead of the dependency-only metadata of all reactor projects. Conceptually, this exists in parts today (e.g. RepositoryReferenceTool.getVisibleRepositories is pretty close to what I mean with "full/final target platform"), but I want this to be a real object with a more obvious data flow. Do you plan to keep it "full/final" target platform as a separate object or amend existing TargetPlatform with full metadata as it becomes available? (In reply to comment #15) > Do you plan to keep it "full/final" target platform as a separate object or > amend existing TargetPlatform with full metadata as it becomes available? I plan to make this a separate object, because I want to make sure that none of the dependency-only metadata can leak into the build results of eclipse-repository, eclipse-product, etc. Changing the title from "Clearly distinguish target platform and project dependency resolution" to "Introduce a target platform object with final metadata" to clarify the main aim of this enhancement. Such a "final" target platform object would be useful for the following features/fixes: - When replacing inclusion versions in feature.xmls, the candidate versions should be taken from the "final" target platform to fix bug 352081. - The test runtime could be resolved from the "final" target platform in order to prevent different behaviour between full reactor builds and module re-builds (e.g. bug 367701). - A key to the implementation of the "final" target platform object is to collect the build output (artifacts and metadata) in a single location so that it can be added into the target platform of the downstream modules. With such an object in place, it should be possible to cleanly replace the build output with the previous build result, which is an important step for implementing bug 362883. (In reply to comment #16) > (In reply to comment #15) > > Do you plan to keep it "full/final" target platform as a separate object or > > amend existing TargetPlatform with full metadata as it becomes available? > I plan to make this a separate object, because I want to make sure that none of > the dependency-only metadata can leak into the build results of > eclipse-repository, eclipse-product, etc. Target platform contains metadata of all reactor projects, so it will always contain dependency-only metadata for some reactor projects until very end of the build. Proper reactor build order should already guarantee that dependency-only metadata does not leak into repositories. There is, of course, always a possibility of a bug in reactor build ordering and/or metadata publishing, but in this case build failure with clear error message is more appropriate. Refactored P2TargetPlatform implementation to hold instances of IReactorArtifactFacade and to calculate installable units contributed by reactor project dynamically, at each request [1]. Made TargetPlatform accessible to mojos via new TychoProject#getTargetPlatform API [2]. Publish 'final' P2 metadata calculated during the build back to IReactorArtifactFacade [3]. This makes final metadata accessible to all reactor projects built this project. As a temporary solution for bug 353889, changed RepositoryReferenceTool to always recalculate eclipse-repository project dependencies. This ensures that packaged eclipse repositories include 'final' p2 metadata from reactor projects built before this project. I've opened follow-up bug 369676 to introduce safeguards against 'leakage' of dependency-only metadata into generated eclipse repositories. [1] http://git.eclipse.org/c/tycho/org.eclipse.tycho.git/commit/?id=c425a9152ad284aff9391f82e9300af33c02cc16 [2] http://git.eclipse.org/c/tycho/org.eclipse.tycho.git/commit/?id=cc02f05dd1106fad72a3384fc3aa2251c56a0526 [3] http://git.eclipse.org/c/tycho/org.eclipse.tycho.git/commit/?id=1f57360e6ac222b50c22245428ed4dd26f13b87a [4] http://git.eclipse.org/c/tycho/org.eclipse.tycho.git/commit/?id=d0aa87d4845e3e14392310c89bc374eb3774e137 *** Bug 369316 has been marked as a duplicate of this bug. *** |
Historically, Tycho didn't distinguish between the "target platform" (=the set of artifacts that can be referenced by the build) and the actual process of matching the project dependencies (e.g. from the MANIFEST.MF) against the target platform ("project dependency resolution"). However, discussions about current activities and future projects show that this distinction is necessary: - Dependency resolution is done more than once in eclipse-test-plugin projects: first to determine the artifacts to be considered for the build class path, and second to determine the set of artifacts in the test runtime. These steps (both done by a p2 dependency resolver) can have different settings, e.g. for how optional runtime dependencies should be treated (see bug 351842), or for additional artifacts in the test runtime (see [1]). - eclipse-feature currently uses dependency resolution to determine the version of included bundles. However this may result in incorrect results (bug 352081). The solution to that problem is to not use dependency resolution, but rather look up the right version in the target platform directly. This is currently not possible, because there is no "target platform" data structure available to the mojo that does the version substitution. - A more flexible eclipse-repository (e.g. allowing multiple versions of the same bundle) also needs to work on the target platform directly instead of the resolved project dependencies. The same makes sense for the future eclipse-product. The main hurdle to a separation of target platform and dependency resolution is the current structure of the code, so the first step would be a refactoring of the current P2TargetPlatformResolver and related classes. [1] http://wiki.eclipse.org/Tycho/Packaging_Types#eclipse-test-plugin