| Summary: | Review Ant task consistency | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Pascal Rapicault <pascal> | ||||||||||||||||||||||||||||||||||
| Component: | p2 | Assignee: | Matthew Piggott <matthew> | ||||||||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||||||||||||||||||||||||
| Severity: | normal | ||||||||||||||||||||||||||||||||||||
| Priority: | P3 | CC: | aniefer, mark.melvin | ||||||||||||||||||||||||||||||||||
| Version: | 3.5 | ||||||||||||||||||||||||||||||||||||
| Target Milestone: | 3.5 M7 | ||||||||||||||||||||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||||||||||||||||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||||||||||||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||
|
Description
Pascal Rapicault
We should also take a look at the input format for repos (URI / URL / path bug 264110). I've taken a look at tasks related to repositories (artifact.MirrorApplicationTask , metadata.MirrorApplicationTask, AbstractRepositoryTask, MirrorTask, Rep2Runnable) and the names of attributes are consistent. There are some differences in how the tasks handle addresses: - MirrorApplicationTask converts both destination and source addresses to URLs, Strings, and finally URIs. - AbstractRepositoryTask converts the source address directly to a URI - AbstractRepositoryTask converts the destination address to a Path, a File, then a URI - artifact.MirrorApplicationTask converts the baseline address to a URL Created attachment 126782 [details]
CSV of attributes used in ant tasks
I believe I've looked through all the p2 ant tasks, and I've compiled a spreadsheet (saved as CVS). I've found a few items which we may want to look at:
- AbstractPublishTask has an error message stating Metadata/Artifact Repository must be a URL which is used when it fails to convert a String into a URI
- MirrorApplicationTask (artifact & metadata) uses "writeMode" clean/append, but AbstractPublisherTask and Generator both use "append" true/false
- The CreateComposite..., Validate, Remove/AddChild tasks all use "location" which may be used source, destination, or both.
- The addresses of destination & source repositories are converted several times before they are converted into a URI. Given the conversions are not always same, could an address be allowable as one input, but not another?
The variations on address conversion are:
String -> URL -> String -> (application converts) URI
String -> Path -> File -> URI -> String
String -> URI
String -> (application converts) URI
After looking at the mirror tasks, it seems that most of the functionality of artifact.MirrorApplicationTask is also present in tools.MirrorTask. As both use the same Mirroring class when the mirror operation is performed, I believe the artifact.MirrorApplicationTask could be removed as the version present in tools allows composite repositories, and multiple sources. The task in tools would need a few attributes added but all should be easy to implement (comparatorID, baseline, ignoreErrors, raw, verbose and writeMode.) If metadata.MirrorApplicationTask were to extend tools.AbstractRepositoryTask the format for input between artifact and metadata mirroring tasks would be the same. This change would also require metadata MirrorApplication extend tools.AbstractApplication. One potential issue is that some input not used by the task (such as an artifact repository) would be valid input but would be completely ignored through the operation. If this is considered an issue perhaps the unused methods could be overridden and throw an exception or otherwise fail. The Add/RemoveChildTasks should probably also be modified so that their input would be similar to that used by the mirror tasks. These tasks could also extend AbstractRepositoryTask, though some thought would be required first to decide if the terms source and destination are appropriate. If they are not appropriate, a different super class could be created with more appropriate terms. Either way the change would mean that running a task would be able to add/remove multiple children instead of the multiple tasks required now. The ValidateTask could also be modified so it could validate multiple repositories with a single run. Misc. - Perhaps AbstractApplication ought to implement IApplication - I believe SlicingOption is mistakenly extending Task, currently it does not implement execute() and is simply used by the MirrorTask. Created attachment 127893 [details] Updated p2 Ant Tasks Here's the completed changes to the Ant tasks. There are a few examples available on the wiki ( http://wiki.eclipse.org/Equinox/p2_Ant_Tasks#Modifying_Children_of_a_Composite_Repository ) I've also modified the new MirrorApplication so it can completely mirror a repository if an IU has not been specified. Few internal changes still needed: - org.eclipse.pde.build.AssembleConfigScriptGenerator.generateMirrorTask() to use 'repository' instead of 'destination' tag - DestinationRepository should probably be renamed to something more generic as its being used for far more than destinations Created attachment 127894 [details]
Ported MetadataRepositories tests for new MirrorApplication
Two tests current fail due to changes with default mirror names. We may want to either update the tests to the new default naming scheme, or change the application to use the old style default.
Created attachment 128031 [details]
Updated p2 Ant Tasks
Created attachment 131327 [details]
Repository Ant Tasks & Tests
Updated Ant tasks as well as several tests for them.
Some things to change: - Externalization of the strings - Make sure PDE Build adopts the new task - Reintroduce options and attributes that have been removed and put a note in the code to deprecate it later Created attachment 131450 [details]
Patch with some of the modifications I have done
Created attachment 131810 [details]
Updated Tasks
Created attachment 131811 [details]
Tests
Contains tests for tasks, as well as ported versions of Metadata/Artifact Mirror tests for the new MirrorApplication.
Created attachment 132255 [details]
Updated Tasks
Created attachment 132298 [details]
Updated Tasks
Created attachment 132421 [details]
Ant Tasks
Created attachment 132422 [details]
Tests
Created attachment 132782 [details]
Ant Tasks
Created attachment 132783 [details]
Tests
Created attachment 132785 [details]
testData/mirror/zippedRepo.zip
Zip repo used in tests: testData/mirror/zippedRepo.zip
Created attachment 132929 [details] Ant Tasks Updated to add fix for bug 265851. I have released the last contribution. |