Bug 326926 - API to configure and import SCM URLs
Summary: API to configure and import SCM URLs
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.7 M4   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL: http://wiki.eclipse.org/PDE/Importing...
Whiteboard:
Keywords:
Depends on:
Blocks: 320552 330490
  Show dependency tree
 
Reported: 2010-10-04 10:25 EDT by Darin Wright CLA
Modified: 2010-12-16 08:24 EST (History)
13 users (show)

See Also:


Attachments
Fix v01 (24.03 KB, patch)
2010-11-03 09:09 EDT, Tomasz Zarna CLA
no flags Details | Diff
mylyn/context/zip (251.59 KB, application/octet-stream)
2010-11-03 09:09 EDT, Tomasz Zarna CLA
no flags Details
Fix v02 (21.31 KB, patch)
2010-11-12 09:03 EST, Tomasz Zarna CLA
no flags Details | Diff
Fix v03 (20.40 KB, patch)
2010-11-26 15:33 EST, Tomasz Zarna CLA
no flags Details | Diff
Fix v04 (19.97 KB, patch)
2010-12-01 09:31 EST, Tomasz Zarna CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Wright CLA 2010-10-04 10:25:34 EDT
PDE requires an API from Team to materialize workspace projects from SCM URLs. SCM URLs are URIs that refer to projects/source code in different repositories (similar in concept to "project set format" files).

SCM URLs can be specified in bundle Manifests to refer to source code associated with a bundle. Having support from team to materialize projects in the workspace allows users to easily import the source code associated with bundles as workspace projects.

The following WIKI page describes the current and desired API/implementation:

   http://wiki.eclipse.org/PDE/Importing_SCMURLs

PDE originally provided this function in 3.6 using internal code and extension points. During 3.7, we'd like to make a public API to allow other team providers to participate.

