Bug 264110 - Input format consistency for application and tasks
Summary: Input format consistency for application and tasks
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: 3.5 M7   Edit
Assignee: Matthew Piggott CLA
QA Contact:
URL:
Whiteboard:
Keywords: polish
Depends on:
Blocks:
 
Reported: 2009-02-08 18:53 EST by Jeff McAffer CLA
Modified: 2009-04-30 16:09 EDT (History)
1 user (show)

See Also:


Attachments
Friendlier Mirror Apps (38.44 KB, patch)
2009-04-01 16:37 EDT, Matthew Piggott CLA
no flags Details | Diff
Friendlier Mirror Apps (18.95 KB, patch)
2009-04-02 16:44 EDT, Matthew Piggott CLA
no flags Details | Diff
Friendlier Mirrors (23.41 KB, patch)
2009-04-02 16:46 EDT, Matthew Piggott CLA
pascal: iplog+
Details | Diff
Director app, Ant tasks (20.65 KB, patch)
2009-04-03 11:40 EDT, Matthew Piggott CLA
pascal: iplog+
Details | Diff
Misc Ant changes & related tests (39.15 KB, patch)
2009-04-27 16:45 EDT, Matthew Piggott CLA
pascal: iplog+
Details | Diff
Publisher Ant Tasks (39.80 KB, patch)
2009-04-27 18:25 EDT, Matthew Piggott CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff McAffer CLA 2009-02-08 18:53:51 EST
in M5

There are cases in the current code where you MUST have URIs like
  file:/d:/foo/bar
(note the leading /) 

Other places the URI
  file:d:/foo/bar
(note the missing leading /) works just fine.

This will be very confusing to our consumers and we'll never be able to fix it as some people will assume one way and others will assume the other.  

Most of this has to do with repo creation.  For example, you can create an artifact repo withOUT the leading / but then you cannot use that repo to install as the download manager will fail.
Comment 1 Jeff McAffer CLA 2009-02-10 13:31:15 EST
in the p2 discussion yesterday it became apparent that various of the URIs I was using are either invalid or poorly formed or not as intended.  Fair enough.  There are two issues then:

- what's a normal person to do? (i.e., can we imporve the situation for folks by being more flexible or spanking earlier)
- can we support (how) relative locations for things like the profile location and the repo locations?  This is an interesting usecase for people wanting to have platform independent paths.
Comment 2 John Arthorne CLA 2009-02-11 16:04:40 EST
Are you talking about programmatic users or end users? In general for end user input we make a best effort to create a reasonable URI from their given input string. We are lenient I believe with malformed URIs such as file:c:/foo.  

For API's that take a URI as input, then there is no pre-validation because every instance of java.net.URI represents a valid URI (it just might not represent what you think it does). For example file:/c:/foo is a valid absolute opaque (non-hierarchical) URI. We could spank earlier if we added restrictions to the API such as "only absolute hierarchical URIs are allowed", but this seems a bit limiting in the general case.

The load problem you mention was bug 264111 which was fixed.

I'm not sure what else to do with this bug as they are very open-ended topics. Can you clarify what part you see as "critical"?
Comment 3 Jeff McAffer CLA 2009-02-21 21:40:56 EST
Completely agreed wrt API.

My comments were mostly around applications -- end users supplying command line args. I had several cases of the director app that were incrementally broken by the clarifications in URI use.  While that was a bummer, the real problem is in figuring out the correct thing to feed the apps.  Very few people understand the technical details of URIs and many type in stuff that they think is right only to find out that stuff does not work. The net result is failures that occur in very obscure places/ways making it nearly impossible for a normal person to figure out what they had done wrong.  I spent some hours stepping through the code trying to sort out the problems.

There are a few functional things like users should be able to spec relateive URIs for their repo locations. I tried all manner of different numbers of '/' etc and was not able to get that to work (though it used to). Perhaps I don't understand or perhaps it really does not work.

In any event, it is marked critical from a usability point of view. Users will be very frustrated trying to use the apps and replying that they should consult RFCxxxx will not win friends :-)  We would be doing ourselves and the community a big favor by being more flexible on the inputs.
Comment 4 Matthew Piggott CLA 2009-04-01 16:37:11 EDT
Created attachment 130616 [details]
Friendlier Mirror Apps

I've made some changes to the three mirror applications (I haven't changed org.eclipse.equinox.internal.p2.tools.mirror.MirrorApplication is it used?) to try and make input for repositories more friendly.

The applications should first attempt user input 'as is', if repository loading/creation fails an attempt is made to shape input.  This should allow Windows paths (C:\myRepo), Unix paths (/myRepo), and will create a zip URI (jar:file:/c:/repo.zip!/) if the file path ends in .jar or .zip

I've added a test for both Metadata & Artifact repositories which pass on Windows but given I'm changing file paths it needs a run on *nix as well.

The case file:d:/repo file:/d:/repo is caught by URIUtil.fromString() so it wasn't a problem mirror applications, I'm looking at some of the other applications and tasks next.
Comment 5 Matthew Piggott CLA 2009-04-02 16:44:14 EDT
Created attachment 130764 [details]
Friendlier Mirror Apps

I spoke with Pascal earlier and we felt that there shouldn't be any issues caused by the potential changes to repository locations, this cleans up the code considerably.

I've also added checks to ensure that destination directories can be written to.
Comment 6 Matthew Piggott CLA 2009-04-02 16:46:51 EDT
Created attachment 130765 [details]
Friendlier Mirrors

Missing a file
Comment 7 Matthew Piggott CLA 2009-04-03 11:40:27 EDT
Created attachment 130845 [details]
Director app, Ant tasks
Comment 8 Pascal Rapicault CLA 2009-04-10 14:27:17 EDT
I have released the last patch. I'm not quite sure of the overlap of the "friendlier mirrors" patch attached here with the code in the other bug related to Ant tasks.

We need to go through all the applications. For example I did not see any change coming for the publisher applications and I think this is necessary (esp verifying the destination repo). If no change are necessary, please mention it here.

The other thing that we want is a set of tests ensuring the behavior in presence of unexpected arguments.
Comment 9 Pascal Rapicault CLA 2009-04-23 14:41:00 EDT
The publisher task and the director needs to be reviewed.
Comment 10 Matthew Piggott CLA 2009-04-27 16:45:40 EDT
Created attachment 133453 [details]
Misc Ant changes & related tests

Ability to specify a filter for the slicer, validate option for Composite Repository task (artifact comparator) and better error messages.
Comment 11 Matthew Piggott CLA 2009-04-27 18:25:55 EDT
Created attachment 133462 [details]
Publisher Ant Tasks
Comment 12 Pascal Rapicault CLA 2009-04-30 16:07:30 EDT
This has been addressed. The publisher ant task work has not been released and I open bug #274576