Community
Participate
Working Groups
When running the director, the source metadata and artifact repositories used end up saved in the resulting install. They will be saved under the profile the director is running in (which may be difference from the profile we are installing) These repos will generally be a build time repo that will not be available for end users. See also bug 268136
I was thinking we could support a value like eclipse.p2.profile=@none, which we would use when we don't want any "self" profile. The profile preferences would never be written to disk in this case. The director app (and possibly others) could set this property on startup.
One problem is that in many cases some properties are already picked up by started services even before the director app start() method is reached. It all depends on how the director was started and what the OSGi configuration looks like. If we add the property per John's suggestion, who will set that property and when? To be completely sure to have control over all properties (such as eclipse.p2.data.area and eclipse.p2.profile) without passing them as vmargs, I have been forced add code like this to the application start(): PackageAdmin packageAdmin = <obtain package admin>; stopTransient(packageAdmin, BUNDLE_P2_EXEMPLARY_SETUP); stopTransient(packageAdmin, BUNDLE_P2_CORE); String p2DataArea = new File(buildRoot, "p2").toString(); System.setProperty(PROP_P2_DATA_AREA, <new data area>); System.setProperty(PROP_P2_PROFILE, <new profile id>); if (!startEarly(packageAdmin, BUNDLE_P2_CORE)) <throw missing bundle exception>; if (!startEarly(packageAdmin, BUNDLE_P2_EXEMPLARY_SETUP)) <throw missing bundle exception>; This has been successful in the places where I've been using it so far since all relevant services are restarted.
A related issue that will be fixed with the approach mentioned is that the eclipse.p2.data.area can be set equal to the -destination argument. Today, I think you must specify both on the command line. I'm working on a patch for this that will start by parsing the command line options, then stop + restart the needed bundles. Since this will make p2 "forget" its self profile and instead use the designated one (I can set eclipse.p2.profile equal to the -profile argument), the issue is resolved. Does that sound OK?
Created attachment 132516 [details] Work in progress, intended for preview only. This patch implements the scheme that I mentioned in the comments. Disclaimer: It should be fully functional but it breaks all director app tests since they incorrectly pass -vmargs directly in the application context. That clashes with the improved command line parsing also included in this patch.
Build's problem is the self profile of the director contaminating the install we are creating. Build forks a new java process to run the director, it points this process at the same configuration area of the eclipse that ran the build, this generally means the director's self profile is the profile from the eclipse running the build. In the case of export, this is the SDKProfile and results in that profile contaminating the results. Build can instead set eclipse.p2.profile as a vm argument, we tried setting it to the same profile we were installing and got contaminated results (repository.prefs). Working with Pascal today, we tried setting eclipse.p2.profile to a random profile that does not exist (@none works), and in this case we see no contamination. Perhaps no changes are necessary here to make build happy.
Even if the initial objective for this bug was to make the PDE build happy, I think we really need the changes proposed in my patch. The current way of invoking the director is far from user-friendly. We can't expect the end user to keep track of internal P2 properties and pass them on the command line. Please try the patch with the PDE build. I suspect it will solve the problem once and for all. I also suspect that the PDE build no longer need to pass system properties to the director. Passing -destination should be enough.
Andrew and I found a workaround for the problem in bug #268136 by simply invoking the director overwriting the eclipse.p2.profile property on the command line when it invokes the director. However I think we will still need this patch but in the current form I get in an even worst situation because I end up with a p2 folder as expected but the other p2 subfolders are located in the install folder (as sibling of the p2 folder). Also given the criticality of this application and the very few tests we have for it we have to be extremely careful on the set of changes we are doing. The few things that come to mind are usage of paths with "../" in them, cross platform export and the shape of paths being created and of course we still have to guarantee that all the scenarios that were documented at http://help.eclipse.org/ganymede/index.jsp?topic=/org.eclipse.platform.doc.isv/guide/p2_director.html In fact I think that we would all be more comfortable with this wide of a change and were adding tests shortly after providing the patch for this. For future reference I was testing with the following command. -application org.eclipse.equinox.p2.director.app.application -metadataRepository http://fullmoon.ottawa.ibm.com/eclipse/updates/3.5-I-builds -artifactRepository http://fullmoon.ottawa.ibm.com/eclipse/updates/3.5-I-builds -installIU org.eclipse.sdk.ide -destination /Users/Pascal/tmp/sdk -profile MySDK profileProperties org.eclipse.update.install.features=true -bundlepool /Users/Pascal/tmp/sdk -p2.os macosx -p2.ws cocoa -p2.arch x86 -roaming -vmargs -Declipse.p2.data.area=/Users/Pascal/tmp/sdk/p2
Created attachment 133079 [details] P2 data area now under configuration The engine is now included among the bundles that are restarted. This was needed to make ProfilePreferences store themselves correctly (the stop of the engine waits for pending SaveJob's and not waiting for that causes problems). The P2 data area for stand-alone installs is now under the configuration directory. I'm working on tests too and this bug gets somewhat entangled with bug 268194 and bug 273559. The latter can be commited right away I guess since it doesn't affect any old behavior. Is it OK to mix fixes for this bug and bug 268194 in one patch?
Why is restart needed? There shouldn't be any p2 bundles running when the application is first started. Is this only needed for cases like tests where we are running the director app within the same VM? In what other cases does this come up?
Also, most of the DirectorAppTests are failing when I run with this patch? Is there another patch for the tests that is not attached?
Never mind, tests are failing without the patch too.
I know that some tests are failing. That's because P2 creates a profile and the tests assumes that nothing at all is created. The tests succeeded earlier since the profile ended up in the wrong place so I think the tests are incorrect in this respect. If you agree, I'll provide a patch for that too.
Created attachment 133165 [details] Fix for tests Yes, you're right about the tests. Because we are not setting the agent location in the destination, the destination is not empty on failure, which is what the tests are asserting. This patch cleans up the agent location at the destination prior to the assertion checks. The correct assertion is "ensure there is nothing at the destination except the agent metadata".
Created attachment 133175 [details] Fix v02 Putting the p2 agent location under a folder called "configuration" rather than p2 seems like a big change that is likely to produce confusion and likely have compatibility problems. I think we should keep it in the same place as the Eclipse SDK here (sibling of configuration folder called "p2").
I haven't looked too closely at the patch. There are plenty of existing releng setups that will be calling the director and setting eclipse.p2.data.area as a vm arg to provision their products. This looks like it will break those releng setups that use a non-standard p2 area and this will require script changes for those people. I would suggest that if -shared is not specified on the command line, then the default is the existing value of the system property.
Another concern that came up in discussion with Pascal is whether we are stopping/starting enough bundles here. There are likely other p2 bundles that are not being restarted, and which might have bugs in their handling of dynamic addition/removal of things like the event bus service.
(In reply to comment #15) > I would suggest that if -shared is not specified on the command line, then the > default is the existing value of the system property. > There's a problem with that since there's no way of telling if the user provided a system property or not. Combine that with the fact that the eclipse.p2.data.area is very likely to appoint the wrong (the callers) data area in case the user doesn't set it. This is not in any way connected to use of -share. I think that a stand-alone install using a -destination will be a very common case and that it would be very unfortunate if we lock ourselves in to an approach where the user has to set properties in order to make things work. We need to get out of that a.s.a.p. IMO, it's better to break existing non-standard stand-alone installs now. I doubt it will cause that much of a turbulence if presented the right way. I.e. stop using obscure system property settings. The director will do the right thing anyway. Another approach would be to leave the director exactly as it stands today (no more improvements) and instead provide a new application with all the bells and whistles. That would vouch for not breaking backward compatibility while at the same time give us a way forward where we can have a more elaborate set of options.
When I first looked at the patch I had forgotten about the case that Andrew is mentioning, which is that basically people use to the p2.data.area to create various scenarios of shared installs and what not. I agree also with Thomas that this needs to get in soon (M7), so I think that the best way to move forward is to create another application because we have already burnt people before and the impact of changing something so fundamental in a non compatible way could be disastreous.
>There's a problem with that since there's no way of telling if the user >provided a system property or not. Actually I think we can. The system property "eclipse.vmargs" is set to the vm arguments that were passed in by the launcher. So if we honor any eclipse.p2.data.area value specified on the command line, but otherwise set it to be the destination, we should be able to keep old clients happy while providing a simpler path for new clients.
(In reply to comment #19) > Actually I think we can. The system property "eclipse.vmargs" is set to the vm > arguments that were passed in by the launcher. > Cool! I didn't know that. I'll put together a patch that is based no my earlier work + Johns patches where I check the content of the eclipse.vmargs property so that the properties set there are retained.
I believe that the eclipse.vmargs is only available if the exe is being used, as such it will be of very little help for most of the scenarios involving releng style usage of the application since we are directly using java.
(In reply to comment #21) > I believe that the eclipse.vmargs is only available if the exe is being used, > as such it will be of very little help for most of the scenarios involving > releng style usage of the application since we are directly using java. > The eclipse.vmargs is avaliable even when the exe is bypassed. I tried this: [thhal@tada eclipse]$ java -jar plugins/org.eclipse.equinox.launcher_1.0.200.v20090413.jar -console -noexit -vmargs -Dfoo=bar osgi> getprop eclipse.vmargs eclipse.vmargs=-Dfoo=bar Or did you mean when executing the p2.director task from Ant?
Hmm, running using a Launch Configuration in the IDE to run the app will not set the eclipse.vmargs. Should that perhaps be considered a bug?
(In reply to comment #22) > The eclipse.vmargs is avaliable even when the exe is bypassed. I tried this: > > [thhal@tada eclipse]$ java -jar > plugins/org.eclipse.equinox.launcher_1.0.200.v20090413.jar -console -noexit > -vmargs -Dfoo=bar > > osgi> getprop eclipse.vmargs > eclipse.vmargs=-Dfoo=bar > This command line did not actually set any vm arguments -vmargs is interpreted by the native launcher to set the vm arguments on the vm and by Main to set the system property. In the case of launching with java, adding -vmargs caused Main to set the sytem property, but they were never set on the vm itself. To set vm arguments when launching java the command line instead is java -Dfoo=bar -jar plugins/org.eclipse.equinox.launcer... ie, everthing before -jar is a vm arg and everything afterwards is program arg.
Also, most people invoking the director with java aren't setting -vmargs since without the eclipse.exe it doesn't do anything other than set the system property.
(In reply to comment #24) > This command line did not actually set any vm arguments > True, no system properties are set. A "getprop foo" returns blank. Funny though, that it does set the eclipse.vmargs property. OK, so everyone in favor of a new app? Any preferences for its name?
(In reply to comment #26) > OK, so everyone in favor of a new app? Any preferences for its name? +1, if you mean an .exe (eg., bug 273889). How about something like: p2dir.exe p2direct.exe director.exe As long as the app spits out usage info if run w/o options, I think we'd be making great strides toward convincing people that p2 via commandline isn't scary, and *is* a viable replacement for `unzip`.
New name org.eclipse.equinox.p2.director Let's not worry about the exe today.
(In reply to comment #28) > New name org.eclipse.equinox.p2.director > Let's not worry about the exe today. > One final detail. AFAIK, the only way to create an application with that name is to create a plug-in with the id 'org.eclipse.equinox.p2' that defines an 'org.eclipse.core.runtime.applications' extension with the id 'director'. I'm I right in that that's the only way? If so, should I go ahead and do this? The plug-in id is somewhat generic...
Never mind. I learned more about IExtension and namespaces tonight ...
Created attachment 133534 [details] New director application This is the current state of the new director application. Highlights: - Can handle multiple IU's - Can do both installs and uninstalls in the same run - New option -repository (or -repositories) for co-located repos - New option -shared [ shared location ] # defaults to $home/.p2 - New option -verifyOnly will cause planning but no install - New option -help currently does nothing (calls a stub method) - No options conflict. If -list is given, it is performed after any install/uninstall - NPE problem with bundle pool fixed - Bundle restart is performed if necessary. eclipse.p2.data.area is set according to -destination or -shared eclipse.p2.profile is set according to -profile This patch uses the VersionedName class and rather then inventing yet another instance, I made this patch dependent on bug 273674. So when testing, please apply that patch first.
Created attachment 133537 [details] Tests for the new director app. This patch contains a new test class for the new director application. It's basically a copy of the old one + Johns previous patch. One test will fail. That's because the VersionedName.toString() yields "xxx/0.0.0" when no version was found when parsing. I have a fix for that too but I'd rather wait with that until the patch in bug 273674 is applied (it's either that or set up a completely new workspace for that each patch).
(In reply to comment #32) > I have a fix for that too but I'd rather wait with that until the patch in > bug 273674 is applied. > As it turned out, that patch became stale so I had to provide a new one anyway. The new one includes the fix.
Created attachment 133578 [details] A replacement with some bugs fixed Some bugfixes related to handling of profileId. If no profile id is given and a config.ini exists in the destination, an attempt is made to use that profile. Otherwise, the destination path is used as the profileId.
I think that having a -p2DataArea argument is a must have for ppl with special needs where to put their p2.adata.rea
Running the following cmd line fails with the exception indicated below ./eclipse -application org.eclipse.equinox.p2.director -metadataRepository http://fullmoon.ottawa.ibm.com/eclipse/updates/3.5-I-builds -artifactRepository http://fullmoon.ottawa.ibm.com/eclipse/updates/3.5-I-builds -installIU org.eclipse.platform.ide -destination /Users/Pascal/tmp/newDirector/ -profile PascalSDK -profileProperties org.eclipse.update.install.features=true -bundlepool /Users/Pascal/tmp/newDirector/ -p2.os macosx -p2.ws cocoa -p2.arch x86 -roaming java.lang.IllegalStateException: !fwConfigLocation.equals(fwPersistentDataLocati on) !fwConfigLocation=/Users/Pascal/Downloads/eclipse 2/configuration ,fwPersistentDataLocation=/Users/Pascal/Downloads/eclipse 2/Eclipse.app/ Contents/MacOS/configuration at org.eclipse.equinox.internal.frameworkadmin.equinox.EquinoxManipulato rImpl.checkConsistencyOfFwConfigLocAndFwPersistentDataLoc(EquinoxManipulatorImpl .java:65) at org.eclipse.equinox.internal.frameworkadmin.equinox.EquinoxManipulato rImpl.loadWithoutFwPersistentData(EquinoxManipulatorImpl.java:360) at org.eclipse.equinox.internal.frameworkadmin.equinox.EquinoxManipulato rImpl.load(EquinoxManipulatorImpl.java:331) at org.eclipse.equinox.internal.p2.touchpoint.eclipse.LazyManipulator.lo adDelegate(LazyManipulator.java:50) at org.eclipse.equinox.internal.p2.touchpoint.eclipse.LazyManipulator.ge tConfigData(LazyManipulator.java:108) at org.eclipse.equinox.internal.p2.touchpoint.eclipse.actions.InstallBun dleAction.installBundle(InstallBundleAction.java:76) at org.eclipse.equinox.internal.p2.touchpoint.eclipse.actions.InstallBun dleAction.execute(InstallBundleAction.java:29)
I have released the last patch in HEAD.
