Community
Participate
Working Groups
Given a repository containing IUs for all plugins/features you are interested in, we want to be able to generate a product IU that installs to a complete product. Current product generation in the metadata generator relies on pde.build to assemble config information and call the generator in multiple steps to get everything correct. We want the publisher to do all this itself based on the .product file, in one short instead of across multiple calls. 1) Generate start level information 2) Generate other config.ini options based on the .product file and any template config.ini files that may be referenced by the .product file. (See bug 252426) (in particular, osgi.splashPath, eclipse.application, eclipse.product as done in pde.build's ProductGenerator#createConfigIni) 3) Generate requirements on appropriate launcher pieces. Generate an artifact containing a branded executable. (See also bug 259763)
See bug 249406. For branding the executable, we should be able to take the org.eclipse.equinox.executable's artifacts and use them to create product specific branded equivalents. Note however, in the absence of the executable feature, we need a mechanism to specify the location of the launcher being included in the product so that we can brand it.
We also want an ant task to invoke this. I believe it warrants its own task instead of having one main publisher task with a multitude of options. perhaps named p2.publish.product, it takes the .product file and perhaps a couple of other options as necessary (the location of a launcher? comment #1)
Created attachment 121658 [details] Initial whack at some of the issues Attaching an inital sketch of using the publisher for this use case. The patch is a little larger as it is also fixing some issues related to multiple versions of things being spec'd in a .product file. Only very rudimentary testing has been done. Some outstanding issues. - The config.ini location in the .product file is ambiguous. From the product file user's point of view it is either relative (to the location of the .product file) or is absolute. In PDE UI it should be absolute or some workspace location. We must have code somewhere that properly deals with this. - if you are publishing a plugin-based product then you have to publish on a per platform basis because we have no way of knowing the filter related to the platform-specific bits. So the product IU has to have a full dependency on all the bits listed regardless of whether or not they are relevant for the platform. The alternative is to use feature-based product or merge feature-like function into the product definition. - need to figure out the iu naming for the product (should it have some platform specific bits?) - optionality might make it better but then you could end up with something that does not work
See also bug 259158 (I'm not sure if this work fixes that problem or not, but it seems like it should?)
(In reply to comment #3) > Attaching an inital sketch of using the publisher for this use case. The patch > is a little larger as it is also fixing some issues related to multiple > versions of things being spec'd in a .product file. We should support a version attribute on plugins and features in the .product file. (A brief look at your patch suggests this is what you're doing). UI appears to already write a version for features. I'm working one having pde/build replace/add versions in the .product file at build time in the same way we replace 0.0.0 in features. This is similar to replacing .qualifier in the product version. See bug 246060 > - The config.ini location in the .product file is ambiguous. From the product > file user's point of view it is either relative (to the location of the > .product file) or is absolute. In PDE UI it should be absolute or some > workspace location. We must have code somewhere that properly deals with this. I can try having pde/build replace this at build time with an absolute path when it does the versions. > > - if you are publishing a plugin-based product then you have to publish on a > per platform basis because we have no way of knowing the filter related to the > platform-specific bits. So the product IU has to have a full dependency on all > the bits listed regardless of whether or not they are relevant for the > platform. The alternative is to use feature-based product or merge > feature-like function into the product definition. I think we need multi platform support. If we don't add the functionality to .product, we should accept advice which could take the form of a feature.xml. (PDE/Build already has a feature.xml it generated from the .product file where it guessed platform filters with the help of an osgi state.)
(In reply to comment #5) > (In reply to comment #3) > > Attaching an inital sketch of using the publisher for this use case. The patch > > is a little larger as it is also fixing some issues related to multiple > > versions of things being spec'd in a .product file. > > We should support a version attribute on plugins and features in the .product > file. (A brief look at your patch suggests this is what you're doing). UI > appears to already write a version for features. > > I'm working one having pde/build replace/add versions in the .product file at > build time in the same way we replace 0.0.0 in features. This is similar to > replacing .qualifier in the product version. Note that the metadata generator supported this via a .properties advice file. (See metadata.generator.ProductQuery's versionAdvice).
Andrew and I spent quite some time today going over this stuff. Here is a summary: 1) For the plugin based product we propose that there be an IFilterAdvice that, given an IU and version (optionally) will return the platform filter (if any) to use. Using this we can implement any number of advisors. for example, one that looked in the result, in any known repos, ... to see if we already know the desired IU and then extract the filter and return it. Action: Do it. see bug 260194 2) For branding the executables the current support takes the location of existing executables and copies and brand those. this location can either be that of the ExecutablesFeature or that of the parent of an existing .exe (e.g., the location of a current install). The proposal in the discussion was to add a third mechanism -- looking up the relevant executables IU in some set of repos and, if found, downloading the related artifacts, unzipping them and then branding as described. Note that we also talked about making the deltapack include a repo for at least the executables feature. <metastatement> The fundamental issue here is that feature root files do not have a "built state". They are included in the dev time features and then the build takes them and in the process of assembling stuff extract the relavant files and lays them down. in the new world we want p2 to do the last step. So having the required artifacts/IUs in repos is goodness. We should consider doing this but unless we can ensure that those artifacts are in the target or otherwise downloaded, we have not helped here. </metastatement> Action: On reviewing this it feels way too specific to embed in the publisher itself. Some application or task outside the publisher can ensure that the executables feature content is available for branding and then supply that location to the ProductAction. There are way too many ways of getting and maintaining that content. I fear that by embedding it in the action we are asking for trouble. 3) Generally speaking a few of these approaches and other bits require the various publishing actions to look in "input repositories". We want to separate keep separate the output of the publishing from any inputs. Right now the publisher knows only about one repo, an output repo. More often than not it starts out empty. In the cases where you are publishing bits in isolation of the complete picture, you may need more context. Something that some existing repos could supply. Action: Add a means of spec'ing input artifact and metadata repos. Need to look at how to talk about and position them but otherwise these should just be setup on the Info. See bug 260195
I just wanted to mention that I spent some time during our last test writing day adding some automated tests for production publication in ProductActionTest. There is a test where config data is specified in the product file, another test that publishes the platform product, and a third test for publishing a branded product. For the branded test, I did the leg work of creating test data (testData/ProductActionTest/brandedProduct), but couldn't seem to make the implementation work. Just mentioning it because if someone is working in this area it should be easy to take this initial shell of a test and make something useful out of it.
Thanks John. I did see that and will try to ensure it works. The code for doing the work seemed to all be there but somehow was not getting invoked. Also, an update on comment 7 point 2. Andrew and I chatted briefly and came to a consensus that there were many different ways that the executables could come to be available for branding. As such, having the executable materialization code be outside the *Action classes would likely be a good thing(tm).
Created attachment 122616 [details] Changes for most of the issues discussed This patch should address pretty much everything. - branding the executables - parsing the new product file data - publishing just a product file (assuming the correct supporting input) - we even handle multiple versions of plugins/features in product files - convert a mess of methods on AbstractPublisherAction to be instance not static - add the ability to pass in metadata and artifact repos for the publisher to use - define FilterAdvice that helps IUs depending on other IUs know the valid platforms for the prereqs
That patch, while likely quite complete, is still initial. I have not included tests. will work on those next.
*** Bug 260194 has been marked as a duplicate of this bug. ***
*** Bug 260195 has been marked as a duplicate of this bug. ***
Created attachment 122843 [details] new patch to include composite repo use This new patch includes the use of Composite*Repository in the AbstractPublisherApplication
Created attachment 124113 [details] Updated Patch Here is an updated patch. This includes: - <properties> in .product files - Makes repofilteradvice, QueryableFilterAdvice - Makes publisher results queryable - Fixes up the naming around ILaunchingAdvice (now it is IExecutableAdvice) - Sets the splash screen as a property - Adds a bunch of test cases for ProductFile and ProductFileAdvice Chris and I are going review the change to the .product file tomorrow.
Question for Jeff or Andrew In the .product file, a splash screen is specified by providing the name of the bundle the splash screen can be found in someBundle. This gets placed in the ini file as follows: osgi.splashPath=platform:/base/plugins/someBundle The question is, who should provide platform:/base/plugins Should this be done when publishing 1. By p2 (we can just assume this path) 2. By PDE build (through some sort of advice) or 3. During the install operation
The splashpath is a tricky problem, I'm not quite sure what the proper IU setup should be. I do think it should be handled in p2 with the ProductAdvice. This is related to the -showsplash argument in the eclipse.ini file. See bug 222260.
Andrew, What's the difference between the showSplash argument and the osgi.splashPath. It seems that PDE export only sets the osgi.splashPath (at least for the little example I created). Also, do you mean we should create some sort of ProductAdvice? Currently we have ProductFileAdvice, but this just gets advice from the product file, and this obviously doesn't know what to do with this.
I have managed to use this patch (with a few fixes) to publish and provision an Eclipse SDK product. There are three tweaks I had to perform (so three things left to do). 1. Updated the config.ini file (currently we don't set all the options here. This may be tricky especially with shared installs, and we have to find a way to "advise" the publisher on which version of things (such as the simple configurator) should be used. 2. Updated the eclipse.ini file. Again, we need to find a way to bind to right version of the launcher, keeping in mind it might be in bundle pool too. 3. Create / provision / generate the bundles.info file. This is not being generated.
(In reply to comment #19) > 3. Create / provision / generate the bundles.info file. This is not being > generated. > It is not up to the publisher to do anything for this. The publisher should only be generating CUs with appropriate actions to set start levels and such. It is the director at install time that creates the bundles.info and I assume it only does this if the product you are installing includes the simpleconfigurator, otherwise it probably sticks everying on the osgi.bundles list in config.ini
(In reply to comment #20) > > It is not up to the publisher to do anything for this. The publisher should > only be generating CUs with appropriate actions to set start levels and such. > It is the director at install time that creates the bundles.info and I assume > it only does this if the product you are installing includes the > simpleconfigurator, otherwise it probably sticks everying on the osgi.bundles > list in config.ini > Perfect! This is what I was hoping to hear :-). I am going to see what I'm doing wrong. Likely just not setting the the start level correctly.
I found the problem. In the CUsAction, we are ignoring the osgi.bundles property. This is where I set: osgi.bundles=reference\:file\:org.eclipse.equinox.simpleconfigurator_1.0.100.v20090112.jar@1\:start Is there a reason this is being ignored? Does the osgi.bundles get set somewhere else?
I don't think we should be setting the osgi.bundles property, that is managed at install time by framework admin. If your product includes simpleconfigurator, but the final installed result doesn't use a bundles.info, then maybe you are missing a CU for org.eclipse.simpleconfigurator. The metadata generator had the notion of default IUs, see Generator.generateConfigIUs and EclipseInstallGeneratorInfoProvider.getDefaultIUs. One of these default IUs was to set simpleconfigurator as started with a start level of 1. Note that when doing this kind of default CU we need to be careful we don't create multiple CUs for the same IU, see bug 252284 and bug 263087
While I haven't had time yet to review the patch here, a conversation with Ian on irc indicates that the patch does not generate default start level information when none is provided. So here are my thoughts on this. We can't make the user do it. Most people don't know how to configure a product, and even those who do would prefer to have it done for them unless they had special requirements. (1) It would be Good to be able to headlessly publish a product without having to get pde.build involved. (For some reason, not everybody loves build). One of the reasons for this bug is to make it easy to compose products from a pre-existing repo of bundles. I see 3 options (A) the Publisher generates the defaults itself (yay!) (B) require the .product file to specify everything, (boo!) (C) accept another form of advice with the defaults. (B) essentially means have the editor (PDE/UI) write everything in the .product file. My reasons against this are: a) I just don't like it, I think it is better to have the .product file just contain special cases. b) This would be a regression, for backwards compatibility Build must continue to work with .product files that don't have start level information. Build will retain the ability to generate defaults so that it can support existing build setups (with and without p2). I think this is good argument against (B). (Also UI pushed its config.ini generation down to build, it would be mean to ask them to put it back). This makes Build the source of the advice for (C). I think the choice between (A) and (C) comes down to the importance of (1). If it is acceptable to require a PDE/Build setup to publish products then we have A (yay!) otherwise we have C.
(In reply to comment #24) Thanks for posting this Andrew, you summed up the problem nicely. Please don't review this patch yet, I will be updating it once I can completely publish and provision a working system. I completely agree with number (1). Not that I don't like PDE build :-). However, I worry about (A). What if I don't want the default start levels, how do I get rid of them? Why are some bundles "special"? I would rather see PDE tooling that can generate (set defaults) for common products. This way users know exactly what's going on. This can likely be achieved with some sort of template. A template could be applied, for example, if somebody wanted to create a product based on SimpleConfigurator. In this case it would add the necessary properties and start levels for things like the simpleconfigurator and o.e.core.runtime. This may be a regression (backwards compatibility), I don't know. Did the generator add "default" or "special case" CUs?
+1 for #1. Encoding start levels etc in the publisher is a "bad thing" (tm). It forces us to assume things that while they may be true 90% of the time, they are not universal truths. If they were we would just embed the start info in the p2.inf file. DS was an example of this. We could not imagine a case where someone wanted DS and did not need it started and at level 1. This was very unique to DS and its function and implementation. SimpleConfigurator is not the same. For example it IS reasonable to have in a configuration but not have it be started (e.g., I am running a non p2 based p2 Agent). So A) is not an option in my mind. B and C are possible. For C it might have to be factored a little but the ConfigCUsAction already takes IConfigAdvice which allows people to set the start level etc for bundles. The product file is just one form of this advice. The contents of the config.ini are parsed in some cases and stored in ConfigAdvice objects which are another source.
The generator currently currently adds default special case IUs for org.eclipse.equinox.simpleconfigurator org.eclipse.equinox.launcher org.eclipse.equinox.launcher.* org.eclipse.equinox.p2.reconciler.dropins I raised bug 263783 to think about adding p2.inf to the launchers, this would remove that special case. That would also make it hard to include these bundles in your product but not actually use them to start, don't know if this is a usecase for anyone. I don't think the reconciler.dropins should have its own p2.inf, that one feels more like a product level decision. Build is special casing, depending on the setup (which doesn't necessarily involve p2): org.eclipse.update.configurator org.eclipse.equinox.simpleconfigurator org.eclipse.equinox.common org.eclipse.osgi org.eclipse.core.runtime org.eclipse.equinox.ds
Created attachment 125098 [details] updated patch Fixes a problem that Jeff and I found where the roots were not getting properly merged.
Created attachment 125197 [details] Updated patch This patch refactors the product action slightly. Now, instead of taking a location to a file, it takes an IProductDescriptor. This can either be the ProductParser itself, or a spoofed up product file.
Ian, I am going to go over this today and hopefully release it, we can then use small patches on more specific bugs for any remaining issues.
(In reply to comment #30) > Ian, I am going to go over this today and hopefully release it, we can then use > small patches on more specific bugs for any remaining issues. > Great! I'll jump on IRC if you need to ping me.
I was about to release these changes, but the EquinoxExecutableActionTests are all failing. I don't know enough about easy mock to say what is wrong. Ian, do you know anything about this?
I forgot about these. These 4 tests have been failing since I first grabbed the publisher patch. I don't know much about these, but I will take a stab and fixing the problem.
Created attachment 125359 [details] updated patch -- fixes broken test cases I managed to learn enough about EasyMock to fix this :). Turns out we have to seed the easy mock objects with information on what to do when certain methods are called. In this case, we needed to tell the proxies what to do when getAdvice() was called.
I have released these changes. I will close this and we can use new bugs for any more changes we need. Thanks for your help Ian.
