branch of subversion files fix
leop - January 30, 2008 - 13:42
| Project: | Subversion |
| Version: | 5.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Jump to:
Description
The function subversion_log_parser assumes that the first part of the path of each file contains the branch (i.e. "/branches/myproject/myfile"). This is not necessarily the case. A better way to determine whether a file is in branches, tags or trunk is to check whether part of the path of a file is in one of the branches, tags or trunk paths specified in a project. Part of this was already implemented in subversion_log_parser, so we have to change only a little bit of code. The code for the case 'path' in function tag_end in subversion_log_parser has to be changed in:
<?php
case 'path':
/** FIXME: only do by rid */
$this->current_path->file = trim($this->data);
$pid = null;
$fields = array("trunk_path", "branch_path", "tag_path");
$possible_tags = array("trunk", "branches", "tags");
for ($i = 0; $i < count($fields); $i++) {
$field = $fields[$i];
$pid = db_result(db_query("SELECT nid FROM {subversion_projects} WHERE rid=%d AND '%s' LIKE CONCAT($field, '%%') ORDER BY LENGTH('%s')-LENGTH($field) LIMIT 1",
$this->rid,
$this->current_path->file,
$this->current_path->file
));
$this->current_path->branch = NULL;
if ($pid) {
$this->current_path->branch = $possible_tags[$i];
break;
}
}
$this->current_path->pid = $pid;
$this->current_item->files[] = $this->current_path;
unset($this->current_path);
break;
?>
#1
I will look more into this. I would like to pass in the possible tags/branches into the parser instead of having the parser do the queries.
Do you think the "/path/to/branch" is enough? or should we have a regular expression? /path/to/branch/(\w+) type thing.
#2
In my opinion the module should not make assumptions on how a repository is structured. For example, some people have branches, trunk and tags directories at the root of their repository, and put project directories in each of branches, trunk and tags, e.g.
/tag/myproject. Others might have project directories at the root of their repository and put branches, trunk and tag directories in each project directory, e.g./myproject/tags. The code I wrote only checks whether one of the words branches, trunk or tags is in the path, and does not make any assumptions on how the repository is structured. Of course this will cause problems when part of the project directory name or filename also contains one of the words branches, trunk or tags, e.g./metagsetproject/branches. A solution to this would be to ask the user how the repository is structured, in the form of a general pattern, e.g./%projectname/%branch, and use this to determine to what branch a directory or file belongs. Note that even the words branches, trunk and tags are not imposed by Subversion, you are free to use your own terms. Asking the user how the repository is structured would deal with that too.#3
In my (still-not-ready) SVN backend for Version Control API, I plan to do a regexp for each kind. Very similar to what leo_pape thought of, only that at least the admin UI (not the matching algorithm, though) has already been in drupal.org CVS for a few months. It goes approximately like this:
/%project/trunk. %project will be translated to([^/]*)and preg_match will check if the start of a path matches the resulting regexp. That way, we can determine both the fact that we're in trunk and the project name too./%project/branches/%branch./%project/tags/%branch/%tag. %branch doesn't matter a lot here, but some repositories also include it in their repository layout. (I find that to be a rather good idea, even.)With this approach, we can not only find out whether a path belongs to trunk, branches or tags but also map it to the other two ones, which might come in handy for repository browsing later on. I cannot, however, distinguish between working braches and stable ones (maybe those two should be separate path settings?). Multiple sub-layouts can't be told too, but that's quite an edge case I guess.