Bug 248045 - Director creates metadata directory with default content.xml if not existing
Summary: Director creates metadata directory with default content.xml if not existing
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 minor (vote)
Target Milestone: 3.5 M3   Edit
Assignee: Andrew Cattle CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2008-09-20 06:53 EDT by Baptiste Mathus CLA
Modified: 2008-11-05 16:10 EST (History)
3 users (show)

See Also:


Attachments
Fix and tests (14.39 KB, patch)
2008-10-01 14:33 EDT, Andrew Cattle CLA
no flags Details | Diff
Updated fix and more tests (29.92 KB, patch)
2008-10-02 10:54 EDT, Andrew Cattle CLA
dj.houghton: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Baptiste Mathus CLA 2008-09-20 06:53:42 EDT
Build ID: I20080617-2000

Steps To Reproduce:
1. Run director to install something. For example, the following command :
eclipsec.exe -nosplash -verbose -application org.eclipse.equinox.p2.director.app.application  \
 -metadataRepository file:c:/doesnotexist \
 -artifactRepository file:c:/neitherexist  \
 -installIU org.eclipse.platform.ide   -destination c:/install_dest   -profile PlatformSDKProfile   -profileProperties org.eclipse.update.install.features=true   -bundlepool c:/install_dest  -roaming   -vmargs   -Declipse.p2.data.area=c:/install_dest/p2
2. Notice that you made a typo in the metadata repository location
3. After a long time, director will create the missing directory and create an content.xml inside that will look like this:
<?xml version='1.0' encoding='UTF-8'?>
<?metadataRepository class='org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository' version='1.0.0'?>
<repository name='file:c:/doesnotexist - metadata' type='org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository' version='1'>
  <properties size='1'>
    <property name='p2.timestamp' value='1221767914837'/>
  </properties>
</repository>


More information:
I guess that this case was not noticed by the unit/integration test because when the repository is exposed through http, director won't be able to create something on this remote location. Maybe file protocol has been forgotten?

Maybe I misunderstood something, but I think that director should just crash or at least display an error message when encountering a non existing repo (exposed via file: or not).

Cheers.
Comment 1 Pascal Rapicault CLA 2008-09-20 14:47:22 EDT
This is correct, we will look at fixing that.
Comment 2 Jeff McAffer CLA 2008-09-27 14:38:09 EDT
its an interesting problem since in general all repos may or may not be present at the time of running the director.  for example, if you were not connected to the net then http: repos would not be accessible.  Note also that even if those specific repos are not present, you may be able to install things.  Having said that, in the specific case of the director app it would make sense to ensure that the repos are present if they were specifically mentioned on the command line
Comment 3 Andrew Cattle CLA 2008-10-01 14:33:26 EDT
Created attachment 114028 [details]
Fix and tests

The default behaviour was to add the repositories (which would try to load them first). I've changed it to try and load the repositories and throw a ProvisionException if they cannot be loaded (such as if they're invalid).

This resulted in changes to the director's message files and manifest was well as the manifests for both the artifact and metadata repositories and p2.core.

I've also included a few tests (neither repo exists, metadata doesn't exist, and artifact doesn't exist) and I've updated the director's AllTests.java
Comment 4 DJ Houghton CLA 2008-10-01 16:30:22 EDT
Andrew I just re-read comment #2 and our code needs to handle this case. (we still have at least one available repo)

Also in the #initializeRepositories method we should honour the "throwException" argument.

Also the copyrights should be updated with the right date.

Thanks.

Comment 5 Andrew Cattle CLA 2008-10-02 10:54:54 EDT
Created attachment 114108 [details]
Updated fix and more tests

I now create a log entry when a repository fails to load. I also keep a counter of how many repos failed to load and only throw an exception if throwException is true and all the repos of one type (artifact or metadata) fail to load.

Added more test cases to test this code.
Comment 6 DJ Houghton CLA 2008-10-02 17:24:27 EDT
Released modified patch. Changes I made:
- updated copyrights
- kept track of failed repos with a boolean... only need to know if we have at least one valid one
- if the manager isn't available only throw an exception if the throwException boolean arg is true