Bugzilla – Full Text Bug Listing
| Summary: | [server] Git URIs should be revisited | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Szymon Brandys <Szymon.Brandys> | ||||||
| Component: | Client | Assignee: | Tomasz Zarna <tomasz.zarna> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | john.arthorne, libingw | ||||||
| Version: | 0.2 | ||||||||
| Target Milestone: | 0.2 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Szymon Brandys
The problem with URIs like /git/diff/[repositoryId]/[path2GitResource] is that the clone has to be added first. We already can link folders to existing Git repositories without cloning them explicitly (John's auto-clone or a pre-created clone in the allowedPrefixes area) and these clones do not have auto generated ids. Another idea would be to use some sort of server locations, e.g.: /git/diff/[virtualLocation] or git/clone/[virtualLocation] where virtualLocation is something like /userarea/[userId]/[pathInUserArea] or /workspace/[pathInWorkspace] It would allow to address any repository no matter how it was created on the server. (In reply to comment #1) > The problem with URIs like /git/diff/[repositoryId]/[path2GitResource] is that > the clone has to be added first. We already can link folders to existing Git > repositories without cloning them explicitly (John's auto-clone or a pre-created > clone in the allowedPrefixes area) and these clones do not have auto generated > ids. We could try creating/looking for a WebClone (with an ID) for auto-clones and clones created outside Orion each time GitUtils.getGitDir[1] is called, but this it doesn't sound like a proper solution to me. [1] org.eclipse.orion.server.git.servlets.GitUtils.getGitDir(IPath, String) (In reply to comment #1) > Another idea would be to use some sort of server locations, e.g.: > /git/diff/[virtualLocation] or git/clone/[virtualLocation] where > virtualLocation is something like /userarea/[userId]/[pathInUserArea] or > /workspace/[pathInWorkspace] The workspace path isn't enough because there may be clones that have not been linked to any projects yet. I also don't like this general approach because it encourages clients to hand-construct URIs to reach into server paths they should not even know about. Clients should only ever be able to refer to a clone by following a link, such as your CommitLocation, StatusLocation, etc. Those links are constructed on the server. Therefore, at the time we construct the link we can ensure there is a WebClone associated with it, and use that clone's id in the link. The server maintains a mapping from clone id to the server path of that clone. This is a valuable abstraction because it means we are not exposing the details of the server file system layout to the client via the URIs. We could easily move those clones to another disk location later without breaking any client links. Therefore I suggest going with your original proposal. URIs look like /git/diff/[cloneId]/[path2File]. > We could try creating/looking for a WebClone (with an ID) for auto-clones and > clones created outside Orion each time GitUtils.getGitDir[1] is called, but > this it doesn't sound like a proper solution to me. Yes that doesn't seem like quite the right place for it. There are only specific places where we either create or discover a clone, where we would need to ensure a WebClone exists, and allocate a clone id if necessary. - During a clone operation - After project creation (if the project location contains a git repository, or if we decide to auto-create one) For any API that operates on particular commits, we would likely need to use the commit hash in the URI to identify it. For example, a diff between two particular commits /git/diff/commits/8c68900b4621d43c699e273ccbd9ba0b0a15b03b/27730cc23ae8143273be716aac3014e9a2d8c40a Requesting the log between two commits: /git/log/commits/8c68900b4621d43c699e273ccbd9ba0b0a15b03b/27730cc23ae8143273be716aac3014e9a2d8c40a (In reply to comment #3) > (In reply to comment #1) > > Another idea would be to use some sort of server locations, e.g.: > > /git/diff/[virtualLocation] or git/clone/[virtualLocation] where > > virtualLocation is something like /userarea/[userId]/[pathInUserArea] or > > /workspace/[pathInWorkspace] > > The workspace path isn't enough because there may be clones that have not been > linked to any projects yet. I also don't like this general approach because it > encourages clients to hand-construct URIs to reach into server paths they should > not even know about. I think I was not clear. 'virtualLocation' is a server location. Since we don't want to expose full server paths like 'c:/something/', I thought we may use aliases for locations reachable by end users. So /workspace/something would be translated to c:/workspaces/foo/something. This location does not have to be added to the Orion Workspace as a project or folder. BTW, there is the same issue with linking server locations to the Orion workspace. I don't think that linking them using real server locations is the right thing. Instead of linking a folder to c:/path/allowedLocation/someLocation/onServer, we should link it to some virtual path on the server e.g. /allowedLocation/somelocation/onServer > Clients should only ever be able to refer to a clone by following a link, such > as your CommitLocation, StatusLocation, etc. Those links are constructed on the > server. Therefore, at the time we construct the link we can ensure there is a > WebClone associated with it, and use that clone's id in the link. The server > maintains a mapping from clone id to the server path of that clone. This is a > valuable abstraction because it means we are not exposing the details of the > server file system layout to the client via the URIs. We could easily move those > clones to another disk location later without breaking any client links. > > Therefore I suggest going with your original proposal. URIs look like > /git/diff/[cloneId]/[path2File]. We just collected some possible approaches in the bug. I like the original proposal more too. We will use URIs like /git/diff/[cloneId]/[path2File] and for auto-created clones we generate cloneId. > For any API that operates on particular commits, we would likely need to use the > commit hash in the URI to identify it. For example, a diff between two > particular commits > > /git/diff/commits/8c68900b4621d43c699e273ccbd9ba0b0a15b03b/27730cc23ae8143273be716aac3014e9a2d8c40a > > Requesting the log between two commits: > > /git/log/commits/8c68900b4621d43c699e273ccbd9ba0b0a15b03b/27730cc23ae8143273be716aac3014e9a2d8c40a That's a different story. This is more complicated since you may want to compare two commits for a file, or two different commits for two different files. Your suggestion does not cover these cases. Thanks John. (In reply to comment #3) > Therefore I suggest going with your original proposal. URIs look like > /git/diff/[cloneId]/[path2File]. Ok, I'll go for that making sure a WebClone exists even when a project has been linked to a git repository or the repository has been auto-created. > a diff between two particular commits > /git/diff/commits/8c68900b4621d43c699e273ccbd9ba0b0a15b03b/27730cc23ae8143273be716aac3014e9a2d8c40a see bug 339682 > Requesting the log between two commits: > /git/log/commits/8c68900b4621d43c699e273ccbd9ba0b0a15b03b/27730cc23ae8143273be716aac3014e9a2d8c40a see bug 339104 Created attachment 191408 [details]
Fix v01
Created attachment 191409 [details]
mylyn/context/zip
The fix is ready (see comment 6), but before releasing it, I would like to make sure everyone is prepared for the change (http://dev.eclipse.org/mhonarc/lists/orion-dev/msg00324.html). Fixed with http://git.eclipse.org/c/e4/org.eclipse.orion.server.git/commit/?id=91e8db2c2df1d1dbd95ac1e714a19477f637fc2d. Repositories can be now cloned into a folder in workspace. The URI has also changed a bit e.g.: /git/clones/workspace/{workspaceId} will return all clones in the workspace, /git/clones/file/{projectId}/folder will return all clones kept under the folder in the project. Creating clones accepts now file paths in the following form "/file/{projectId}[/{path}]. I will update the doc as soon as possible. See bug 345367 comment 0 for more details. |