Bugzilla – Full Text Bug Listing
|
Description
Anthony Dahanne
Created attachment 199289 [details]
This is a set of maven tycho modules to reproduce the issue
It should be possible to add custom bundles to p2 runtime used by tycho already. To do this you need to implement org.eclipse.tycho.p2.facade.internal.TychoP2RuntimeMetadata component interface to return GAV of additional bundles you need, package the component into a maven plugin and add the plugin with <extensions>true</extensions> to pom.xml. See org.eclipse.tycho.p2.facade.internal.DefaultTychoP2RuntimeMetadata for default implementation of TychoP2RuntimeMetadata. We'll be happy to answer further questions on tycho-dev mailing list. Created attachment 206887 [details]
maven plugin to add the needed touchpoint to the tycho p2 runtime
Igor,
thank you for your answer.
Following your suggestion, I created a new maven plugin that I added to my parent pom build section :
<build>
<plugins>
...
<plugin>
<groupId>sample.plugin</groupId>
<artifactId>hello-maven-plugin</artifactId>
<version>1.0-SNAPSHOT</version>
<executions>
<execution>
<phase>compile</phase>
<goals>
<goal>touch</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
I am pretty new to developing plexus components, so I am not sure I did the right thing (see attached maven plugin); am I doing something wrong ? if so could you please tell me what ?
On another side, I also redefined the tycho-p2-runtime so that the equinox runtime used by tycho is based on p2 : and I could build my project (first zip).
I think this approach (make the tycho p2 runtime p2 aware) is the best approach since it does not urge the user to create a maven plugin each time a new meta requirement is needed in the tycho p2 runtime. (I'll create a BZ enhancement request + a patch + integration test next week)
thanks again for your help
(In reply to comment #3) > Created attachment 206887 [details] > maven plugin to add the needed touchpoint to the tycho p2 runtime > Couple comments You don't need to define any goals in this plugin... unless, of course, you need to touch a timestamp file during your build. Since your plugin requires maven 3.0, it's better to explicitly state this as <prerequisites> in plugin's pom.xml and also use 3.0 version of maven-plugin-api. You MUST NOT use "hint = TychoP2RuntimeMetadata.HINT_FRAMEWORK". It is not possible to replace p2 runtime used by tycho, only to add additional bundles to it. Use of "hint = TychoP2RuntimeMetadata.HINT_FRAMEWORK" will result in unpredictable runtime behaviour. > Igor, > thank you for your answer. > Following your suggestion, I created a new maven plugin that I added to my > parent pom build section : > <build> > <plugins> > ... > <plugin> > <groupId>sample.plugin</groupId> > <artifactId>hello-maven-plugin</artifactId> > <version>1.0-SNAPSHOT</version> > <executions> > <execution> > <phase>compile</phase> > <goals> > <goal>touch</goal> > </goals> > </execution> > </executions> > </plugin> > </plugins> > </build> > You need to specify <extensions>true</extensions>, components defined in the plugin will be ignored without it. > I am pretty new to developing plexus components, so I am not sure I did the > right thing (see attached maven plugin); am I doing something wrong ? if so > could you please tell me what ? > > On another side, I also redefined the tycho-p2-runtime so that the equinox > runtime used by tycho is based on p2 : and I could build my project (first > zip). > I think this approach (make the tycho p2 runtime p2 aware) is the best approach > since it does not urge the user to create a maven plugin each time a new meta > requirement is needed in the tycho p2 runtime. (I'll create a BZ enhancement > request + a patch + integration test next week) > thanks again for your help I am not sure what you mean by "tycho p2 runtime p2 aware". In any case, bugzilla is not the right place for tycho development discussions, so please use tycho-dev mailing list if you have any further questions or comments. Created attachment 207061 [details]
makes tycho-p2-runtime p2 updatable
Created attachment 207063 [details]
makes tycho-p2-runtime p2 updatable Integration test
Hello, I provided 2 patches (the implementation of the solution and its integration test). Igor I used your advice to correct my maven plugin, but it is still not working (even though now I could see in debug that it was considered), the code is there : https://github.com/anthonydahanne/tycho-Bug-351487/tree/master/hello-maven-plugin But as I stated on tycho-dev (http://dev.eclipse.org/mhonarc/lists/tycho-dev/msg00344.html) , I do not think this is the right approach to solve this problem, the tycho p2 runtime should be able to provision itself to extend its capabilities (without saying that it would be additional work to create a maven plugin each time I want to execute a custom touchpoint action) Maven extensions plugins that contribute TychoP2RuntimeMetadata implementation is the only way to extend tycho p2 runtime we plan to support. This approach allows different project built with the same version of tycho contribute different additional bundles (or different versions of the same bundle) to tycho p2 runtime, which I believe will not be the case with "self-provisioning" approach. > This approach allows different project built with the same version of tycho > contribute different additional bundles (or different versions of the same > bundle) to tycho p2 runtime, which I believe will not be the case with "self- > provisioning" approach. that is exactly why I extended the mechanism to delete the runtime if a "pollution" (ie a touchpoint action got installed) of the runtime is detected. I do believe this approach is the most natural and easy to understand one. Could you please provide an example of a working maven plugin contributing TychoP2RuntimeMetadata ? or can you tell me why my attempt (https://github.com/anthonydahanne/tycho-Bug-351487/tree/master/hello-maven-plugin) fail ? thank you This is really an enhancement request, and therefore can't just be closed as "wontfix". The use case is valid, and just because there is one extension mechanism doesn't mean that there can't be a second one. (Tycho is an open source project after all...) @Anthony: There are a couple of other patches waiting, so it unfortunately still take a while until I get round to reviewing your patch. @Tobias : thank you very much for reopening this enhancement (I was afraid of patching tycho for my company for every release *sigh*) Whenever you start trying to merge it, feel free to ask me any questions! thanks again ! (In reply to comment #10) > This is really an enhancement request, and therefore can't just be closed as > "wontfix". The use case is valid, and just because there is one extension > mechanism doesn't mean that there can't be a second one. (Tycho is an open > source project after all...) > > @Anthony: There are a couple of other patches waiting, so it unfortunately > still take a while until I get round to reviewing your patch. Hmm, okay, here are the "quality patch" requirements 1. Changes to the tycho p2 runtime MUST be local to the project. In other words, absolutely no "leakage" of p2 runtime extensions between different builds. 2. Set of p2 runtime extensions used for a project MUST be predictable and stable. Specifically, set of p2 runtime extensions used for a module project MUST NOT depend on reactor build order. Ideally, the same set of extensions SHOULD be used when a module project is built standalone and as part of a reactor build. 3. Implementation MUST support running multiple builds using the same maven local repository in parallel. I am quite certain attached patch fails on requirement #3 and I am fairly certain getting all three right will require significant amount of code. In my opinion the underlying usecase does not justify the development and maintenance effort required to manage the added complexity, especially given other solution is already available. But, of course, this is my opinion and you are welcome to work on this enhancement request. > I am quite certain attached patch fails on requirement #3 and I am fairly > certain getting all three right will require significant amount of code. did you apply the patch to your local repo ? if so, could you experience this problem ? > In my opinion the underlying usecase does not justify the development and > maintenance effort required to manage the added complexity, especially given > other solution is already available. AFAIK, the given other solution does not have any working examples (and I could not make it work, if you can, then I'd be glad to know how you did). Could you make this other solution work for the examples set I provided ? https://github.com/anthonydahanne/tycho-Bug-351487/ There is another reason why anyway this solution (the maven plugin to extend TychoP2RuntimeMetadata) would not be satisfying (even if we could make the example work) : take the case of a plugin providing a custom touchpoint action, a plugin that would itself depend on other plugins. Correct me if I'm wrong, but with your solution, I would have to know in advance statically what are the needed dependencies, and I would have to set them manually in the maven plugin. The plugin dependencies change ? Then I'm stuck, I have to change the maven plugin... this is what statically defining the dependencies implies; and this is not acceptable. What's the problem with letting p2 deal with the p2 stuff ? (In reply to comment #13) > > I am quite certain attached patch fails on requirement #3 and I am fairly > > certain getting all three right will require significant amount of code. > > did you apply the patch to your local repo ? if so, could you experience this > problem ? > The same p2Directory is shared by all builds that use the same maven local repository. When two builds overlap in time, one build can remove p2Directory, while the other is running. Likewise, if two builds try to simultaniosly install different additional bundles the result is pretty much unpredictable... not to mention that p2 was not very good at serializing access to a profile from multiple processes last time I checked. (In reply to comment #12) > 1. Changes to the tycho p2 runtime MUST be local to the project. In other words, > absolutely no "leakage" of p2 runtime extensions between different builds. > 2. Set of p2 runtime extensions used for a project MUST be predictable and > stable. Specifically, set of p2 runtime extensions used for a module project > MUST NOT depend on reactor build order. Ideally, the same set of extensions > SHOULD be used when a module project is built standalone and as part of a > reactor build. > 3. Implementation MUST support running multiple builds using the same maven > local repository in parallel. The solution to all these requirements is to create a p2 director installation in the target folder of the project calling "materialize-product", and to use that director to install the actual product to be built. Then, it doesn't matter if the director installation is changed through meta-requirements. The director installation in the target folder can be created with the director in the tycho p2 runtime. We would just need a source p2 repository for that operation - I guess just building an extra zipped p2 repository for this in the build of Tycho, and unzipping that repository in the local Maven repository when running Tycho could work. (In reply to comment #15) Agreed, having a tycho-p2-runtime local to the project materializing a product is the solution to not deal with pollution of the base tycho-p2-runtime. The patches proposed still make sense for creating a tycho-p2-runtime p2 updatable + for the integration tests. Now what is missing is the creation of a local tycho-p2-runtime in the target folder of a project calling materialize-product. I'm not sure to understannd why we need an extra p2 repository Tobias, since if the runtime running p2 director can access the target platform definition + the reactor's projects it has everything it needs ? DirectorMojo.java : RepositoryReferences sources = RepositoryReferenceTool.getVisibleRepositories(getProject(), getSession(), flags); Isn't it already enough ? Then, just a matter of creating a new runtime + wrap it final DirectorApplicationWrapper director = new LocalDirectorApplicationWrapperImpl(); and we're good to go ? (In reply to comment #16) > The patches proposed still make sense for creating a tycho-p2-runtime p2 > updatable + for the integration tests. Why would we want this? AFAIK, the current tycho-p2-runtime prevents p2 updates through missing p2 metadata, and therefore is safe from metaRequirements. > Now what is missing is the creation of a local tycho-p2-runtime in the target > folder of a project calling materialize-product. Yes, except that I would call this a "p2 director installation" > I'm not sure to understannd why we need an extra p2 repository Tobias, since if > the runtime running p2 director can access the target platform definition + the > reactor's projects it has everything it needs ? The target platform doesn't need to include p2. Plus we don't want different versions of the director to be running but always the one that matches the p2 version embedded in Tycho. (In reply to comment #17) > Why would we want this? AFAIK, the current tycho-p2-runtime prevents p2 updates > through missing p2 metadata, and therefore is safe from metaRequirements. OK, then we need a new product definition (having the simple configurator activated) defined somewhere else... However, the integration tests can be kept (I still think) > The target platform doesn't need to include p2. Plus we don't want different > versions of the director to be running but always the one that matches the p2 > version embedded in Tycho. I'm confused a bit. According to you, what are the (high level) steps to create (in the target folder) and run this "p2 director installation" when the goal materialize-product is executed ? thank you ! Okay, then let me summarize again: 1. In the Tycho build, create a p2 repository with the content needed to install a director application. AFAIK, the features org.eclipse.equinox.p2.core.feature and the org.eclipse.rcp should contain at least most of what we need. This new eclipse-repository module should use the tycho-bundles POM as parent so that it inherits the right target platform configuration. 2. When running materialize-product, 2a. Tycho should first create a p2 director in target/director. This can be done with the current DirectorApplicationWrapper. As source repository, use a "jar:file:"-URL pointing to the artifact from 1 in the local maven repository. Add a POM dependency from tycho-p2-director-plugin to the artifact from 1 to make sure that Maven downloads it automatically. 2b. Launch the p2 director in target/director to install the thing to be built. For launching, you don't need an eclipse.exe - a java call with the launcher on the class-path also works. Then, meta-requirements should work. Beware of Tycho embedded runtime synchronization work done as part of bug 365012. (In reply to comment #20) > Beware of Tycho embedded runtime synchronization work done as part of bug > 365012. I don't see any side-effects of this: The embedded OSGi runtime would only be used in step 2a, and that step is no different to what is being done with the director today. The modifiable director application will reside in target/director (In reply to comment #19) well, unfortunately, it is not as easy as the steps you described Tobias... I created a product (attached) built with a eclipse-repository packaging, and I used, as you described, the DirectorMojo to unzip in target/director this "p2 updatable" equinox runtime, using -application o.e.e.p2.director to launch it. At first, I did not notice that in tycho, you are providing IArtifactRepository to handle special artifact repositories : file:/resolution-context-artifacts@C:%5Cworkspaces%5Cproduct,file:/C:/m2-repo/ Once I understood that my equinox "p2 updatable" runtime needed org.eclipse.tycho.p2.resolver.impl , most particularly the OSGi services it provides (LocalRepositoryP2Indices for example) , I added it in my product. Well, now I'm stuck since LocalRepositoryP2Indices depends on other OSGi services such as FileLockService (see https://git.eclipse.org/c/tycho/org.eclipse.tycho.git/tree/tycho-bundles/org.eclipse.tycho.p2.resolver.impl/OSGI-INF/localRepoP2Indices.xml) that apparently never published on the OSGi registry ... I understand that, today, since DirectorMojo uses directly the DirectorApplication, some kind of maven/equinox magic can happen to inject the services... But do you still think it is possible to create a runtime with all the needed services to consider special artifact repositories (file:/resolution-context-artifacts@C:%5Cworkspaces%5Cproduct,file:/C:/m2-repo/) ? thanks Created attachment 208474 [details]
tycho-bundles-external-dynamic : p2 updatable equinox runtime product definition
(In reply to comment #22) > At first, I did not notice that in tycho, you are providing IArtifactRepository > to handle special artifact repositories : > file:/resolution-context-artifacts@C:%5Cworkspaces%5Cproduct,file:/C:/m2-repo/ > Once I understood that my equinox "p2 updatable" runtime needed > org.eclipse.tycho.p2.resolver.impl , most particularly the OSGi services it > provides (LocalRepositoryP2Indices for example) , I added it in my product. Sorry for not seeing this before: It won't be possible to call the standalone director installation in the same way as the director in Tycho's own OSGi runtime. The reasons are (as you already found out): 1. The built-in version reads from non-standard p2 repository locations, like the local Maven repository. The implementation for this (currently) doesn't work outside of Tycho's OSGi runtime because they rely on services coming from Maven. 2. The built-in version reads p2 repositories only stored in memory (from Tycho's target platform computation/dependency resolution). These will never be accessible in a separate process. However launching the p2 director in a separate process is what protects Tycho's OSGi runtime from side-effects, so this is a blocker. The only solution I see from this is to install from the build result of eclipse-repository (target/repository) instead of the target platform, for this particular use case. The normal materialize-product goal still needs to install from the target platform, but we could add a separate goal which installs from the eclipse-repository build result, and that implementation can use the standalone director which supports metaRequirements. Sorry for all the detours - I am impressed that you still don't give up :-) If this helps we could also do a phone conference with screen sharing to talk about the details. If you want to do this, just propose it on tycho-dev. (In reply to comment #24) > The only solution I see from this is to install from the build result of > eclipse-repository (target/repository) instead of the target platform, for this > particular use case. The normal materialize-product goal still needs to install > from the target platform, but we could add a separate goal which installs from > the eclipse-repository build result, and that implementation can use the > standalone director which supports metaRequirements. Quite interestingly, this is what I emplemented before leaving for vacations : a new goal (matareialize-product-metarequirements, that copies in target/director a p2 runtime updatable,then launches it, and use the .target locations (using the tp api) + target/repository repo : I made my old example work with 0.14.0-SNAPSHOT and this patch > Sorry for all the detours - I am impressed that you still don't give up :-) never let tycho down : this is our moto ;-) but also business requirements for our products make us hold it strong ;-) > If this helps we could also do a phone conference with screen sharing to talk > about the details. If you want to do this, just propose it on tycho-dev. Excellent, I'd like to, the week of the January 3rd I 'll be able to send the patch and demo it to you. Created attachment 209113 [details]
refactoring to expose common methods for director mojos
as discussed during the webex demo, a first patch with a refactoring of the director mojo to expose common methods (getArgsForDirectorCall and toCommaSeparatedList)
Created attachment 209114 [details]
providing a new goal, materialize-products-metarequirements allowing the user to materialize a product depending on metarequirements (such as custom touchpoint actions)
as discussed during the webex demo, the implementation of a new dynamic director product and a mojo to launch it (DynamicDirectorMojo)
Notes : I have been able to change the code so that the product is generated in target/director when needed (so no troubles with the target platform environments); unfortunately I could not find a way to use the includeAllDependencies of the tycho repository plugin; using repositoryReferenceTool.getVisibleRepositories simply won't work to define metadata and artficat repositories for this director product (it is not aware of special tycho p2 repositories layout)
Created attachment 209141 [details]
providing a new goal, materialize-products-metarequirements allowing the user to materialize a product depending on metarequirements (such as custom touchpoint actions)
just found out a tiny bug in the previous patch; forget to mention that the dependency
<dependency>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-bundles-external-dynamic</artifactId>
<version>${project.version}</version>
<type>zip</type>
</dependency>
was a zip type
(In reply to comment #27) > unfortunately I could not find a way to use the > includeAllDependencies of the tycho repository plugin; using > repositoryReferenceTool.getVisibleRepositories simply won't work to define > metadata and artficat repositories for this director product (it is not aware of > special tycho p2 repositories layout) I thought we already clarified this: this won't work. The proposal was install exclusively from target/repository. When includeAllDependencies is set to true, this should work. Could you please also upload the proposed changes to github? There are a couple of small things that still need to be changed, e.g. so that the contribution can pass the legal review, and discussing them here without the code is a bit tedious. (In reply to comment #29) > I thought we already clarified this: this won't work. The proposal was install > exclusively from target/repository. When includeAllDependencies is set to true, > this should work. yes, we did talk about this; and we did agree it was the right solution instead of re parsing the tp... So I gave it another try today and it worked, I guess my brain was shut down last time I tried... > Could you please also upload the proposed changes to github? There are a couple > of small things that still need to be changed, e.g. so that the contribution > can pass the legal review, and discussing them here without the code is a bit > tedious. here it is : https://github.com/anthonydahanne/org.eclipse.tycho/commits/master/ So, in a nutshell : c9ebff3a03db005a3c334398db008d1f8745b51b : refactoring 14c8d19c6a8cc63f0b61b52cc90bdd4cbeaa3fdb : first attempt using target/repository + parsing the project target platform 2a1df9799599e41bdde15198ba0fc8ed3c209d7e : second attempt only using target/repository, provided the product to build specifies the includeAllDependencies option in its pom thanks for reviewing those commits. I just added a new commit to support different envs. so the previous commits are still good + you have to add this one : https://github.com/anthonydahanne/org.eclipse.tycho/commit/aa709a4c420bd9c9e28049842a4bf6b1c16057d3 I also synced with master, no problems merging. should be ready for prime time now ;-) I'm running into the same problem with Tycho 0.13 so I'd like to ask if there is any outlook on which version of Tycho might be fixing the issue. (In reply to comment #32) > I'm running into the same problem with Tycho 0.13 so I'd like to ask if there > is any outlook on which version of Tycho might be fixing the issue. Sorry, this won't make it into 0.15.0, but I'm planning to integrate it in the following release. Created attachment 216043 [details]
merged patch
Applied both previous patches to master (0.15.0) and created one patch
Created attachment 216074 [details]
merged patch
needed to update previous patch
I went ahead and posted the commit[1] as well as another commit[2] which implements Bug 361722 on top of these changes the commits are: 1. https://github.com/JonHubbard/tycho/commit/d1d4abd9f5c22808830b4c2b19fd810e4ec382f4 2. https://github.com/JonHubbard/tycho/commit/bcbf0c7362a06940196158c52c6b5dee504583a3 As a preparation step, I have refactored [1] the existing DirectorMojo and introduced the new interface DirectorRuntime, which encapsulates the director application calls. Next, I have used that new interface in order to implement a standalone, p2-updatable director runtime, which supports meta-requirements. This change is currently proposed in Gerrit [2]. The change also includes a significantly more concise integration test for the new functionality, but I also checked the originally proposed test setup and that also works (with minor modifications due to different goal¶meter names). [1] http://git.eclipse.org/c/tycho/org.eclipse.tycho.git/commit/?id=4f0b07ef69ba99fc8a35d8234133820c62635822 [2] https://git.eclipse.org/r/#/c/6808/1 This is finally implemented :-) There is an integration test [1] that shows how to build a product which includes a custom touchpoint action. Not entirely trivial, but I don't expect everyone to have custom touchpoints anyway. After the 0.16.0 release, there also will be documentation for the new switches in the tycho-p2-director-plugin on Tycho's site [2]. Sorry, nothing to see now, we don't have a site CI build. [1] http://git.eclipse.org/c/tycho/org.eclipse.tycho.git/tree/tycho-its/projects/product.metaRequirements [2] http://www.eclipse.org/tycho/sitedocs/index.html Thanks a lot Tobias ! I followed instructions form [1] and was able to create custom touchpoint and execute an action on install using standalone p2 director. Thanks, Tobias! However I bumped into timeout exception: [ERROR] Failed to execute goal org.eclipse.tycho:tycho-p2-director-plugin:0.18.1:materialize-products (materialize-products) on project studio-product: Execution materialize-products of goal org.eclipse.tycho:tycho-p2-director-plugin:0.18.1:materialize-products failed: org.codehaus.plexus.util.cli.CommandLineTimeOutException: Error while executing external command, process killed. Process timeout out after 60 seconds Any ideas how to increase this 60 seconds timeout? I followed instructions from [1] and was able to create custom touchpoint and execute an action on install using standalone p2 director. Thanks, Tobias! However I bumped into timeout exception: [ERROR] Failed to execute goal org.eclipse.tycho:tycho-p2-director-plugin:0.18.1:materialize-products (materialize-products) on project studio-product: Execution materialize-products of goal org.eclipse.tycho:tycho-p2-director-plugin:0.18.1:materialize-products failed: org.codehaus.plexus.util.cli.CommandLineTimeOutException: Error while executing external command, process killed. Process timeout out after 60 seconds Any ideas how to increase this 60 seconds timeout? (In reply to comment #40) > Any ideas how to increase this 60 seconds timeout? The 60s timeout is currently hard-coded in /tycho-p2-director-plugin/src/main/java/org/eclipse/tycho/plugins/p2/director/runtime/StandaloneDirectorRuntime.java, line 56 Open a new bug if you need this configurable. |