| Summary: | Do we really need to remove "eclipse" from repo locations? | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | DJ Houghton <dj.houghton> | ||||
| Component: | p2 | Assignee: | DJ Houghton <dj.houghton> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | dave.henderson, jeffmcaffer, john.arthorne, simon_kaegi | ||||
| Version: | 3.4 | Flags: | john.arthorne:
review+
|
||||
| Target Milestone: | 3.4 RC1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Adding John to CC because he is interested in this problem and likes getting email. Adding Simon to CC in case he has any ideas about what is going on. For reference, I should add that the questionable code is in: PlatformXMLListener#synchronizeConfiguration SiteListener#getToBeRemoved This probably causes various subtle bugs, such as bug 230097. I could also imagine this causing problems if the install directory is renamed from "eclipse" to something else or vice-versa. just fyi, bug 231120 doesn't happen when I'm running with modified JARs that don't have this extra URL path removal code. Created attachment 99536 [details]
patch
I have been doing some testing with this code removed and haven't had any problems. I believe the code was first added to order to help differentiate between the different repository types when calling #load on the repository manager. Since then we have:
- improved the individual repository validation methods
- changed the load/create process for extension location and update site repositories to match the other repositories
Adding John for review. +1. It bothers me a bit that we don't know why we were doing this. But, we already know a few subtle bugs this is causing, so I think we need to fix it. Since the install directory can have any name, any check that is specifically checking for an install directory called "eclipse" is problematic. Released. *** Bug 231979 has been marked as a duplicate of this bug. *** Re-opening due to bug 232340. Problem was addressed in bug 232340. Closing again. |
There is code in a couple of places which basically says: ... String location = site.getURL(); if (location.startsWith("file:") && location.endsWith("/eclipse/")) location = location.subString(0, location.length() - 8); ... This is curious. Why do we do this? Could this be the cause of (for instance) bug 231120? One theory is that we do it to aid in discovery when we ask the repository managers to load... the type is determined from the location when we call #validate. If we don't remove the last segment from the path perhaps we would confuse extension location repositories with update site repositories and regular local repositories? We need to investigate what is going on here, and if this code is really necessary.