| Message ID | e644a8ab4928349ed83ac9ab6ffdbcafc3a3a7b5.1553207234.git.amazo@checkvideo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-p4: a few assorted fixes for branches, excludes | expand |
On Thu, 21 Mar 2019 at 22:32, Mazo, Andrey <amazo@checkvideo.com> wrote: > > git-p4 knows how to handle case insensitivity in file paths > if core.ignorecase is set. > However, when determining a branch for a file, > it still does a case-sensitive prefix match. > This may result in some file changes to be lost on import. > > For example, given the following commits > 1. add //depot/main/file1 > 2. add //depot/DirA/file2 > 3. add //depot/dira/file3 > 4. add //depot/DirA/file4 > and "branchList = main:DirA" branch mapping, > commit 3 will be lost. > > So, do branch search case insensitively if running with core.ignorecase set. > Teach splitFilesIntoBranches() to use the p4PathStartsWith() function > for path prefix matches instead of always case-sensitive match. I wonder what other code paths break due to this problem! Looks reasonable but I fear there may be some other holes in there - quickly looking through the code suggests there are several other places this problem occurs. Luke > > Signed-off-by: Andrey Mazo <amazo@checkvideo.com> > --- > git-p4.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/git-p4.py b/git-p4.py > index c0a3068b6f..91c610f960 100755 > --- a/git-p4.py > +++ b/git-p4.py > @@ -2721,11 +2721,11 @@ def splitFilesIntoBranches(self, commit): > relPath = self.stripRepoPath(path, self.depotPaths) > > for branch in self.knownBranches.keys(): > # add a trailing slash so that a commit into qt/4.2foo > # doesn't end up in qt/4.2, e.g. > - if relPath.startswith(branch + "/"): > + if p4PathStartsWith(relPath, branch + "/"): > if branch not in branches: > branches[branch] = [] > branches[branch].append(file) > break > > -- > 2.19.2 >
From: "Mazo, Andrey" <amazo@checkvideo.com> 23.03.2019, 05:16, "Luke Diamand" <luke@diamand.org>: > On Thu, 21 Mar 2019 at 22:32, Mazo, Andrey <amazo@checkvideo.com> wrote: >> git-p4 knows how to handle case insensitivity in file paths >> if core.ignorecase is set. >> However, when determining a branch for a file, >> it still does a case-sensitive prefix match. >> This may result in some file changes to be lost on import. >> >> For example, given the following commits >> 1. add //depot/main/file1 >> 2. add //depot/DirA/file2 >> 3. add //depot/dira/file3 >> 4. add //depot/DirA/file4 >> and "branchList = main:DirA" branch mapping, >> commit 3 will be lost. >> >> So, do branch search case insensitively if running with core.ignorecase set. >> Teach splitFilesIntoBranches() to use the p4PathStartsWith() function >> for path prefix matches instead of always case-sensitive match. > > I wonder what other code paths break due to this problem! > > Looks reasonable but I fear there may be some other holes in there - > quickly looking through the code suggests there are several other > places this problem occurs. From a quick search for .startswith(), I only see that stripRepoPath() might have a similar problem in useclientspec case. If you see other apparent problematic places, could you, please, point them out? Or let me try to come up with a test case, and see what other places break. Thank you, Andrey.
diff --git a/git-p4.py b/git-p4.py index c0a3068b6f..91c610f960 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2721,11 +2721,11 @@ def splitFilesIntoBranches(self, commit): relPath = self.stripRepoPath(path, self.depotPaths) for branch in self.knownBranches.keys(): # add a trailing slash so that a commit into qt/4.2foo # doesn't end up in qt/4.2, e.g. - if relPath.startswith(branch + "/"): + if p4PathStartsWith(relPath, branch + "/"): if branch not in branches: branches[branch] = [] branches[branch].append(file) break
git-p4 knows how to handle case insensitivity in file paths if core.ignorecase is set. However, when determining a branch for a file, it still does a case-sensitive prefix match. This may result in some file changes to be lost on import. For example, given the following commits 1. add //depot/main/file1 2. add //depot/DirA/file2 3. add //depot/dira/file3 4. add //depot/DirA/file4 and "branchList = main:DirA" branch mapping, commit 3 will be lost. So, do branch search case insensitively if running with core.ignorecase set. Teach splitFilesIntoBranches() to use the p4PathStartsWith() function for path prefix matches instead of always case-sensitive match. Signed-off-by: Andrey Mazo <amazo@checkvideo.com> --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)