git-p4 migrates perforce “main” branch into git branches as subdirectories (doubled code in git branches)
Using git p4 on git version 1.7.12.rc2 has path issues. Standard clone/sync ops apparently place detected master and branches on independent and parallel directory structures instead of git branches. See http://stackoverflow.com/q/11893688/1588831 for a full demo of the problem. Thanks,
-Matt Korich
cover git p4: fix branch detection with --use-client-spec
[email protected] wrote on Fri, 10 Aug 2012 12:14 -0700:
Quoted 4 lines
Using git p4 on git version 1.7.12.rc2 has path issues. Standard clone/sync ops apparently place detected master and branches on independent and parallel directory structures instead of git branches. See http://stackoverflow.com/q/11893688/1588831 for a full demo of the problem.
Thank you for the detailed report. It is a bug in 1.7.12-rc2. This series fixes it, on top of origin/master.
The crux of the matter is that files are mapped into the wrong locations in git when both --use-client-spec and --branch-detection are enabled.
Pete Wyckoff (5):
git p4 test: move client_view() function to library git p4 test: add broken --use-client-spec --detect-branches tests git p4: set self.branchPrefixes in initialization
git p4: do wildcard decoding in stripRepoPath
git p4: make branch detection work with --use-client-spec
git-p4.py | 75 +++++++++++++++++++++++++++-------------- t/lib-git-p4.sh | 18 ++++++++++ t/t9801-git-p4-branch.sh | 77 +++++++++++++++++++++++++++++++++++++++++++ t/t9809-git-p4-client-view.sh | 17 ---------- 4 files changed, 146 insertions(+), 41 deletions(-)
Signature · 1 line
1.7.12.rc2.24.gc304662
PATCH 1/5 git p4 test: move client_view() function to library
This code will be useful in --detect-branches --use-client-spec tests.
--- t/lib-git-p4.sh | 18 ++++++++++++++++++ t/t9809-git-p4-client-view.sh | 17 ----------------- 2 files changed, 18 insertions(+), 17 deletions(-)
t/lib-git-p4.sh +18 -0
@@ -115,3 +115,21 @@ marshal_dump() { EOF "$PYTHON_PATH" "$TRASH_DIRECTORY/marshal-dump.py" } + +# +# Construct a client with this list of View lines +# +client_view() { + ( + cat <<-EOF && + Client: client + Description: client + Root: $cli + View: + EOF + for arg ; do + printf "\t$arg\n" + done + ) | p4 client -i +} +
t/t9809-git-p4-client-view.sh +0 -17
@@ -9,23 +9,6 @@ test_expect_success 'start p4d' ' ' # -# Construct a client with this list of View lines -# -client_view() { - ( - cat <<-EOF && - Client: client - Description: client - Root: $cli - View: - EOF - for arg ; do - printf "\t$arg\n" - done - ) | p4 client -i -} - -# # Verify these files exist, exactly. Caller creates # a list of files in file "files". #
Signature · 1 line
1.7.12.rc2.24.gc304662
PATCH 2/5 git p4 test: add broken --use-client-spec --detect-branches tests
In reply to Pete Wyckoff--- t/t9801-git-p4-branch.sh | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+)
t/t9801-git-p4-branch.sh +77 -0
@@ -410,6 +410,83 @@ test_expect_failure 'git p4 clone file subset branch' ' test_path_is_missing file3 ) ' + +# From a report in http://stackoverflow.com/questions/11893688 +# where --use-client-spec caused branch prefixes not to be removed; +# every file in git appeared into a subdirectory of the branch name. +test_expect_success 'use-client-spec detect-branches setup' ' + rm -rf "$cli" && + mkdir "$cli" && + ( + cd "$cli" && + client_view "//depot/usecs/... //client/..." && + mkdir b1 && + echo b1/b1-file1 >b1/b1-file1 && + p4 add b1/b1-file1 && + p4 submit -d "b1/b1-file1" && + + p4 integrate //depot/usecs/b1/... //depot/usecs/b2/... && + p4 submit -d "b1 -> b2" && + p4 branch -i <<-EOF && + Branch: b2 + View: //depot/usecs/b1/... //depot/usecs/b2/... + EOF + + echo b2/b2-file2 >b2/b2-file2 && + p4 add b2/b2-file2 && + p4 submit -d "b2/b2-file2" + ) +' + +test_expect_failure 'use-client-spec detect-branches files in top-level' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git p4 sync --detect-branches --use-client-spec //depot/usecs@all && + git checkout -b master p4/usecs/b1 && + test_path_is_file b1-file1 && + test_path_is_missing b2-file2 && + test_path_is_missing b1 && + test_path_is_missing b2 && + + git checkout -b b2 p4/usecs/b2 && + test_path_is_file b1-file1 && + test_path_is_file b2-file2 && + test_path_is_missing b1 && + test_path_is_missing b2 + ) +' + +test_expect_success 'use-client-spec detect-branches skips branches setup' ' + ( + cd "$cli" && + + p4 integrate //depot/usecs/b1/... //depot/usecs/b3/... && + p4 submit -d "b1 -> b3" && + p4 branch -i <<-EOF && + Branch: b3 + View: //depot/usecs/b1/... //depot/usecs/b3/... + EOF + + echo b3/b3-file3 >b3/b3-file3 && + p4 add b3/b3-file3 && + p4 submit -d "b3/b3-file3" + ) +' + +test_expect_success 'use-client-spec detect-branches skips branches' ' + client_view "//depot/usecs/... //client/..." \ + "-//depot/usecs/b3/... //client/b3/..." && + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git p4 sync --detect-branches --use-client-spec //depot/usecs@all && + test_must_fail git rev-parse refs/remotes/p4/usecs/b3 + ) +' + test_expect_success 'kill p4d' ' kill_p4d '
Signature · 1 line
1.7.12.rc2.24.gc304662
PATCH 3/5 git p4: set self.branchPrefixes in initialization
In reply to Pete WyckoffThis instance variable is needed during commit() to map files from p4 to their relative locations in git. Set it when initializing P4Sync to avoid passing it to every commit() call.
--- git-p4.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
git-p4.py +14 -11
@@ -2041,10 +2041,9 @@ class P4Sync(Command, P4UserMap): gitStream.write(description) gitStream.write("\n") - def commit(self, details, files, branch, branchPrefixes, parent = ""): + def commit(self, details, files, branch, parent = ""): epoch = details["time"] author = details["user"] - self.branchPrefixes = branchPrefixes if self.verbose: print "commit into %s" % branch
@@ -2053,7 +2052,7 @@ class P4Sync(Command, P4UserMap): # create a commit. new_files = [] for f in files: - if [p for p in branchPrefixes if p4PathStartsWith(f['path'], p)]: + if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], p)]: new_files.append (f) else: sys.stderr.write("Ignoring file outside of prefix: %s\n" % f['path'])
@@ -2070,8 +2069,8 @@ class P4Sync(Command, P4UserMap): self.gitStream.write("data <<EOT\n") self.gitStream.write(details["desc"]) - self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s" - % (','.join (branchPrefixes), details["change"])) + self.gitStream.write("\n[git-p4: depot-paths = \"%s\": change = %s" % + (','.join(self.branchPrefixes), details["change"])) if len(details['options']) > 0: self.gitStream.write(": options = %s" % details['options']) self.gitStream.write("]\nEOT\n\n")
@@ -2094,7 +2093,7 @@ class P4Sync(Command, P4UserMap): print "Change %s is labelled %s" % (change, labelDetails) files = p4CmdList(["files"] + ["%s...@%s" % (p, change) - for p in branchPrefixes]) + for p in self.branchPrefixes]) if len(files) == len(labelRevisions):
@@ -2405,6 +2404,7 @@ class P4Sync(Command, P4UserMap): for branch in branches.keys(): ## HACK --hwn branchPrefix = self.depotPaths[0] + branch + "/" + self.branchPrefixes = [ branchPrefix ] parent = ""
@@ -2449,19 +2449,19 @@ class P4Sync(Command, P4UserMap): tempBranch = os.path.join(self.tempBranchLocation, "%d" % (change)) if self.verbose: print "Creating temporary branch: " + tempBranch - self.commit(description, filesForCommit, tempBranch, [branchPrefix]) + self.commit(description, filesForCommit, tempBranch) self.tempBranches.append(tempBranch) self.checkpoint() blob = self.searchParent(parent, branch, tempBranch) if blob: - self.commit(description, filesForCommit, branch, [branchPrefix], blob) + self.commit(description, filesForCommit, branch, blob) else: if self.verbose: print "Parent of %s not found. Committing into head of %s" % (branch, parent) - self.commit(description, filesForCommit, branch, [branchPrefix], parent) + self.commit(description, filesForCommit, branch, parent) else: files = self.extractFilesFromCommit(description) - self.commit(description, files, self.branch, self.depotPaths, + self.commit(description, files, self.branch, self.initialParent) self.initialParent = "" except IOError:
@@ -2525,7 +2525,7 @@ class P4Sync(Command, P4UserMap): self.updateOptionDict(details) try: - self.commit(details, self.extractFilesFromCommit(details), self.branch, self.depotPaths) + self.commit(details, self.extractFilesFromCommit(details), self.branch) except IOError: print "IO error with git fast-import. Is your git version recent enough?" print self.gitError.read()
@@ -2683,6 +2683,9 @@ class P4Sync(Command, P4UserMap): self.depotPaths = newPaths + # --detect-branches may change this for each branch + self.branchPrefixes = self.depotPaths + self.loadUserMapFromCache() self.labels = {} if self.detectLabels:
Signature · 1 line
1.7.12.rc2.24.gc304662
PATCH 4/5 git p4: do wildcard decoding in stripRepoPath
In reply to Pete WyckoffInstead of having to remember to do it after each call to stripRepoPath, make it part of that function.
--- git-p4.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
git-p4.py +8 -9
@@ -1819,15 +1819,17 @@ class P4Sync(Command, P4UserMap): def stripRepoPath(self, path, prefixes): if self.useClientSpec: - return self.clientSpecDirs.map_in_client(path) + path = self.clientSpecDirs.map_in_client(path) - if self.keepRepoPath: - prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])] + else: + if self.keepRepoPath: + prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])] - for p in prefixes: - if p4PathStartsWith(path, p): - path = path[len(p):] + for p in prefixes: + if p4PathStartsWith(path, p): + path = path[len(p):] + path = wildcard_decode(path) return path def splitFilesIntoBranches(self, commit):
@@ -1849,7 +1851,6 @@ class P4Sync(Command, P4UserMap): fnum = fnum + 1 relPath = self.stripRepoPath(path, self.depotPaths) - relPath = wildcard_decode(relPath) for branch in self.knownBranches.keys():
@@ -1867,7 +1868,6 @@ class P4Sync(Command, P4UserMap): def streamOneP4File(self, file, contents): relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes) - relPath = wildcard_decode(relPath) if verbose: sys.stderr.write("%s\n" % relPath)
@@ -1936,7 +1936,6 @@ class P4Sync(Command, P4UserMap): def streamOneP4Deletion(self, file): relPath = self.stripRepoPath(file['path'], self.branchPrefixes) - relPath = wildcard_decode(relPath) if verbose: sys.stderr.write("delete %s\n" % relPath) self.gitStream.write("D %s\n" % relPath)
Signature · 1 line
1.7.12.rc2.24.gc304662
PATCH 5/5 git p4: make branch detection work with --use-client-spec
In reply to Pete WyckoffThe bug report in http://stackoverflow.com/questions/11893688
observes that files are mapped into the wrong locations in git when both --use-client-spec and --branch-detection are enabled.
Fix this by changing the relative path prefix to match discovered branches when using a client spec.
The problem was likely introduced with ecb7cf9 (git-p4: rewrite view handling, 2012-01-02).
--- git-p4.py | 37 +++++++++++++++++++++++++++++++------ t/t9801-git-p4-branch.sh | 2 +- 2 files changed, 32 insertions(+), 7 deletions(-)
git-p4.py +31 -6
@@ -1818,21 +1818,41 @@ class P4Sync(Command, P4UserMap): return files def stripRepoPath(self, path, prefixes): + """When streaming files, this is called to map a p4 depot path + to where it should go in git. The prefixes are either + self.depotPaths, or self.branchPrefixes in the case of + branch detection.""" + if self.useClientSpec: + # branch detection moves files up a level (the branch name) + # from what client spec interpretation gives path = self.clientSpecDirs.map_in_client(path) + if self.detectBranches: + for b in self.knownBranches: + if path.startswith(b + "/"): + path = path[len(b)+1:] + + elif self.keepRepoPath: + # Preserve everything in relative path name except leading + # //depot/; just look at first prefix as they all should + # be in the same depot. + depot = re.sub("^(//[^/]+/).*", r'\1', prefixes[0]) + if p4PathStartsWith(path, depot): + path = path[len(depot):] else: - if self.keepRepoPath: - prefixes = [re.sub("^(//[^/]+/).*", r'\1', prefixes[0])] - for p in prefixes: if p4PathStartsWith(path, p): path = path[len(p):] + break path = wildcard_decode(path) return path def splitFilesIntoBranches(self, commit): + """Look at each depotFile in the commit to figure out to what + branch it belongs.""" + branches = {} fnum = 0 while commit.has_key("depotFile%s" % fnum):
@@ -1850,11 +1870,16 @@ class P4Sync(Command, P4UserMap): file["type"] = commit["type%s" % fnum] fnum = fnum + 1 - relPath = self.stripRepoPath(path, self.depotPaths) + # start with the full relative path where this file would + # go in a p4 client + if self.useClientSpec: + relPath = self.clientSpecDirs.map_in_client(path) + else: + 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 + # 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 branch not in branches: branches[branch] = []
t/t9801-git-p4-branch.sh +1 -1
@@ -438,7 +438,7 @@ test_expect_success 'use-client-spec detect-branches setup' ' ) ' -test_expect_failure 'use-client-spec detect-branches files in top-level' ' +test_expect_success 'use-client-spec detect-branches files in top-level' ' test_when_finished cleanup_git && test_create_repo "$git" && (
Signature · 1 line
1.7.12.rc2.24.gc304662
Re: [PATCH 0/5] git p4: fix branch detection with --use-client-spec
In reply to Pete WyckoffQuoted 7 lines · Pete Wyckoff
[email protected] wrote on Fri, 10 Aug 2012 12:14 -0700:
Quoted 4 lines
Using git p4 on git version 1.7.12.rc2 has path issues. Standard clone/sync ops apparently place detected master and branches on independent and parallel directory structures instead of git branches. See http://stackoverflow.com/q/11893688/1588831 for a full demo of the problem.
Thank you for the detailed report. It is a bug in 1.7.12-rc2.
Do you mean "a feature that was present in 1.7.11 without this bug was broken when used with 1.7.12-rc2"? Or do you mean "this bug exists in 1.7.12-rc2 (older versions may or may not have it, but I am stressing that it is not fixed)"?
The description for [PATCH 5/5] blames v1.7.9-rc0~4^2~1, which tells me it is the latter. And if that were the case, and if this were in the area of the system I oversee, I wouldn't push it to the upcoming release at this late in the cycle, when I do not know what other things it might break while fixing this bug (in other words, a fix to an old bug is not an execuse to introduce a regression).
But git-p4 is not in my area, so if you meant this should go in the upcoming 1.7.12 release, I'll queue them directly on 'master'.
Please tell me what your preference is.
Thanks.
Quoted 18 lines
This series fixes it, on top of origin/master.
The crux of the matter is that files are mapped into the wrong locations in git when both --use-client-spec and --branch-detection are enabled.
Pete Wyckoff (5):
git p4 test: moveclient_view()function to library git p4 test: add broken --use-client-spec --detect-branches tests git p4: set self.branchPrefixes in initialization
git p4: do wildcard decoding in stripRepoPath
git p4: make branch detection work with --use-client-specgit-p4.py | 75 +++++++++++++++++++++++++++-------------- t/lib-git-p4.sh | 18 ++++++++++ t/t9801-git-p4-branch.sh | 77 +++++++++++++++++++++++++++++++++++++++++++ t/t9809-git-p4-client-view.sh | 17 ---------- 4 files changed, 146 insertions(+), 41 deletions(-)
Re: [PATCH 0/5] git p4: fix branch detection with --use-client-spec
[email protected] wrote on Sat, 11 Aug 2012 21:41 -0700:
Quoted 22 lines
Quoted 7 lines · Pete Wyckoff
[email protected] wrote on Fri, 10 Aug 2012 12:14 -0700:
Quoted 4 lines
Using git p4 on git version 1.7.12.rc2 has path issues. Standard clone/sync ops apparently place detected master and branches on independent and parallel directory structures instead of git branches. See http://stackoverflow.com/q/11893688/1588831 for a full demo of the problem.
Thank you for the detailed report. It is a bug in 1.7.12-rc2.
Do you mean "a feature that was present in 1.7.11 without this bug was broken when used with 1.7.12-rc2"? Or do you mean "this bug exists in 1.7.12-rc2 (older versions may or may not have it, but I am stressing that it is not fixed)"?
The description for [PATCH 5/5] blames v1.7.9-rc0~4^2~1, which tells me it is the latter. And if that were the case, and if this were in the area of the system I oversee, I wouldn't push it to the upcoming release at this late in the cycle, when I do not know what other things it might break while fixing this bug (in other words, a fix to an old bug is not an execuse to introduce a regression).
Quoted 4 lines
But git-p4 is not in my area, so if you meant this should go in the upcoming 1.7.12 release, I'll queue them directly on 'master'.
Please tell me what your preference is.
Good point about "already released bugs". I confirmed it was broken in 1.7.11 too, so there's no reason to rush this fix into 1.7.12. If you could queue it up in pu, that would be great. Otherwise I'll resubmit after the upcoming release.
Thanks,
-- Pete
Re: [PATCH 0/5] git p4: fix branch detection with --use-client-spec
Quoted 7 lines · Pete Wyckoff
Quoted 7 lines
The description for [PATCH 5/5] blames v1.7.9-rc0~4^2~1, which tells me it is the latter. And if that were the case, and if this were in the area of the system I oversee, I wouldn't push it to the upcoming release at this late in the cycle, when I do not know what other things it might break while fixing this bug (in other words, a fix to an old bug is not an execuse to introduce a regression).
Quoted 8 lines
Quoted 4 lines
But git-p4 is not in my area, so if you meant this should go in the upcoming 1.7.12 release, I'll queue them directly on 'master'.
Please tell me what your preference is.
Good point about "already released bugs". I confirmed it was broken in 1.7.11 too, so there's no reason to rush this fix into 1.7.12. If you could queue it up in pu, that would be great.
OK, I've done so already last night when I wrote my message but haven't pushed the result out yet.
Note (primarily to other people) that the above exchange does not mean "a fix to an old bug is automatically disqualified during the rc freeze period". If the fix is so focused and obvious that there is no way the change inadvertently and negatively affects other code and introduce a new bug, it is perfectly fine to apply the fix any time. I don't know git-p4 well enough to tell if this five patch series was in that "obviously safe" category myself, so I asked Pete to decide it for me.
Thanks.