Community
Participate
Working Groups
Packaged versions of Eclipse on Unix/Linux put Eclipse in /usr/share. Since this location is not writable by the user, an empty Install Location is presented to the user when they try to install a feature with the update manager. It would be better if an update site is automatically added to the user's home directory in this case. Patch to follow.
Created attachment 20147 [details] patch against org.eclipse.update.ui HEAD to fix this problem Even if you don't want to automatically make directory in the user's homedir to put the updates in, you should consider removing the "foo" label and checking if the install location is actually writable in getFirstTargetSite(). Both of these changes are in this patch. This patch will be included in the RPM packaged version of Eclipse >= 3.1.0_fc-0.M6.7.
Thanks for the patch. Does the site get created for feature updates as well? Particularly in the shared install scenario when users should not be able to update the shared features? Instead of automatically creating a site, shouldn't the wizard prompt the user to create the site and launch the existing action?
Yes the site would be created for updating installed as well as installing new features. Whether this is a good thing or not is debateable. It would be really nice to be able to lock down features that are in the shared location and only allow users to install/update the features that they specifically install. I think that this type for feature would be benifical for shared installs on Windows as well as on Unix/Linux. How difficult would this be to implement? Regarding creating the installation site automatically vs. prompting the user for a location, I'm partial to automatically creating the site. IMO most users don't really care where the thing goes as long as it works. For those that do care, they can always change the location if they want. Perhaps the location should only be display and not be created until the user clicks the “Finish” button. That way if the user doesn't choose the default homedir installation location, nothing will be created.
For updates, the newer version should always go to the location of the old version, so I think we should eliminate this case. The install wizard keeps track whether it works in "update" or "install new" mode, so the new location should only be created in "install new" mode. Also, given that this is a shared install, I think the code should also only (automatically) create the site in that case, and not in the general single user install. The new site should, perhaps, have a well-known path under the user's home directory (either a hard coded name, or something pased on the current product, etc.). This is no biggie, but I think it prevents collisions when the user has many eclipse based apps that don't work together. Not creating the site until the user clicks finish is a good option, but I am not sure if the code can currently handle this. Thoughts?
(In reply to comment #4) > For updates, the newer version should always go to the location of the old > version, so I think we should eliminate this case. The install wizard keeps > track whether it works in "update" or "install new" mode, so the new location > should only be created in "install new" mode. Ok, I wasn't aware of this, so I'll update the patch to do this. > Also, given that this is a shared install, I think the code should also only > (automatically) create the site in that case, and not in the general single > user install. I'm not sure I understand this point. Right now the way it determines if it's a shared install is if ECLIPSE_HOME is not writable. Is there something else it should check to see if it's working with a shared install? > The new site should, perhaps, have a well-known path under the > user's home directory (either a hard coded name, or something pased on the > current product, etc.). This is no biggie, but I think it prevents collisions > when the user has many eclipse based apps that don't work together. Good point, I'll see if I can come up with something that is based on the product plugin. > Not creating the site until the user clicks finish is a good option, but I am > not sure if the code can currently handle this. Ok, I'll look into it. My time is very limited right now, so I might not be able to make these changes right away. Feel free to ping me if I'm taking too long. Cheers, Ben
Ben, check the org.eclipse.osgi.service.datalocation.Location class for readonly status. You get an instance by calling Platform.getConfigurationLocation();
*** Bug 94036 has been marked as a duplicate of this bug. ***
Created attachment 21307 [details] updated patch for 3.1M7 with additons based on suggestions Changelog ========= o check if the Eclipse installation location is readonly o path now based on product plugin id o disable update radio button if Eclipse installation location is readonly I agree that it would be awkward to create the directory after the Finish button is clicked so I left that out. The only thing that needs to be done is to eliminate the case of updating features that are in the Eclipse installation location. Is disabling the update radio button sufficient for this? If not, do you have any suggestions on how to go about doing that? (BTW, sorry for the badly formatted patch. Eclipse seems to change the line endings when a file is edited.)
The patch is mostly ok, but there are a few tweaks that need to be done (I should have thought about this a bit earlier): If the user has already installed some new features in his own location, those features should be updateable. Therefore, disabling the update button is not enough (also, with automatic updates, the first wizard is not even run, the update job runs in the background and the results are sent straight to InstallWizard2). Here is what I'd suggest: - InstallWizard2 already keeps track of install mode (update or install new); use that to verify that you can automatically create the new site when in "install new" mode when the install location is read-only. Your patch needs minor adjustments for this (TargetPage) - perhaps you can remove the patch for ModeSelectionPage. The current patch may be good if you verify that the only site containing features is the install location (which is read-only). In that case it makes sense to disable the update button. - the update search classes in the core may need to be changed so that no lookups are performed for features in a read-only install location. Alternatively, this can be done in the OperationValidator, so you're informed of the error when selecting those updates. The latter is easier to implement (or maybe they're are equally easy/difficult), but the former improves search performance, as it doesn't do extra lookups. Sorry for going back and forth on this one, but the code changes are on a critical execution path, so there is potential for regressions.
Created attachment 27296 [details] Rebuild last patch against eclipse 3.1 I rebuilded the last patch, because you can't see what was really changed. The changes got lost because of the reformation. I create the patch against the released Eclipse 3.1.
Created attachment 30277 [details] patch for auto update siite in user's homedir Hi Dorian, I updated the patch with the first two suggestions from comment #9. I will continue to work on the third suggestion from comment #9 so that this can get committed. I tested this patch with I20051116 and org.eclipse.update.ui from HEAD today. To test, I used the subclipse update site (http://subclipse.tigris.org/update) to install an eariler version of the plugin and then tried updating to the latest. It worked without problems. I think that Stephan's patch is no longer needed. This patch applies to 3.1.1 cleanly. I haven't tested with 3.1.1 but I don't imagine there will be problems.
Created attachment 30664 [details] eclipse workspace patch - implements comment #9 This patch should fully implement what's described in comment #9. I need to do some more testing with it, but I'd like to get some comments in the mean time. I moved a private method from an internal class to UpdateUtils. I know a way to avoid this if it's not desirable.
I just tried out some RPMs built with Ben's latest patch and I can successfully scan for updates for things I have installed in my home directory's update site. I installed an older version of Subclipse (uncheck the "latest version only" checkbox) and then scan for an update and install it. Nice work, Ben!
Created attachment 31212 [details] updated patch for update site in home dir (core) Ben asked me to post this patch here. It is the version that I have been testing.
Created attachment 31213 [details] updated UI part of the update-site-in-home-dir patch Again, Ben asked me to post this as this is the patch I was testing. This one was particularly difficult because the line endings are DOS in one of the files but not the other and Eclipse generated the patch with no DOS line endings for either file (and `patch` attempted to apply the non-DOS line ending parts to a DOS file ... grr). I believe there's a bug already filed about this so feel free to ignore my rant.
Thanks Andrew. I'm going to spend some time tomorrow testing these patches outside of the rpm to make sure it will still work in the non-packaged case too. I'll report back.
Created attachment 31331 [details] org.eclipse.update.core patch I removed the canWrite method from the core plugin. I forgot to remove it when I changed some stuff around. Andrew, can you please mark the core patch that you submitted as obsolete? Thanks. I tested this outside of the rpm use case and it seems to be working. Dorian, it would be nice to get some comments when you have some time. Thanks.
Comment on attachment 31212 [details] updated patch for update site in home dir (core) Obsolete as per Ben's request.
Hi, is there any chance this will be considered for 3.2? Thanks, Ben
Created attachment 34000 [details] patch for update.core and update.ui Patch ChangeLog =============== * delete update site directory if a problem is encounter while making the site
This looks better. Without being too picky, it would be more precise to actually check if the site directory exists, and *only* then delete it upon failure. In most cases, I would expect the main path code to work if the folder is already there, but you never know...
(In reply to comment #21) > This looks better. Without being too picky, it would be more precise to > actually check if the site directory exists, and *only* then delete it upon > failure. Right, my understanding is the the File.delete() method would just return false if the File isn't there. I can put an explicit test if you want. Let me know.
Created attachment 34094 [details] small update in patch for update.core and update.ui I tested out the delete code on a directory that doesn't exist and it in deed does work. I added a comment to reflect this.
Created attachment 34095 [details] class used to test deleteDir code
Just adding my 2 cents: Ben and Andrew have been maintaining that patch for almost a year now, avoiding meticulously that went goes stale.... It has been already reviewed by Dorian, they followed his recommendations. Could somebody have a look at it, like, now before RC0? I think that would be fair given the amount of work those two have put in, and is important with Calisto being released as an update site.
My understanding is that this patch can not be applied until after the 3.2 release because the API is currently frozen.
My understanding is that this is not an API change and even though, changes to the API are still possible, they just require a bit more "paperwork".
We will look at it and see if we can squeeze it into 3.2.
I tried applying the patch against the code in head and have some problems. Can you update the patch for the list time to be compatible with the latest Update code?
Created attachment 37266 [details] workspace patch for update.core and update.ui updated to HEAD Mar 30, 2006 12:30AM
Created attachment 37269 [details] update patch for real
Ahh.... New code for digest support went in last night, making the patch incompatible again. I suggest you wait until M6 is out, then attach a patch compatible with M6 on Monday. I will release it right away so that it is picked up by the Tuesday I build. This will give us enough time to flush out any side-effects that may show up. Branko, please coordinate other code releases with this patch - it is a pain to track the moving target and you will release more code soon.
(In reply to comment #32) > Ahh.... New code for digest support went in last night, making the patch > incompatible again. It should work, I updated the patch after that code went in. I just re-tested the functionality again this morning and tested applying the patch to a fresh check out. Both tests passed.
Hi, What actually happened with this? The patch I submitted on Thursday morning was compatible with HEAD at that time, but I guess that was really close to M6. If I update the patch today, will it be applied? I'm not intending to sound rude, I just have other stuff on the go ATM so it would be nice if you could let me know when would be a good time to update the patch :)
Branko, please look the patch over and release it if ok.
Any updates on this one? It seems fairly trivial and would be *nice* for 3.2.1 I'll let Wassim take you out for lunch if this is in for 3.2.1 ;)
(In reply to comment #36) > Any updates on this one? It seems fairly trivial and would be *nice* for 3.2.1 There's an API addition so it probably shouldn't go into 3.2.1. I'm going to update the patch to HEAD next week sometime and try to get it applied early this time.
Can you do it without API changes?
(In reply to comment #38) > Can you do it without API changes? I suspect not, but I'll investigate next week and report back.
Ben, As far as I could tell in your patch the only place where you change public API is in org.eclipse.update.search.IUpdateSearchQuery.java where you add getFeature() method. And the only place you are using it is in org.eclipse.update.search.UpdateSearchRequest.java. You can try to test there if the query is one of the internal classes and if it is cast it and call the method. This would allow us not to change public PAI at least for 3.2.1. Also, I would expect that if there is already installation location that is writable that we do not add user home to installation locations automatically.
Created attachment 51212 [details] eclipse-sdk-3.2.1.patch The patch for eclipse 3.2.1 is missing but this bug is still open. This is the patch used to build eclipse on Gentoo Revise it to meet your needs
any updates on this bug? I'm getting the "foo" behavior in an RCP app and it looks really bad! Is this fixed in any released eclipse?
I'm planning on updating the patch when I have time - I should be able to get to it in the next few weeks, but I can't promise anything. If I'm able to get it done before 3.2.2 I'll look at removing the API addition so that it can be applied for that release.
I just realized that the proposed patch goes about installing plugins the in OSGi configuration area, which should only be about transient data imho. I am not sure yet of all the ramifications, but OTH that could lead to some fun and weird issues, as I shoudl be able to delete my ~./eclipse folder, without loss of software (possibly some loss of settings like the recently used workspaces, but not losing installed software).
So what about putting them in ~/.eclipseplugins/?
(In reply to comment #44) > I just realized that the proposed patch goes about installing plugins the in > OSGi configuration area, which should only be about transient data imho. > I am not sure yet of all the ramifications, but OTH that could lead to some fun > and weird issues, as I shoudl be able to delete my ~./eclipse folder, without > loss of software (possibly some loss of settings like the recently used > workspaces, but not losing installed software). I'm not sure I agree with you here Philippe. As a user if I in install plugins in a directory outside of ~/.eclipse, say ~/.eclipseplugins, and then rm -r ~/.eclipse, I still lose the plugins because the file that contains the reference to ~/.eclipseplugins (~/.eclipse/org.eclipse.platform_3.2.0/configuration/org.eclipse.update/platform.xml) has been deleted. Sure I can just point eclipse to ~/.eclipseplugins and I will not have to download the plugins again, but since it's created automatically I may not know where my plugins are. What about automatically adding ~/.eclipseplugins as an eclipse extension site on unix systems when the eclipse install directory is read-only? That way you can delete ~/.eclipse and you will not lose your locally installed plugins. Branko, would it be possible to get your input on this? I will have time to work on this patch next week and I want to be sure that I'm going in the right direction. Thanks.
Created attachment 55864 [details] patch updated to HEAD -- 13:00 EST - 2006-12-18 I just updated this patch to HEAD - the only changes that I made were some whitespace changes that allow the patch apply cleanly. I didn't remove the API as suggested in comment #40 because this isn't needed to be included in 3.3. I also verified that the patch doesn't create an install location in the user's home directory if there is a pre-existing install location that is writable -- as requested in comment #40. As far as the concerns in comment #44 go, I think that we should file a separate bug for this once the patch has been applied because the patch that I'm attaching has already been reviewed. Thanks.
Hi, is there any news here?
Dejan/Branko: Can you consider this one for 3.3? It's been reviewed by multiple update committers already, and as far as I can see it just wasn't put into 3.2 because it was past the API freeze. Ben's been updating this patch for over 18 months now...
The idea of adding home directory as a new installation directory for plug-ins is very nice but there are several issues with doing it automatically: 1) What if user does not want his home directory to be used? If I understood patch we will ignore that completely. 2) What if user has several Eclipse installations (as many eclipse developers do) and one of them already uses home for some if its plug-ins/features. This might destroy other (valid) installation. 3) Assume that I have decided on purpose to disable site in home directory? We just enable it without asking questions (could be pretty big installation there)? Delete it although it might be useful to user (even worse s/he might have bought that software)? I think better solution would be to somehow mark location as locked (maybe new column in table of features on Target Page) and act on it in the same way we act on not having enough space. Add button and option in pop-up menu to add home directory as installation directory. Warn user if there is a site already in his home directory. I think this should be good enough for users and avoid all of the above issues and many other (by putting it in user hands).
(In reply to comment #50) > The idea of adding home directory as a new installation directory for plug-ins > is very nice but there are several issues with doing it automatically: > > 1) What if user does not want his home directory to be used? If I understood > patch we will ignore that completely. The patch creates only creates a new installation directory if there are no other installation directories available that are writable. If the installation directory is created, the path is presented to the user and there is a button they can click to change the location. If a user does end up changing the location, the location that was just created in the home dir is not remove. Perhaps the patch should be updated to remove the installation directory in the home dir in this case. > 2) What if user has several Eclipse installations (as many eclipse developers > do) and one of them already uses home for some if its plug-ins/features. This > might destroy other (valid) installation. If this happened they could disable the installation directory in their home dir. Realistically I don't think this would be a problem because Eclipse developers don't work with read-only Eclipse installation directories so updates would be installed in the Eclipse installation directory rather than the installation directory in their home dir. > 3) Assume that I have decided on purpose to disable site in home directory? We > just enable it without asking questions (could be pretty big installation > there)? Delete it although it might be useful to user (even worse s/he might > have bought that software)? If the site exists and is disabled, an error should be presented at the top of the wizard indicating this problem. The user would then have the option of cancelling the installation or changing the location directory. I think this functionality would have to be added to the patch. > I think better solution would be to somehow mark location as locked (maybe new > column in table of features on Target Page) and act on it in the same way we > act on not having enough space. Add button and option in pop-up menu to add > home directory as installation directory. Warn user if there is a site already > in his home directory. I respectfully disagree for the reasons stated above. Also, I don't think that adding more options is the way to go with this. The patch works for 99% of the cases we see - most people don't really care where there software installed as long as it works. And this patch works because we've been using for over a year and a half. If addressing the problems in points 1 and 3 with the proposed solutions is the last little bit of polish that needs to be done to get this patch accepted, I will be happy to do it. Otherwise, I suppose we will have to disagree and Fedora and Red Hat Eclipse (and possibly other versions of Eclipse on Linux) will continue to use the patch in its current form because it's hard for me to justify spending more time on this without it getting committed. Thanks for your input here Branko :)
Created attachment 61115 [details] patch for vista issues Here is a modified patch that uses the same approach as bug 168445 to figure out if the install area is read-only. In addition this patch first attempts to place the install location in the configuration area. If that is read-only then we fall back to creating an install location in the users home like the original patch. John Arthorne is going to investigate removing the new API given how close we are to API freeze.
The latest patch writes the plugins to: System.getProperty("user.home") + File.separator + ".eclipse" + File.separator + Platform.getProduct().getId() + File.separator + "updates" Would it be better to write it to "eclipse" instead of ".eclipse" on Windows? Any thoughts here?
Eclipse has several other places where a prepended '.' is used (.metadata, .log etc.). The code in the launcher also sets the default configuration location to be in user.home/.eclipse if it determines that the install area configuration is read-only. I'm not in favor of changing all of the file names to remove the prepended '.' at this point.
Created attachment 61410 [details] Updated patch This is an update on Tom's patch that eliminates the API change as suggested by Branko in comment #40. Since the new method was only used within the same plugin, it is not necessary to expose it as API.
It is important to address this for 3.3, or update will fail to work properly on Windows Vista. The short summary is that Windows Vista doesn't allow non-administrator users to write files under "Program Files". However, as a backwards-compatibility measure, they "virtualize" the Program Files directory, so that modifications actually occur in a private copy of Program Files that is local to the current user. When such a user runs Update Manager, everything they download gets written to their private virtual copy of Program Files. This actually works ok for *some* plugins. However, as a security measure, Vista doesn't allow certain kinds of files to be written to the virtual store. This includes .exe, .dll, and other executable file types. If a user attempts to install a plugin containing such files when eclipse is installed under Program Files, the update will fail. Note: The latest patch needs further testing. I don't think this is important to address for M6.
From testing, it looks like there is a corner case not covered by the current patch. UpdateUtils.getDefaultTargetSite will use the same site as the old feature in the case where a newer version of a feature is being installed. If the old feature is not in a writable configured site, then you will get a failure when it tries to install the new version of the feature. I could take a stab at also fixing this, but I'm curious why this was never encountered with the patch used by Linux distros. Perhaps there is a deeper update problem here I'm not understanding. It could be related to the current state of the Europa update site, which currently contains an older version of the PDE feature that appears to be newer because of changes to the format of feature version qualifiers. I created a fake exception to mark the point of failure when this happens (if you try to install all of Europa against latest platform M6, with Eclipse install directory read only, you will get this): at org.eclipse.update.internal.core.ConfiguredSite.canWrite(ConfiguredSite.java:797) at org.eclipse.update.internal.core.ConfiguredSite.verifyUpdatableStatus(ConfiguredSite.java:782) at org.eclipse.update.internal.core.ConfiguredSite.install(ConfiguredSite.java:101) at org.eclipse.update.internal.core.ConfiguredSite.install(ConfiguredSite.java:87) at org.eclipse.update.internal.operations.InstallOperation.execute(InstallOperation.java:92) at org.eclipse.update.internal.operations.BatchInstallOperation.execute(BatchInstallOperation.java:84) at org.eclipse.update.internal.ui.wizards.InstallWizard2.install(InstallWizard2.java:349) at org.eclipse.update.internal.ui.wizards.InstallWizard2.access$1(InstallWizard2.java:346) at org.eclipse.update.internal.ui.wizards.InstallWizard2$1.run(InstallWizard2.java:459) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:58)
Arghh... Due to the recent modifications around the same classes (InstallWizard2 and TargetPage), the patch cannot be applied without conflicts any more. John? can you merge the changes from Head and recreate the patch? Thanks.
Created attachment 61736 [details] Patch merged with latest HEAD
Versioned and released. It is up to you if you want to resolve as fixed or keep it around for the issue in comment 57.
Created attachment 61746 [details] Removed debugging statement I wasn't expecting you to release that... a debugging statement made its way into that patch when I merged the changes with HEAD. Here is an update to remove the debugging statement.
Removed the debugging statement. In the future, it would be help to mark a patch as 'good to go' or something.
Marking as fixed.
Dejan, John mentioned that the patch was not fully tested in comment #56. (also the bug was marked as targeted for M7 at the time he attached the patch, hence the surprise at release into M6)
Agreed. I guess I was in a 'patch releasing' mood. Never pleasing some people - first 'Update does not release patches for months', now 'he released our patches' :-).