There is also some relevant history on bug 195729.
Comment 1 Jeff McAffer CLA 2010-10-04 10:48:37 EDT
sounds great!
Comment 2 Darin Wright CLA 2010-11-02 10:32:14 EDT
Thomas asked me if the "tag" attribute PDE writes in the bundle headers currently is part of the SCMURL standard for CVS. Neither one of us sees how to specify a version/tag in a CVS SCMURL (http://maven.apache.org/scm/cvs.html), but it seems like there must be a way to do this.

Perhaps Alex or someone else out there knows?
Comment 3 Alex Blewitt CLA 2010-11-02 17:18:14 EDT
(In reply to comment #2)
> Thomas asked me if the "tag" attribute PDE writes in the bundle headers
> currently is part of the SCMURL standard for CVS. Neither one of us sees how to
> specify a version/tag in a CVS SCMURL (http://maven.apache.org/scm/cvs.html),
> but it seems like there must be a way to do this.
> 
> Perhaps Alex or someone else out there knows?

No, it isn't possible to do this with the CVS-based Maven SCM URL. But that's OK. The ;tag that gets appended is actually an OSGi parameter to the value.

For SVN, the branch is an implicit part of the name (mostly used as 'trunk') but similar constraints are needed.

For the Git repository, there is a desire to add the commit id after the end of the URL using a similar OSGi parameter.

The main reason is that the Maven SCM URLs often are used to say 'where to get this software from', whereas PDE uses it to say 'and which exact version is used to do the build'. So the idea behind using the OSGi parameter was a way of extending that information without changing the original SCM URL.

It's currently written out with the Source-References IIRC.
Comment 4 Alex Blewitt CLA 2010-11-02 17:22:55 EDT
BTW this came up on the egit-dev mailing list recently:

http://dev.eclipse.org/mhonarc/lists/egit-dev/msg01549.html
Comment 5 Tomasz Zarna CLA 2010-11-03 09:09:09 EDT
Created attachment 182292 [details]
Fix v01

This is a working version of the ultimate fix. 

What's done:
* CVSURI.fromURI(URI) accepts now URIs in the SCMURL format which allows PDE to use ProjectSetCapability.asReference(URI, String) for CVS-based SCMURLs
* keeping in mind the suggestion that the configuration when importing project references should be done at the team provider level I have removed the RepositoryImportWizard. The "Import As > Project from a Repository" action calls now IBundleImporter.performImport, and it's up to the importer implementation to provide the UI. Currently, the CVSProjectSetCapability simply follows the configuration from refs.

What's still missing:
* more test cases with SCM URLs. For instance, I read on the thread from comment 4 about ";project=" parameter, which at the moment is ignored by the code
* I haven't touched the "Import > Plug-ins and Fragments" wizard
* Given the fact we would like to make team providers responsible for import configuration, I think we can remove CVSBundleImportPage or maybe even the bundleImportPages ext. point. Ideally, the code from the page could be use as a primer for the UI in CVS.
* I would like to use PluginImportOperation in ImportActionGroup to schedule the import. Right now importer.performImport(BundleImportDescription[], IProgressMonitor) is called with null progress monitor and gives no info about the operation state.

Let me know if this is going in the right direction. Maybe you don't want me play with your (PDE) code so I should stick to the Team part (implement ProjectSetCapability.asReference and move PDE's wizard page to CVS). Comments are welcome.
Comment 6 Tomasz Zarna CLA 2010-11-03 09:09:16 EDT
Created attachment 182293 [details]
mylyn/context/zip
Comment 7 Darin Wright CLA 2010-11-09 10:06:57 EST
Hi Thomasz. Yes - the API for converting a SCMURL to a project reference looks good, and the patch is headed in a good direction. The next step would be to set up a UI such that the user could choose HEAD vs. a specific project version.

I wonder - does the approach allow for bundles from different repository providers? For example, I have some bundles from CVS, some bundles from SVN, and some bundles from Git. I assume PDE could compute 3 sets of project references and then start import operations?
Comment 8 Szymon Brandys CLA 2010-11-10 07:46:27 EST
(In reply to comment #7)
> Hi Thomasz. Yes - the API for converting a SCMURL to a project reference looks
> good, and the patch is headed in a good direction. The next step would be to
> set up a UI such that the user could choose HEAD vs. a specific project
> version.

Darin, I would raise a separate bug for the UI change and leave this one for the API for converting a SCMURL. We may not make the UI change for M4, however API seems to be ready.
Comment 9 Tomasz Zarna CLA 2010-11-12 07:23:04 EST
One more question regarding SCM URLs: I mentioned in comment 5, that I came across an Eclipse-SourceReference that looks like this: 

scm:cvs:pserver:dev.eclipse.org:/cvsroot/rt:org.eclipse.equinox/compendium/bundles/org.eclipse.equinox.http.jetty6;project="org.eclipse.equinox.http.jetty";tag=v20100503

The problem with it is that when creating an URI for that String I get a java.net.URISyntaxException. Is this expected? I assume the role of quotation marks here is to allow project names with spaces, but are they really necessary? Or maybe we should encode Strings before creating URIs for them?
Comment 10 Alex Blewitt CLA 2010-11-12 08:02:33 EST
The format is:

[mavenurl (;name=quoted-or-not)*]+

The entire thing is not a URI - indeed, there can be many comma separated ones (hence references). You should only treat to the ; as the maven uri.
Comment 11 Tomasz Zarna CLA 2010-11-12 08:48:05 EST
(In reply to comment #7)
> I wonder - does the approach allow for bundles from different repository
> providers? For example, I have some bundles from CVS, some bundles from SVN, and
> some bundles from Git. I assume PDE could compute 3 sets of project references
> and then start import operations?

Well, the ProjectSetImporter iterates over all team providers from a project set, tries to find an implementation of ProjectSetCapability for each of them and then calls addToWorkspace()[1] using reference strings for each provider. So the answer is yes, the scenario you mentioned will work... but giving no way to modify the configuration from the psf file, because there is no UI for it. This is something you added with your CVSBundleImportPage.

So, the trickiest part is to move the page to CVS, add it to the Team Project Set wizard and allow PDE to open the wizard with the psf location already set. Of course, the wizard should look for pages for each provider taking part in the psf import. However, a team provider doesn't have to define a page to be displayed by the wizard. If so, the import will take place according to settings from the file (as it's happening now).
Comment 12 Tomasz Zarna CLA 2010-11-12 09:03:30 EST
Created attachment 182994 [details]
Fix v02

Phase 2: CvsBundleImporterDelegate uses Team API to import bundles, works in both context menu action and the import wizard. Moreover, using CvsBundleImportPage you're able to alter the URIs passed to Team, so the option to import from HEAD or using a revision is honored.

Still missing, phase 3: 
* move the page to CVS
* add ext. point for these kind of pages in Team UI
* open the Team Project Set wizard primed with CVSURIs from PDE
* remove obsolete ext points in PDE
* figure out how to handle ;name="value" params
Comment 13 Tomasz Zarna CLA 2010-11-12 10:09:57 EST
(In reply to comment #11)
> the answer is yes, the scenario you mentioned will work

Forgot to mention that it will work only if all team providers are able to understand SCM URLs, otherwise you need to fire up the import operation with valid reference strings (a psf file).
Comment 14 Darin Wright CLA 2010-11-16 10:47:01 EST
(In reply to comment #12)
> Still missing, phase 3: 
> * move the page to CVS
> * add ext. point for these kind of pages in Team UI
> * open the Team Project Set wizard primed with CVSURIs from PDE
> * remove obsolete ext points in PDE
> * figure out how to handle ;name="value" params

Patch/approach looks good. One question about the ProjectSetCapability.asReference(URI, projectName) API. Looks like the API always expects a project name (i.e. is not spec'd that it can be null). Should you doc that null can be passed in? or do you want consumers of this API to always pass in a project name?
Comment 15 Szymon Brandys CLA 2010-11-17 07:42:23 EST
(In reply to comment #12)
> Still missing, phase 3: 
> * move the page to CVS
> * add ext. point for these kind of pages in Team UI
> * open the Team Project Set wizard primed with CVSURIs from PDE
> * remove obsolete ext points in PDE
> * figure out how to handle ;name="value" params

* move the page to CVS
* add ext. point for these kind of pages in Team UI
* open the Team Project Set wizard primed with CVSURIs from PDE

Tomasz, Darin as said before, please open a separate bug for this. We may not make the UI part for M4.
Comment 16 Darin Wright CLA 2010-11-17 12:39:45 EST
I'll open a new bug for the UI side of things. However, I think we still need one more piece of API on the team side in order to be able to get rid of the PDE bundleImporters extension point.

Currently, we have a CVS specific extension in PDE to import bundles, however, what we'd like to do is have no extension point and just let PDE ask team for the appropriate RepositoryProviderType given a URI (SCMURL). Alternatively, if team provided an API to get *all* RepositoryProviderTypes, we could just iteratively ask each provdier for a project reference given a URI (and if a provider does not understand the URI it returns null as per the API contract...). Currently the team API only allows clients to ask for a specific provider by ID, which means one has to know about all providers up front.
Comment 17 Tomasz Zarna CLA 2010-11-26 12:25:32 EST
(In reply to comment #14)
> Should you doc that null can be passed in? or do you want consumers of this API to
> always pass in a project name?

Haven't decided yet, but I think I'll go with the former.

(In reply to comment #16)
> Alternatively, if team provided an API to get all RepositoryProviderTypes...

The API is already there. You can call org.eclipse.team.core.RepositoryProvider.getAllProviderTypeIds() to get ids of all known repository providers. And then for each id you could ask org.eclipse.team.core.RepositoryProviderType.getProviderType(id) to get a RepositoryProviderType.
Comment 18 Tomasz Zarna CLA 2010-11-26 15:33:51 EST
Created attachment 183958 [details]
Fix v03

API changes and tests only.
Comment 19 Tomasz Zarna CLA 2010-11-26 15:40:27 EST
Next week I will review the patch, probably adjust wording for javadocs and hopefully release it. The changes from Fix v02 will be moved to bug 330490.
Comment 20 Tomasz Zarna CLA 2010-12-01 09:31:37 EST
Created attachment 184262 [details]
Fix v04

The final version of the fix.
Comment 21 Tomasz Zarna CLA 2010-12-01 09:33:40 EST
In HEAD, available in builds >N20101130-2000.
Comment 22 Dani Megert CLA 2010-12-02 03:17:56 EST
The @since tag for org.eclipse.team.core.ProjectSetCapability.SCHEME_SCM was wrong. Fixed that in HEAD.

Please enable the API Tools in your workspace by defining the correct API baseline, as this would have reported the error to you.
Comment 23 Tomasz Zarna CLA 2010-12-02 05:59:49 EST
The @since tag was fine, I forgot to increment the bundle version in MANIFEST.MF. Reverted your change in HEAD and updated the manifest. Sorry for the trouble.

I do have the API Tools enabled, I just didn't know my baselines got broken, see bug 331644.
Comment 24 Dani Megert CLA 2010-12-02 07:05:28 EST
(In reply to comment #23)
> The @since tag was fine, I forgot to increment the bundle version in
> MANIFEST.MF. Reverted your change in HEAD and updated the manifest. Sorry for
> the trouble.
Actually the bundle version already got increased during 3.7 development before, therefore I reverted your changes in HEAD.

> I do have the API Tools enabled, I just didn't know my baselines got broken,
> see bug 331644.
Oops.
Comment 25 Tomasz Zarna CLA 2010-12-07 07:12:31 EST
Verified in I20101206-1800, the API is there and the tests still pass. 

While on it I noticed that AllTestsCVSResources suite is disabled on Windows. Opened bug 332006 to re-enable it.
Comment 26 Ankur Sharma CLA 2010-12-15 14:32:16 EST
(In reply to comment #17)
> (In reply to comment #14)
> > Should you doc that null can be passed in? or do you want consumers of this API to
> > always pass in a project name?
> 
> Haven't decided yet, but I think I'll go with the former.
> 

If I don't supply the project name to 'asReference', it returns null. This means, project name can not supplied null?

 this was may call

asReference("scm:cvs:pserver:dev.eclipse.org:/cvsroot/eclipse:pde/ui/org.eclipse.pde;tag=v20100705", null)
Comment 27 Alex Blewitt CLA 2010-12-15 16:13:26 EST
An earlier iteration used the last (slash separated) component of the scm url as the project name if it wasn't supplied. Given that all (plain) maven projects won't have a project attribute, requiring one would be the wrong thing to do (but allowing it as an override).
Comment 28 Tomasz Zarna CLA 2010-12-16 06:01:09 EST
(In reply to comment #26)
> If I don't supply the project name to 'asReference', it returns null. This
> means, project name can not supplied null?
> 
> this was may call
> 
> asReference("scm:cvs:pserver:dev.eclipse.org:/cvsroot/eclipse:pde/ui/org.eclipse.pde;tag=v20100705",
> null)

Right, the code in HEAD will return _null_ if you don't provide a project name *and* the SCM URL doesn't have a "project" attribute. Alex is right, there was a version of the fix that in the above situation extracted the name from the last segment of the URL's path, but eventually I decided to make the code simpler and removed that. Please open a separate bug if you think this is something we need to change and I'll fix it. If you could modify org.eclipse.team.tests.ccvs.core.cvsresources.CVSURITest to show me how exactly you would like the API to work that would be fantastic.
Comment 29 Alex Blewitt CLA 2010-12-16 08:24:12 EST
Raised bug 332732 to deal with the regression.