Bugzilla – Full Text Bug Listing
| Summary: | IProjectSetSerializer cannot be used in headless environment | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Dan Rubel <danrubel> | ||||||||||||
| Component: | Team | Assignee: | Michael Valenta <Michael.Valenta> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||
| Severity: | normal | ||||||||||||||
| Priority: | P3 | CC: | clayberg, rwilms | ||||||||||||
| Version: | 2.1 | ||||||||||||||
| Target Milestone: | 3.0 M9 | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Windows XP | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
Very good! Such a new interface would allow us to get rid of some unnecessary dialogs in the ProjectSet Support yielding direct benefit for users. This is an interesting proposal but I have the following concerns. 1) I can't see what this buys you from a CVS standpoint since the CVS client in Eclipse does not work in headless mode (i.e. there is no API so there can be no headless operation). Are you populating your workspace to perform headless operations from other plugins on the resources or are you expecting to perform CVS operations on them? 2) You say that a repository provider could override the default IProjectReferenceHandler to provide custom prompting, etc. This would only work if the there was yet another extension point for the handler since it is general Team UI code that invokes the serializer and it is this code that would need a means of instantiating the custom handler. 3) Given point 2, this seems like a bandaid fix to me. The IProjectSetSerializer was not designed to support headless operations. The reason that the parameter is an Object and not a Shell is because the interface lives in Core. At the end of 2.1 we introduced a RepositoryProvideType class which we had intended as a future replacement for all the extension points we were creating (like IProjectSetSerializer). If headless operation for team operations is desirable, perhaps we need to look at a design that involves one Team core extension point (RepositoryProviderType) from which all provider related objects can be obtained (RepositoryProvider, IProjectSetSerializer, etc) and one UI extenstion point (RepositoryProviderUI?) per provider that gives access to all the needed core and ui pieces (IProjectReferenceHandler?). That way, a repository provider would only need to extend two extension points and could override behavior in subclass. Addressing issues raised in comment #2: A1) "I can't see what this buys you from a CVS standpoint since the CVS client in Eclipse does not work in headless mode" Correct, the CVS client does not work in headless mode, but I'm looking at placing the new CVSProjectReferenceSerializer somewhere in the org.eclipse.team.cvs.core plugin which has no UI references, and then indirectly using that class from the proposed Team.getProjectReferenceSerializer(...) method. Using this approach, I can write repository independent code that, using the proposed IProjectReferenceSerializer interface, adds a project to the workspace from a repository. The goal is to enhance the ProjectSets functionality (see bug 30626 and http://www.ejbprovider.de/homepage/index.html) so that it can be driven from a headless environment... e.g. an Ant task. A2) "You say that a repository provider could override the default IProjectReferenceHandler to provide custom prompting, etc. This would only work if the there was yet another extension point for the handler" Correct. The existing IProjectSetSerializer interface and existing Team.getProjectSetSerializer(...) method would both be deprecated in favor of the proposed interface and proposed Team method. As stated above, implementors of the proposed IProjectReferenceSerializer would register using a new TBD extension point. A3) "Given point 2, this seems like a bandaid fix to me. The IProjectSetSerializer was not designed to support headless operations." Correct, but the proposed IProjectReferenceSerializer would support headless operations. The existing interface, Team method, and extension point would exist only for backward compatibility, to be removed at some future date. The existing Project Set code that relies on the existing interface would, at some point, be rewritten to use the proposed interface. "The reason that the parameter is an Object and not a Shell is because the interface lives in Core." Sorry, but that smells like bad design to me. I'm assuming that there were reasons that I cannot see for not putting the existing IProjectSetSerializer, its related Team method, and the corresponding extension point in the Team Core plugin rather than the Team UI plugin where it appears it should reside. The goal of the proposed interface, method, and extension point is provide a cleaner access to this same functionality without any UI references so that it can be used in a headless environment. "At the end of 2.1 we introduced a RepositoryProviderType class which we had intended as a future replacement for all the extension points we were creating (like IProjectSetSerializer)." Interesting suggestion. If the functionality described in the proposed IProjectReferenceSerializer would be available via the RepositoryProviderType, then I would be all for it rather than the proposed interface. My goal is not to push the proposed interface, but rather to have access to the functionality described through some repository and vendor neutral public API. I would be perfectly happy if the RepositoryProviderType was enhanced with the ability to convert a project into an opaque string based project reference and then, in another workspace, pass that project reference to a RepositoryProviderType causing it to load that project into the workspace. You smell it too;-)? In retrospect, there were a couple of design problems with
this feature. The first is the fact that it was an interface instead of an
abstract class. This basically requires any future additions, such as the one
you are proposing, to be another extension point. The second is the separation
between core and UI. The following is a counter proposal for this (it's
actually derived from yours with some modifications based on past discussions
on this subject).
The following classes (not interfaces) would live in the org.eclipse.team.core
package:
RepositoryProviderType
ProjectSetSerializer getProjectSetSerializer()
- method which can be overridden by subclass, and returns the
project set serializer for the repository type
ProjectSetSerializer
String asReference(
IProject[] providerProjects,
ProjectSetSerializationContext context,
IProgressMonitor monitor) throws TeamException
void addToWorkspace(
String[] referenceStrings,
String filename,
ProjectSetSerializationContext context,
IProgressMonitor monitor) throws TeamException
DefaultProjectSetSerializer extends ProjectSetSerializer
- provides backwards compatible behavior by using IProjectSetSerializer
if one was registered with the extension point.
- is returned by deafult from RepositoryProviderType
ProjectSetSerializationContext
Object getShell()
- returns a shell if there is one or null if headless.
boolean[] (IProject[]?) abstract confirmOverwrite(IProject[] projects)
- must be overridden by sublcasses.
- I think it is cleaner to return the list of projects to be overwritten
instead of boolean[].
UIProjectSetSerializationContext (lives in org.eclipse.team.ui)
Object getShell()
- returns the shell
??? abstract confirmOverwrite(IProject[] projects)
- prompts to overwrite the given projects.
I still have several concerns about this proposal.
1. I understand your desire to add the confirmOverwrite method to confirm the
overwrite of the projects. It greatly simplifies the implemenation of the
ProjectSetSerializer for repository providers that only prompt for project
overwrite. My concern is that it leaves repository providers who need to do
more no better off then they are today.
2. Part of the 3.0 plan for Team is to investigate the possibility of
supporting a generic Team API. Work in this area may have an impact on the
project set serialization API (and an impact on concern 1 as well depending on
how the generic API is realized).
Given the above concerns, I'm not convinced that we should proceed on this at
this time. I would, at the very least, want to gather feedback from repository
providers on the subject, since they are the ones who are the most effected.
while we're on the way, I'd like to add one method to the proposed ProjectSetSerializationContext: String[] (File?) abstract getProjectLocations(IProject[] projects) - returns the locations where Projects should be added in the file system. This addresses a problem in https://bugs.eclipse.org/bugs/show_bug.cgi? id=30626. Though the problem forcing a new Project to a specific location is solved by first creating a dummy project at the desired location, this solution feels like a hack. Yet another method to be added to the proposed ProjectSetSerializationContext:
void reportErrors(String[] errorMessages)
or perhaps
void reportStatus(IStatus status)
so that the operation can forward error messages to the context and allow the
context to decide whether to display those errors (e.g. UI context) or log them
(e.g. headless context).
The goal is to reach a point where ProjectSetSerializationContext.getShell()
method is no longer needed.
(from comment #4):
"boolean[] (IProject[]?) abstract confirmOverwrite(IProject[] projects)
- must be overridden by sublcasses.
- I think it is cleaner to return the list of projects to be overwritten
instead of boolean[]."
Returning the array of projects that should be overwritten rather than an array
of booleans sounds good me. The default implementation could be to always
return an empty array indicating that no projects should ever be overwritten,
and subclasses could override this behavior.
Dan, given that this PR is getting very long could you please help out by summarizing in a document and providing a first cut reference implementation based on the given suggestions. For now, we should aim at being backwards compatible. There are still a couple of areas that we aren't sure with but we would rather have something for you to try then wait until the ideal solution suddenly appears :) It may be too late again, and not make it into 3.0. We are really busy for 3.0 and any help in this area would be great. I'm assigning for M2, meaning that we would like to release a first cut to HEAD for that milestone. ... in progress .... These next 4 patches contains the new serialization support, but do *not* address comment 4 point 1 above... see bug 39651. Created attachment 5348 [details]
new serializer support for team.core
Summary:
RepositoryProviderType # getProjectSetSerializer()
new method to return new serializer
ProjectSerializer
new class for project serialization
DefaultProjectSerializer
internal class returned by #getProjectSetSerializer above
for backward compatibility with IProjectSerializer
ProjectSerializerContext
base class for all serialization contexts
IProjectSetSerializer
deprecated
Team # getProjectSetSerializer(...)
deprecated
Created attachment 5349 [details]
new serializer support for team.ui
Summary:
UIProjectSerializerContext
suggested base for all UI serialization contexts
ProjectSetImportWizard
modified to use new serialization
ProjectSetExportWizard
modified to use new serialization
Created attachment 5350 [details]
new serializer support for CVS core
Summary:
CVSTeamProviderType # getProjectSerializer()
return new CVS specific serializer
CVSProjectSetSerializer
new CVS specific serializer
Created attachment 5351 [details]
new serializer support for CVS ui
Summary:
CVSProjectSetSerializer
deprecated
Thanks Dan, I've applied the patches and released to a branch called Bug_38048. They have been committed 'as is'. We should have time next week to have a quick look and then we can merge into HEAD. Again, thanks for helping us out with this. Code will remain in the branch for M2. While working on the user settings plan item (Bug 36965) we are trying to understand how team projects sets relate to user settings. They seem similar, a project set is a saved configuration of settings in the workspace. Why doesn't the project set contain other settings as well? Good point. Project Sets don't contain other information for historical reasons. I was striving for compatibility with the existing spec so I did not modify the file format. Given my choice, I would like to see Project Sets contain additional optional information such as preferred project location and java classpath variables. If project sets become shareable user settings and configuration of sharing through (shared) projects is supported, then things can get tricky. The project set could for example reference the project it is shared through in another version than loaded in the workspace. The problems to be solved by introducing this kind of self-reference might be beyond the intended scope of the user settings approach. Dan, I'm starting to have a look at this again since we are trying to finalize our 3.0 API. It turns out that nothing was released to the above mentioned branch. I would like to release it to a branch but before I can, any new files must have the proper copyrights. The Eclipse charter describes the licensing and copyright requirements for newly submitted files. Go to the bottom of the following link to read them. http://www.eclipse.org/eclipse/eclipse-charter.html Could you add the required copyright so I can commit the code to the repository? It will probably be easiest if you zip up the four patches in a single attachment. Thanks. Created attachment 7396 [details]
zip containing revised revised patch
Added copyright and recreated patch against HEAD as requested.
I have released the patches to branch_20040330_ProjectSetCapability with the following modifications: - I had forgotten in my original comments that we already had a ProjectSetCapability class which we intended for the same purpose as ProjectSetSerializer. Hence, I have move the behavior from ProjectSetSerializer to ProjectSetCapability. - I removed the project location code since we do not have time in 3.0 to add location specification to the project set import wizard. I want to do some testing before releasing the changes to HEAD. Fix released to HEAD |
Problem: IProjectSetSerializer methods have UI based arguments and thus cannot be used in a headless environment. Discussion: Both methods in the IProjectSetSerializer interface have an argument named "context" with the following javadoc * @param context a UI context object. This object will either be a * com.ibm.swt.widgets.Shell or it will be null. The context argument is of type Object so that there is no direct reference to UI specific types, but because of this context argument, the interface encourages implementors to build concrete classes implementing this interface that have UI dependencies and cannot be used in a headless environment. Specifically, the CVSProjectSetSerializer class, which implements the IProjectSetSerializer interface, is built using UI specific references. If the Team.getProjectSetSerializer(...) method is used in a headless environment with a CVS repository, an instance of CVSProjectSetSerializer will be created thus dragging in several UI specific plugins... not ideal for a headless environment. Proposal: Leave the current interface in place and create two new interfaces IProjectReferenceSerializer and IProjectReferenceHandler as shown below. A new Team.getProjectReferenceSerializer(...) method would be created with a new plugin extension point similar to the existing Team.getProjectSetSerializer(...) method and its plugin extension point. The existing IProjectSetSerializer interface and corresponding Team method could be restructured to call the new Team method and wrapper the result if an existing extension for the specified identifier is not available. This would result in zero changes to the existing ProjectSet code and no API breakage. The existing CVSProjectSetSerializer class would be discarded in favor of a new CVSProjectReferenceSerializer class implementing the new interface and using the new extension point. Repository providers that needed additional functionality via the IProjectReferenceHandler would create a subinterface specific for their own type of repository with additional methods, but would still be responsible for gracefully degrading operations if the handler provided does not implemented the repository specific interface. Summary: If the above proposal is accepted, I'll be happy to contribute code to bring these changes about. Interfaces discussed above: ----------------------------------------- package org.eclipse.team.core; import org.eclipse.core.resources.IProject; import org.eclipse.core.runtime.IProgressMonitor; /** * IProjectReferenceSerializer manages the serializing and deserializing * of references to projects. Given a project, it can produce a * UTF-8 encoded String which can be stored in a file. * Given this String, it can create in the workspace an IProject. * * @author Dan Rubel */ public interface IProjectReferenceSerializer { /** * For every IProject in providerProjects, return an opaque * UTF-8 encoded String to act as a reference to that project. * The format of the String is specific to the provider. * The format of the String must be such that * IProjectSetSerializer.addToWorskpace() will be able to * consume it and recreate a corresponding project. * * @see IProjectSetSerializer#addToWorkspace(String[] referenceStrings, String filename, IProjectReferenceHandler handler, IProgressMonitor monitor) * * @param providerProjects an array of projects that the serializer should create * text references for * @param handler a reference handler or null. * @param monitor a progress monitor * @return String[] an array of serialized reference strings uniquely identifying the projects * @throws TeamException */ String[] asReference(IProject[] providerProjects, IProjectReferenceHandler handler, IProgressMonitor monitor) throws TeamException; /** * For every String in referenceStrings, create in the workspace a * corresponding IProject. Return an Array of the resulting IProjects. * Result is unspecified in the case where an IProject of that name * already exists. In the case of failure, a TeamException must be thrown. * The opaque strings in referenceStrings are guaranteed to have been previously * produced by IProjectSetSerializer.asReference(). * * @see IProjectSetSerializer#asReference(IProject[] providerProjects, IProjectReferenceHandler handler, IProgressMonitor monitor) * * @param referenceStrings an array of referene strings uniquely identifying the projects * @param filename the name of the file that the references were read from. This is included * in case the provider needs to deduce relative paths * @param handler a reference handler or null. * @param monitor a progress monitor * @return IProject[] an array of projects that were created * @throws TeamException */ IProject[] addToWorkspace(String[] referenceStrings, String filename, IProjectReferenceHandler handler, IProgressMonitor monitor) throws TeamException; } ----------------------------------------- package org.eclipse.team.core; import org.eclipse.core.resources.IProject; /** * A callback interface for processing issues * that occur in IProjectReferenceSerializer. * * @author Dan Rubel */ public interface IProjectReferenceHandler { /** * A call back during the IProjectReferenceSerializer.addToWorkspace method * used if projects to be added already exist in the workspace. * * @see IProjectReferenceSerializer#addToWorkspace(String[], String, IProjectReferenceHandler, IProgressMonitor) * * @param projects an array of projects that will be overwritten by this process * (not <code>null</code>, contains no <code>null</code>s) * @return an array of booleans (not <code>null</code>) * parallel to the projects array argument * indicating which projects should be overwritten (<code>true</code>) * and which projects should not (<code>false</code>). * @throws TeamException */ boolean[] confirmOverwrite(IProject[] projects) throws TeamException; }