| Summary: | [repo] RepositoryManager.contains(URI) throws IAE and breaks install when repo has bad reference | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Emanuel Graf <emanuel> | ||||
| Component: | p2 | Assignee: | Susan McCourt <susan> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | david_williams, pascal, pnehrer, thomas | ||||
| Version: | 3.6 | Flags: | pascal:
review+
|
||||
| Target Milestone: | 3.6 RC1 | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Emanuel Graf
What are you installing, from which update site? I tried Mylyn Task List and Eclipse Communication Framework SDK for both http://download.eclipse.org/releases/helios and http://download.eclipse.org/releases/staging similar symptom in bug 311367. I'm not marking a duplicate because the UI could be slightly more graceful in dealing with a null plan. Funny that we just start hearing about this. I wonder if the repo reference following is surfacing this. Is it possible we've had malformed repo references all along and the errors have been swallowed before, but now are showing up during resolution since ProvisioningContext is now using the reference API? I'm seeing this with a bunch of components, like Mylyn, Linux Tools, RSE, Subversive, and Buckminster (http://download.eclipse.org/tools/buckminster/updates-3.6). Doing the same exact thing on M6 does not cause problems. My M7 installation is now essentially un-updateable. (In reply to comment #2) > I tried Mylyn Task List and Eclipse Communication Framework SDK for both > http://download.eclipse.org/releases/helios and > http://download.eclipse.org/releases/staging I am able to select and install Eclipse Communication Framework SDK 3.2.0.v20100202-1607 Mylyn Task List (Required) 3.4.0.I20100204-0200-e3x without issue. We need to be able to reproduce this. From the stack trace, I can only see that there was a reference from some repository to this bogus location. But there seem to be some other strange things going on there. 1. For those having the problem. - do you have "Contact all update sites to find required software" checked in the install wizard? Do you have failure with or without this box checked? - try disabling all sites besides Helios and see if the install succeeds. It would appear some other site is involved. 2. I'm a bit concerned by the stack because it appears to be in the repo reference following code on the first attempt to resolve. Normally the reference following should only engage if the first attempt came up with a missing requirement. Could be a bug, but need to reproduce 3. I think throwing the IllegalArgumentException is really not appropriate. The client code seems quite innocent. It is trying to determine if the location in a repository reference is both known by the manager, and enabled by the manager. The failure indicates that the manager confirmed it contained the location and then IAE'ed on checking the enablement. This seems wrong. private boolean isEnabled(IRepositoryManager manager, IRepositoryReference reference) { return (manager.contains(reference.getLocation()) && manager.isEnabled(reference.getLocation())) || ((!manager.contains(reference.getLocation())) && ((reference.getOptions() | IRepository.ENABLED) == IRepository.ENABLED)); } You're right, the culprit is http://download.eclipse.org/releases/staging/. After disabling it in preferences I can now install Buckminster and I no longer get the error when updating. Having only http://download.eclipse.org/releases/staging/ enabled (and nothing else) and then running Help->Check for Updates reproduces the error. I can easily reproduce this now. Taking bug to fix the problem in the repo manager. (In reply to comment #6) > 1. For those having the problem. <snip> OK, so I've verified that there is an enabled artifact repository reference with a location of "%25update.label" in the staging repo. I did not track down who it's coming from. cc'ing David Williams. (where do I open a bug against the staging repo?) > > 2. I'm a bit concerned by the stack because it appears to be in the repo > reference following code on the first attempt to resolve. Normally the > reference following should only engage if the first attempt came up with a > missing requirement. Could be a bug, but need to reproduce I misread the stack. The reference being followed was an artifact repository, which is expected. Good. > 3. I think throwing the IllegalArgumentException is really not appropriate. > The client code seems quite innocent. It is trying to determine if the > location in a repository reference is both known by the manager, and enabled by > the manager. The failure indicates that the manager confirmed it contained the > location and then IAE'ed on checking the enablement. This seems wrong. Actually, it IAE'd when asking contains(URI) which is even worse in my opinion. This means anyone who hands a bogus URI to this API will IAE. I recall some discussion very late in 3.5 about whether repo managers should be so unforgiving, but it was so late in the game, we tried to ensure the UI did all the validation up front. In this case, though, the URI is coming from a repo reference, so no one gets a chance to validate it before the manager sees it. I think the proper fix (post 3.6) is to fix the repo manager. If we are going to be so strict, we should at least throw a checked exception so that clients know where they will get hit by this. Note that this can happen in any number of places: calling contains(URI), loading the repo, etc... For 3.6, I can catch IAE in the ProvisioningContext code and ignore these bad references. Created attachment 166994 [details]
patch to ProvisioningContext
This patch catches IllegalArgumentException in the loop where we are working with referenced repo locations. Any metadata or artifact repo reference that is malformed will be ignored by ProvisioningContext.
Note that this fix prevents the error in the install wizard. I was able to successfully install Mylyn Task List and ECF SDK with this fix. However, the problem in the staging repository must be fixed, or else it's possible that some other install will later fail with artifacts that were not found since this reference is ignored. > OK, so I've verified that there is an enabled artifact repository reference > with a location of "%25update.label" in the staging repo. I did not track down > who it's coming from. cc'ing David Williams. (where do I open a bug against > the staging repo?) > Cross-project bugzilla component (in "Foundation" --> Community: https://bugs.eclipse.org/bugs/enter_bug.cgi?product=Community (In reply to comment #12) > > OK, so I've verified that there is an enabled artifact repository reference > > with a location of "%25update.label" in the staging repo. I did not track down > > who it's coming from. cc'ing David Williams. (where do I open a bug against > > the staging repo?) > > > > Cross-project bugzilla component (in "Foundation" --> Community: > > https://bugs.eclipse.org/bugs/enter_bug.cgi?product=Community thx. opened bug 311596 fixed in HEAD >20100506. |