git-p4: a few assorted fixes for branches, excludes
cover git-p4: a few assorted fixes for branches, excludes
This series fixes a couple of corner cases with branch detection and handling of excludes by git-p4.
Andrey Mazo (5):
git-p4: detect/prevent infinite loop in gitCommitByP4Change()
git-p4: match branches case insensitively if configured
git-p4: don't groom exclude path list on every commit
git-p4: add failing test for "don't exclude other files with same prefix"
git-p4: don't exclude other files with same prefix
git-p4.py | 39 +++++++++++++++++++----------- t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 19 deletions(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Signature · 1 line
2.19.2
PATCH 1/5 git-p4: detect/prevent infinite loop in gitCommitByP4Change()
Under certain circumstances, gitCommitByP4Change() can enter an infinite loop resulting in git p4 sync hanging forever.
The problem is thatgit rev-list --bisect <latest> ^<earliest> can return <latest>, which would result in reinspecting <latest> and potentially an infinite loop.
This can happen when importing just a subset of P4 repository and/or with explicit "--changesfile" option.
A real-life example:
"""
looking in ref refs/remotes/p4/mybranch for change 26894 using bisect...
Reading pipe: git rev-parse refs/remotes/p4/mybranch
trying: earliest latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git cat-file commit 147f5d3292af2e1cc4a56a7b96db845144c68486
current change 25339
trying: earliest ^147f5d3292af2e1cc4a56a7b96db845144c68486 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^147f5d3292af2e1cc4a56a7b96db845144c68486
Reading pipe: git cat-file commit 51db83df9d588010d0bd995641c85aa0408a5bb9
current change 25420
trying: earliest ^51db83df9d588010d0bd995641c85aa0408a5bb9 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^51db83df9d588010d0bd995641c85aa0408a5bb9
Reading pipe: git cat-file commit e8f83909ceb570f5a7e48c2853f3c5d8207cea52
current change 25448
trying: earliest ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52
Reading pipe: git cat-file commit 09a48eb7acd594dce52e06681be9c366e1844d66
current change 25521
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
..."""
The fix is two-fold:
* detect an infinite loop and die right away
instead of looping forever;
* make sure, git rev-list --bisect can't return "latestCommit" again by excluding it from the rev-list range explicitly.
---
Notes:
I don't have a simple test-case for this yet,
and I was able to perform a few complex initial git p4 sync runs without hitting this problem.
I suspect, I had somehow messed up with branch definitions and --changesfile option at some point.
git-p4.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
git-p4.py +3 -1
@@ -3323,11 +3323,13 @@ def gitCommitByP4Change(self, ref, change): return next if currentChange < change: earliestCommit = "^%s" % next else: - latestCommit = "%s" % next + if next == latestCommit: + die("Infinite loop while looking in ref %s for change %s. Check your branch mappings" % (ref, change)) + latestCommit = "%s^@" % next return "" def importNewBranch(self, branch, maxChange): # make fast-import flush all changes to disk and update the refs using the checkpoint
Signature · 1 line
2.19.2
PATCH 2/5 git-p4: match branches case insensitively if configured
In reply to Mazo, Andreygit-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.
--- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
git-p4.py +1 -1
@@ -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
Signature · 1 line
2.19.2
PATCH 3/5 git-p4: don't groom exclude path list on every commit
In reply to Mazo, AndreyCurrently, cloneExclude array is being groomed (by removing trailing "...") on every changeset.
(since extractFilesFromCommit() is called on every imported changeset)
As a micro-optimization, do it once while parsing arguments. Also, prepend "/" and remove trailing "..." at the same time.
--- git-p4.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
git-p4.py +7 -5
@@ -1314,11 +1314,11 @@ class Command: def __init__(self): self.usage = "usage: %prog [options]" self.needsGit = True self.verbose = False - # This is required for the "append" cloneExclude action + # This is required for the "append" update_shelve action def ensure_value(self, attr, value): if not hasattr(self, attr) or getattr(self, attr) is None: setattr(self, attr, value) return getattr(self, attr)
@@ -2528,10 +2528,15 @@ def map_in_client(self, depot_path): return self.client_spec_path_cache[depot_path] die( "Error: %s is not found in client spec path" % depot_path ) return "" +def cloneExcludeCallback(option, opt_str, value, parser): + # prepend "/" because the first "/" was consumed as part of the option itself. + # ("-//depot/A/..." becomes "/depot/A/..." after option parsing) + parser.values.cloneExclude += ["/" + re.sub(r"\.\.\.$", "", value)] + class P4Sync(Command, P4UserMap): def __init__(self): Command.__init__(self) P4UserMap.__init__(self)
@@ -2551,11 +2556,11 @@ def __init__(self): optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true', help="Keep entire BRANCH/DIR/SUBDIR prefix during import"), optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true', help="Only sync files that are included in the Perforce Client Spec"), optparse.make_option("-/", dest="cloneExclude", - action="append", type="string", + action="callback", callback=cloneExcludeCallback, type="string", help="exclude depot path"), ] self.description = """Imports from Perforce into a git repository.\n example: //depot/my/project/ -- to import the current head
@@ -2617,12 +2622,10 @@ def checkpoint(self): out = self.gitOutput.readline() if self.verbose: print("checkpoint finished: " + out) def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0): - self.cloneExclude = [re.sub(r"\.\.\.$", "", path) - for path in self.cloneExclude] files = [] fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum]
@@ -3888,11 +3891,10 @@ def run(self, args): if not self.cloneDestination and len(depotPaths) > 1: self.cloneDestination = depotPaths[-1] depotPaths = depotPaths[:-1] - self.cloneExclude = ["/"+p for p in self.cloneExclude] for p in depotPaths: if not p.startswith("//"): sys.stderr.write('Depot paths must start with "//": %s\n' % p) return False
Signature · 1 line
2.19.2
PATCH 4/5 git-p4: add failing test for "don't exclude other files with same prefix"
In reply to Mazo, AndreyIn preparation for a fix, add a failing test case to test that git-p4 doesn't exclude files with the same prefix unintentionally when exclude paths are specified without a trailing /.
I.e., don't exclude "//depot/file_dont_exclude" if run with "-//depot/file". or don't exclude "//depot/discard_file_not" if run with "-//depot/discard_file".
--- t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 5 deletions(-)
t/t9817-git-p4-exclude.sh +46 -5
@@ -20,49 +20,90 @@ test_expect_success 'create exclude repo' ' ( cd "$cli" && mkdir -p wanted discard && echo wanted >wanted/foo && echo discard >discard/foo && - p4 add wanted/foo discard/foo && + echo discard_file >discard_file && + echo discard_file_not >discard_file_not && + p4 add wanted/foo discard/foo discard_file discard_file_not && p4 submit -d "initial revision" ) ' test_expect_success 'check the repo was created correctly' ' test_when_finished cleanup_git && git p4 clone --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo && - test_path_is_file discard/foo + test_path_is_file discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'clone, excluding part of repo' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo && - test_path_is_missing discard/foo + test_path_is_missing discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not + ) +' + +test_expect_failure 'clone, excluding single file, no trailing /' ' + test_when_finished cleanup_git && + git p4 clone -//depot/discard_file --dest="$git" //depot/...@all && + ( + cd "$git" && + test_path_is_file wanted/foo && + test_path_is_file discard/foo && + test_path_is_missing discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'clone, then sync with exclude' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... --dest="$git" //depot/...@all && ( cd "$cli" && - p4 edit wanted/foo discard/foo && + p4 edit wanted/foo discard/foo discard_file_not && date >>wanted/foo && date >>discard/foo && + date >>discard_file_not && p4 submit -d "updating" && cd "$git" && git p4 sync -//depot/discard/... && test_path_is_file wanted/foo && - test_path_is_missing discard/foo + test_path_is_missing discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not + ) +' + +test_expect_failure 'clone, then sync with exclude, no trailing /' ' + test_when_finished cleanup_git && + git p4 clone -//depot/discard/... -//depot/discard_file --dest="$git" //depot/...@all && + ( + cd "$cli" && + p4 edit wanted/foo discard/foo discard_file_not && + date >>wanted/foo && + date >>discard/foo && + date >>discard_file_not && + p4 submit -d "updating" && + + cd "$git" && + git p4 sync -//depot/discard/... -//depot/discard_file && + test_path_is_file wanted/foo && + test_path_is_missing discard/foo && + test_path_is_missing discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'kill p4d' ' kill_p4d
Signature · 1 line
2.19.2
PATCH 5/5 git-p4: don't exclude other files with same prefix
In reply to Mazo, AndreyMake sure not to exclude files unintentionally
if exclude paths are specified without a trailing /.
I.e., don't exclude "//depot/file_dont_exclude" if run with "-//depot/file".
Do this by ensuring that paths without a trailing "/" are only matched completely.
Also, abort path search on the first match as a micro-optimization.
--- git-p4.py | 21 ++++++++++++++------- t/t9817-git-p4-exclude.sh | 4 ++-- 2 files changed, 16 insertions(+), 9 deletions(-)
git-p4.py +14 -7
@@ -2621,22 +2621,29 @@ def checkpoint(self): self.gitStream.write("progress checkpoint\n\n") out = self.gitOutput.readline() if self.verbose: print("checkpoint finished: " + out) + def isPathWanted(self, path): + for p in self.cloneExclude: + if p.endswith("/"): + if p4PathStartsWith(path, p): + return False + # "-//depot/file1" without a trailing "/" should only exclude "file1", but not "file111" or "file1_dir/file2" + elif path.lower() == p.lower(): + return False + for p in self.depotPaths: + if p4PathStartsWith(path, p): + return True + return False + def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0): files = [] fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum] - - if [p for p in self.cloneExclude - if p4PathStartsWith(path, p)]: - found = False - else: - found = [p for p in self.depotPaths - if p4PathStartsWith(path, p)] + found = self.isPathWanted(path) if not found: fnum = fnum + 1 continue file = {}
t/t9817-git-p4-exclude.sh +2 -2
@@ -51,11 +51,11 @@ test_expect_success 'clone, excluding part of repo' ' test_path_is_file discard_file && test_path_is_file discard_file_not ) ' -test_expect_failure 'clone, excluding single file, no trailing /' ' +test_expect_success 'clone, excluding single file, no trailing /' ' test_when_finished cleanup_git && git p4 clone -//depot/discard_file --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo &&
@@ -83,11 +83,11 @@ test_expect_success 'clone, then sync with exclude' ' test_path_is_file discard_file && test_path_is_file discard_file_not ) ' -test_expect_failure 'clone, then sync with exclude, no trailing /' ' +test_expect_success 'clone, then sync with exclude, no trailing /' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... -//depot/discard_file --dest="$git" //depot/...@all && ( cd "$cli" && p4 edit wanted/foo discard/foo discard_file_not &&
Signature · 1 line
2.19.2
v2 cover git-p4: a few assorted fixes for branches, excludes
In reply to Mazo, AndreyThis series fixes a few cases with branch detection and handling of excludes by git-p4.
This is the second iteration of the patch series. Changes since the v1 [1]:
* Added new test case for excluded paths when detecting branches;
* Added a new fix for excluded paths when detecting branches.
[1] https://public-inbox.org/git/[email protected]
Range-diff vs v1:
1: 3ac39171d4 = 1: 3ac39171d4 git-p4: detect/prevent infinite loop in gitCommitByP4Change() 2: e644a8ab49 = 2: e644a8ab49 git-p4: match branches case insensitively if configured 3: 44fed954dc = 3: 44fed954dc git-p4: don't groom exclude path list on every commit 4: a0d3fa6add = 4: a0d3fa6add git-p4: add failing test for "don't exclude other files with same prefix" 5: 3330f88a0d = 5: 3330f88a0d git-p4: don't exclude other files with same prefix -: ---------- > 6: 6170d45951 git-p4: add failing test for "git-p4: respect excluded paths when detecting branches" -: ---------- > 7: 758d8e8486 git-p4: respect excluded paths when detecting branches
Andrey Mazo (7):
git-p4: detect/prevent infinite loop in gitCommitByP4Change()
git-p4: match branches case insensitively if configured
git-p4: don't groom exclude path list on every commit
git-p4: add failing test for "don't exclude other files with same prefix"
git-p4: don't exclude other files with same prefix
git-p4: add failing test for "git-p4: respect excluded paths when detecting branches"
git-p4: respect excluded paths when detecting branches
git-p4.py | 42 ++++++++++++++++++++------------ t/t9801-git-p4-branch.sh | 40 ++++++++++++++++++++++++++++++ t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++---- 3 files changed, 112 insertions(+), 21 deletions(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Signature · 1 line
2.19.2
PATCH v2 1/7 git-p4: detect/prevent infinite loop in gitCommitByP4Change()
Under certain circumstances, gitCommitByP4Change() can enter an infinite loop resulting in git p4 sync hanging forever.
The problem is thatgit rev-list --bisect <latest> ^<earliest> can return <latest>, which would result in reinspecting <latest> and potentially an infinite loop.
This can happen when importing just a subset of P4 repository and/or with explicit "--changesfile" option.
A real-life example:
"""
looking in ref refs/remotes/p4/mybranch for change 26894 using bisect...
Reading pipe: git rev-parse refs/remotes/p4/mybranch
trying: earliest latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git cat-file commit 147f5d3292af2e1cc4a56a7b96db845144c68486
current change 25339
trying: earliest ^147f5d3292af2e1cc4a56a7b96db845144c68486 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^147f5d3292af2e1cc4a56a7b96db845144c68486
Reading pipe: git cat-file commit 51db83df9d588010d0bd995641c85aa0408a5bb9
current change 25420
trying: earliest ^51db83df9d588010d0bd995641c85aa0408a5bb9 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^51db83df9d588010d0bd995641c85aa0408a5bb9
Reading pipe: git cat-file commit e8f83909ceb570f5a7e48c2853f3c5d8207cea52
current change 25448
trying: earliest ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52
Reading pipe: git cat-file commit 09a48eb7acd594dce52e06681be9c366e1844d66
current change 25521
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
..."""
The fix is two-fold:
* detect an infinite loop and die right away
instead of looping forever;
* make sure, git rev-list --bisect can't return "latestCommit" again by excluding it from the rev-list range explicitly.
---
Notes:
I don't have a simple test-case for this yet,
and I was able to perform a few complex initial git p4 sync runs without hitting this problem.
I suspect, I had somehow messed up with branch definitions and --changesfile option at some point.
git-p4.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
git-p4.py +3 -1
@@ -3323,11 +3323,13 @@ def gitCommitByP4Change(self, ref, change): return next if currentChange < change: earliestCommit = "^%s" % next else: - latestCommit = "%s" % next + if next == latestCommit: + die("Infinite loop while looking in ref %s for change %s. Check your branch mappings" % (ref, change)) + latestCommit = "%s^@" % next return "" def importNewBranch(self, branch, maxChange): # make fast-import flush all changes to disk and update the refs using the checkpoint
Signature · 1 line
2.19.2
PATCH v2 2/7 git-p4: match branches case insensitively if configured
In reply to Mazo, Andreygit-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.
--- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
git-p4.py +1 -1
@@ -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
Signature · 1 line
2.19.2
Re: [PATCH v2 2/7] git-p4: match branches case insensitively if configured
Quoted 15 lines · Andrey
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.
Quoted 3 lines
So, do branch search case insensitively if running with core.ignorecase set. Teach
splitFilesIntoBranches()to use thep4PathStartsWith()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
Quoted 26 lines
Signed-off-by Andrey Mazo <[email protected]>--- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)git-p4.py +1
-2@@ -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
Re: [PATCH v2 2/7] git-p4: match branches case insensitively if configured
From: "Mazo, Andrey" <[email protected]>
23.03.2019, 05:16, "Luke Diamand" <[email protected]>:
Quoted 19 lines · Andrey
Quoted 14 lines
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.Quoted 3 lines
So, do branch search case insensitively if running with core.ignorecase set. Teach
splitFilesIntoBranches()to use thep4PathStartsWith()function for path prefix matches instead of always case-sensitive match.
Quoted 5 lines
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.
PATCH v2 3/7 git-p4: don't groom exclude path list on every commit
In reply to Mazo, AndreyCurrently, cloneExclude array is being groomed (by removing trailing "...") on every changeset.
(since extractFilesFromCommit() is called on every imported changeset)
As a micro-optimization, do it once while parsing arguments. Also, prepend "/" and remove trailing "..." at the same time.
--- git-p4.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
git-p4.py +7 -5
@@ -1314,11 +1314,11 @@ class Command: def __init__(self): self.usage = "usage: %prog [options]" self.needsGit = True self.verbose = False - # This is required for the "append" cloneExclude action + # This is required for the "append" update_shelve action def ensure_value(self, attr, value): if not hasattr(self, attr) or getattr(self, attr) is None: setattr(self, attr, value) return getattr(self, attr)
@@ -2528,10 +2528,15 @@ def map_in_client(self, depot_path): return self.client_spec_path_cache[depot_path] die( "Error: %s is not found in client spec path" % depot_path ) return "" +def cloneExcludeCallback(option, opt_str, value, parser): + # prepend "/" because the first "/" was consumed as part of the option itself. + # ("-//depot/A/..." becomes "/depot/A/..." after option parsing) + parser.values.cloneExclude += ["/" + re.sub(r"\.\.\.$", "", value)] + class P4Sync(Command, P4UserMap): def __init__(self): Command.__init__(self) P4UserMap.__init__(self)
@@ -2551,11 +2556,11 @@ def __init__(self): optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true', help="Keep entire BRANCH/DIR/SUBDIR prefix during import"), optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true', help="Only sync files that are included in the Perforce Client Spec"), optparse.make_option("-/", dest="cloneExclude", - action="append", type="string", + action="callback", callback=cloneExcludeCallback, type="string", help="exclude depot path"), ] self.description = """Imports from Perforce into a git repository.\n example: //depot/my/project/ -- to import the current head
@@ -2617,12 +2622,10 @@ def checkpoint(self): out = self.gitOutput.readline() if self.verbose: print("checkpoint finished: " + out) def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0): - self.cloneExclude = [re.sub(r"\.\.\.$", "", path) - for path in self.cloneExclude] files = [] fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum]
@@ -3888,11 +3891,10 @@ def run(self, args): if not self.cloneDestination and len(depotPaths) > 1: self.cloneDestination = depotPaths[-1] depotPaths = depotPaths[:-1] - self.cloneExclude = ["/"+p for p in self.cloneExclude] for p in depotPaths: if not p.startswith("//"): sys.stderr.write('Depot paths must start with "//": %s\n' % p) return False
Signature · 1 line
2.19.2
PATCH v2 4/7 git-p4: add failing test for "don't exclude other files with same prefix"
In reply to Mazo, AndreyIn preparation for a fix, add a failing test case to test that git-p4 doesn't exclude files with the same prefix unintentionally when exclude paths are specified without a trailing /.
I.e., don't exclude "//depot/file_dont_exclude" if run with "-//depot/file". or don't exclude "//depot/discard_file_not" if run with "-//depot/discard_file".
--- t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 5 deletions(-)
t/t9817-git-p4-exclude.sh +46 -5
@@ -20,49 +20,90 @@ test_expect_success 'create exclude repo' ' ( cd "$cli" && mkdir -p wanted discard && echo wanted >wanted/foo && echo discard >discard/foo && - p4 add wanted/foo discard/foo && + echo discard_file >discard_file && + echo discard_file_not >discard_file_not && + p4 add wanted/foo discard/foo discard_file discard_file_not && p4 submit -d "initial revision" ) ' test_expect_success 'check the repo was created correctly' ' test_when_finished cleanup_git && git p4 clone --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo && - test_path_is_file discard/foo + test_path_is_file discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'clone, excluding part of repo' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo && - test_path_is_missing discard/foo + test_path_is_missing discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not + ) +' + +test_expect_failure 'clone, excluding single file, no trailing /' ' + test_when_finished cleanup_git && + git p4 clone -//depot/discard_file --dest="$git" //depot/...@all && + ( + cd "$git" && + test_path_is_file wanted/foo && + test_path_is_file discard/foo && + test_path_is_missing discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'clone, then sync with exclude' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... --dest="$git" //depot/...@all && ( cd "$cli" && - p4 edit wanted/foo discard/foo && + p4 edit wanted/foo discard/foo discard_file_not && date >>wanted/foo && date >>discard/foo && + date >>discard_file_not && p4 submit -d "updating" && cd "$git" && git p4 sync -//depot/discard/... && test_path_is_file wanted/foo && - test_path_is_missing discard/foo + test_path_is_missing discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not + ) +' + +test_expect_failure 'clone, then sync with exclude, no trailing /' ' + test_when_finished cleanup_git && + git p4 clone -//depot/discard/... -//depot/discard_file --dest="$git" //depot/...@all && + ( + cd "$cli" && + p4 edit wanted/foo discard/foo discard_file_not && + date >>wanted/foo && + date >>discard/foo && + date >>discard_file_not && + p4 submit -d "updating" && + + cd "$git" && + git p4 sync -//depot/discard/... -//depot/discard_file && + test_path_is_file wanted/foo && + test_path_is_missing discard/foo && + test_path_is_missing discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'kill p4d' ' kill_p4d
Signature · 1 line
2.19.2
PATCH v2 5/7 git-p4: don't exclude other files with same prefix
In reply to Mazo, AndreyMake sure not to exclude files unintentionally
if exclude paths are specified without a trailing /.
I.e., don't exclude "//depot/file_dont_exclude" if run with "-//depot/file".
Do this by ensuring that paths without a trailing "/" are only matched completely.
Also, abort path search on the first match as a micro-optimization.
--- git-p4.py | 21 ++++++++++++++------- t/t9817-git-p4-exclude.sh | 4 ++-- 2 files changed, 16 insertions(+), 9 deletions(-)
git-p4.py +14 -7
@@ -2621,22 +2621,29 @@ def checkpoint(self): self.gitStream.write("progress checkpoint\n\n") out = self.gitOutput.readline() if self.verbose: print("checkpoint finished: " + out) + def isPathWanted(self, path): + for p in self.cloneExclude: + if p.endswith("/"): + if p4PathStartsWith(path, p): + return False + # "-//depot/file1" without a trailing "/" should only exclude "file1", but not "file111" or "file1_dir/file2" + elif path.lower() == p.lower(): + return False + for p in self.depotPaths: + if p4PathStartsWith(path, p): + return True + return False + def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0): files = [] fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum] - - if [p for p in self.cloneExclude - if p4PathStartsWith(path, p)]: - found = False - else: - found = [p for p in self.depotPaths - if p4PathStartsWith(path, p)] + found = self.isPathWanted(path) if not found: fnum = fnum + 1 continue file = {}
t/t9817-git-p4-exclude.sh +2 -2
@@ -51,11 +51,11 @@ test_expect_success 'clone, excluding part of repo' ' test_path_is_file discard_file && test_path_is_file discard_file_not ) ' -test_expect_failure 'clone, excluding single file, no trailing /' ' +test_expect_success 'clone, excluding single file, no trailing /' ' test_when_finished cleanup_git && git p4 clone -//depot/discard_file --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo &&
@@ -83,11 +83,11 @@ test_expect_success 'clone, then sync with exclude' ' test_path_is_file discard_file && test_path_is_file discard_file_not ) ' -test_expect_failure 'clone, then sync with exclude, no trailing /' ' +test_expect_success 'clone, then sync with exclude, no trailing /' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... -//depot/discard_file --dest="$git" //depot/...@all && ( cd "$cli" && p4 edit wanted/foo discard/foo discard_file_not &&
Signature · 1 line
2.19.2
PATCH v2 6/7 git-p4: add failing test for "git-p4: respect excluded paths when detecting branches"
In reply to Mazo, AndreyIn preparation for a fix, add a failing test case to test that git-p4 doesn't exclude files despite being told to when handling multiple branches.
I.e., it should exclude //depot/branch2/file2 when run with -//depot/branch2/file2, but doesn't do this right now.
The test is based on 'git p4 clone complex branches' test with the following changes:
* account for file3 moved from branch3 to branch4 in test 'git p4 submit to two branches in a single changelist';
* account for branch6 created in test 'git p4 clone file subset branch';
* file2 is expected to be missing from all branches due to explicit exclude.
--- t/t9801-git-p4-branch.sh | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
t/t9801-git-p4-branch.sh +40 -0
@@ -409,10 +409,50 @@ test_expect_failure 'git p4 clone file subset branch' ' test_path_is_missing file2 && test_path_is_missing file3 ) ' +# Check that excluded files are omitted during import +test_expect_failure 'git p4 clone complex branches with excluded files' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList branch1:branch2 && + git config --add git-p4.branchList branch1:branch3 && + git config --add git-p4.branchList branch1:branch4 && + git config --add git-p4.branchList branch1:branch5 && + git config --add git-p4.branchList branch1:branch6 && + git p4 clone --dest=. --detect-branches -//depot/branch1/file2 -//depot/branch2/file2 -//depot/branch3/file2 -//depot/branch4/file2 -//depot/branch5/file2 -//depot/branch6/file2 //depot@all && + git log --all --graph --decorate --stat && + git reset --hard p4/depot/branch1 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_file file3 && + git reset --hard p4/depot/branch2 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_missing file3 && + git reset --hard p4/depot/branch3 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_missing file3 && + git reset --hard p4/depot/branch4 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_file file3 && + git reset --hard p4/depot/branch5 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_file file3 && + git reset --hard p4/depot/branch6 && + test_path_is_file file1 && + test_path_is_missing file2 && + 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" &&
Signature · 1 line
2.19.2
PATCH v2 7/7 git-p4: respect excluded paths when detecting branches
In reply to Mazo, AndreyCurrently, excluded paths are only handled in the following cases:
* no branch detection;
* branch detection with using clientspec.
However, excluded paths are not respected in case of branch detection without using clientspec.
Fix this by consulting the list of excluded paths
when splitting files across branches.
--- git-p4.py | 3 +-- t/t9801-git-p4-branch.sh | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-)
git-p4.py +1 -2
@@ -2708,12 +2708,11 @@ def splitFilesIntoBranches(self, commit): branches = {} fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum] - found = [p for p in self.depotPaths - if p4PathStartsWith(path, p)] + found = self.isPathWanted(path) if not found: fnum = fnum + 1 continue file = {}
t/t9801-git-p4-branch.sh +1 -1
@@ -410,11 +410,11 @@ test_expect_failure 'git p4 clone file subset branch' ' test_path_is_missing file3 ) ' # Check that excluded files are omitted during import -test_expect_failure 'git p4 clone complex branches with excluded files' ' +test_expect_success 'git p4 clone complex branches with excluded files' ' test_when_finished cleanup_git && test_create_repo "$git" && ( cd "$git" && git config git-p4.branchList branch1:branch2 &&
Signature · 1 line
2.19.2
cover git-p4: "alien" branches and load changelist info from file
In reply to Mazo, AndreyThis patch series introduces two experimental features to git-p4, which unrelated to each other.
1. The first patch adds support for so-called "alien" branches.
The feature lets git-p4 create empty commits
to make the history or tags more accurate.
It is particularly useful when splitting a large Perforce depot into multiple git repositories.
2. The second patch adds support for loading changelist information from a file. (p4 -G describe equivalent)
The original use case is to be able to migrate a Perforce depot, which database got a little bit corrupted, into git.
It would be nice to get some feedback to see
if these features are usable in general and are worth mainlining at all.
The patches don't contain documentation or test changes yet, because I wanted to get feedback first
if there is interest in mainlining these features in the first place.
This patch series should be applied on top of
"[PATCH v2 0/7] git-p4: a few assorted fixes for branches, excludes" [1]
[1] https://public-inbox.org/git/[email protected]/t/#m965fb5895d25d6b42638dd8efbb96e9fa9182978
Andrey Mazo (2):
git-p4: introduce alien branch mappings
git-p4: support loading changelist descriptions from files
git-p4.py | 84 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 71 insertions(+), 13 deletions(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
prerequisite-patch-id: 23e039fec7a1f5c51c98326a14d788adb1ecb5ba
prerequisite-patch-id: 030a0acdce715ff99916fd412832e5a9471225c3
prerequisite-patch-id: 10661f77392f4131d2375976c77a7cd231fdf9ab
prerequisite-patch-id: a55360c904eba1b9e9c934405d3141eb96c5ad30
prerequisite-patch-id: 46357586199c02d956d53d782a12f1ee0c991302
prerequisite-patch-id: c683e7d6017580df9385a1544af409ca615d770c
prerequisite-patch-id: 411dcb5e95aff036e0cb3e850ea75f2424b260a6
Signature · 1 line
2.19.2
RFC PATCH 1/2 git-p4: introduce alien branch mappings
Labels in Perforce are not global, but can be placed on a particular view/subdirectory. This might pose difficulties when importing only parts of Perforce depot into a git repository. For example:
1. Depot layout is as follows:
//depot/metaproject/branch1/subprojectA/...
//depot/metaproject/branch1/subprojectB/...
//depot/metaproject/branch2/subprojectA/...
//depot/metaproject/branch2/subprojectB/... 2. Labels are placed as follows:
* label 1A on //depot/metaproject/branch1/subprojectA/...
* label 1B on //depot/metaproject/branch1/subprojectB/...
* label 2A on //depot/metaproject/branch2/subprojectA/...
* label 2B on //depot/metaproject/branch2/subprojectB/...
3. The goal is to import
subprojectA into subprojectA.git and
subprojectB into subprojectB.git
preserving all the branches and labels.
4. Importing subprojectA.
Label 1A is imported fine because it's placed on certain commit on branch1. However, label 1B is not imported because it's placed on a commit in another subproject:
git-p4 says: "importing label 1B: could not find git commit for changelist ..." The same is with label 2A, which is imported; and 2B, which is not.
Currently, there is no easy way (that I'm aware of) to tell git-p4 to
import an empty commit into a desired branch,
so that a label placed on that changelist could be imported as well,
It might be possible to get a similar effect by importing both subprojectA and B in a single git repo, and then running git filter-branch --subdirectory-filter subprojectA,
but this might produce way more irrelevant empty commits, than needed for labels. (although imported changelists can be limited with git-p4 --changesfile option)
So, introduce a concept of an "alien" branch.
In the above example of importing subprojectA,
* branch1/subprojectB is an alien branch for branch1/subprojectA;
* branch2/subprojectB is an alien branch for branch2/subprojectA.
Any changelist for branch1/subprojectB will be imported into subprojectA.git branch1 as an empty commit for the sole purpose of being labeled with a tag later or just to preserve the history of changes across the branches.
This relation between branches is specified in a similar way to branchList:git config --add git-p4.alienLabelBranchMap alien_branch:real_branch
For the example of importing subprojectA above, config parameters are
``` git config --add git-p4.branchList branch1/subprojectA:branch2/subprojectA git config --add git-p4.alienLabelBranchMap branch1/subprojectB:branch1/subprojectA git config --add git-p4.alienLabelBranchMap branch2/subprojectB:branch2/subprojectA ```
A similar use case, is when a label is placed on a changelist for an excluded path.
1. Depot layout is as follows:
//depot/branch1/...
//depot/branch1/exclude_me/...
2. Labels are placed as follows:
* label 1 on //depot/branch1/...
* label 1E on //depot/branch1/exclude_me/...
3. The goal is to import
//depot/... into depot.git excluding files under
//depot/branch1/exclude_me/...
and preserving all the branches and labels. 4. Importing subprojectA.
Label 1 is imported fine because it's placed on certain commit on branch1. However, label 1E is not imported because it's placed on a commit which is excluded.
For this use case, the config would be
``` git config --add git-p4.alienLabelBranchMap branch1/exclude_me:branch1 ```
Note that the current implementation doesn't process alien branches when a clientspec is used.
Diff best viewed with --ignore-all-space .
---
Notes:
Documentation changes and tests are obviously missing, but I hoped to get some feedback on the idea overall before working on those.
A better name for "alien" branches is very welcome.
git-p4.py | 59 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 12 deletions(-)
git-p4.py +47 -12
@@ -2610,10 +2610,12 @@ def __init__(self): # map from branch depot path to parent branch self.knownBranches = {} self.initialParents = {} + self.knownAlienLabelBranches = {} + self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60)) self.labels = {} # Force a checkpoint in fast-import and wait for it to finish def checkpoint(self):
@@ -2705,14 +2707,37 @@ def splitFilesIntoBranches(self, commit): if self.clientSpecDirs: files = self.extractFilesFromCommit(commit) self.clientSpecDirs.update_client_spec_path_cache(files) branches = {} + alienBranches = {} fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum] found = self.isPathWanted(path) + + # start with the full relative path where this file would + # go in a p4 client + if self.useClientSpec: + # with a clientspec, we won't know where the file goes if it's excluded + if found: + relPath = self.clientSpecDirs.map_in_client(path) + else: + relPath = None + else: + # without a clientspec, we can guess a file path even if it's excluded + relPath = self.stripRepoPath(path, self.depotPaths) + + # Process alien branches before excludes (i.e. before respecting `found`) -- + # picking (empty) commits from excluded paths is one of the use-cases for alien branches. + # We don't commit any files for alien branches, so don't violate excludes. + for (alienPath, ourPath) in self.knownAlienLabelBranches.items(): + if relPath is not None and p4PathStartsWith(relPath, alienPath + '/') and ourPath in self.knownBranches: + # we don't put any files since they are under the paths, we're not interested in. + # however, we still want the commit message and etc. + alienBranches[ourPath] = [{"alienPath": alienPath}] + if not found: fnum = fnum + 1 continue file = {}
@@ -2720,26 +2745,23 @@ def splitFilesIntoBranches(self, commit): file["rev"] = commit["rev%s" % fnum] file["action"] = commit["action%s" % fnum] file["type"] = commit["type%s" % fnum] fnum = fnum + 1 - # 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, e.g. if p4PathStartsWith(relPath, branch + "/"): if branch not in branches: branches[branch] = [] branches[branch].append(file) break + # not on any branch, check out our alien mapping + if not branches: + branches = alienBranches + return branches def writeToGitStream(self, gitMode, relPath, contents): self.gitStream.write('M %s inline %s\n' % (gitMode, relPath)) self.gitStream.write('data %d\n' % sum(len(d) for d in contents))
@@ -3283,10 +3305,17 @@ def getBranchMappingFromGitBranches(self): branch = "main" else: branch = branch[len(self.projectName):] self.knownBranches[branch] = branch + def getAlienLabelBranchMapping(self): + alienLabelBranches = gitConfigList("git-p4.alienLabelBranchMap") + for mapping in alienLabelBranches: + if mapping: + (alien, ours) = mapping.split(":") + self.knownAlienLabelBranches[alien] = ours + def updateOptionDict(self, d): option_keys = {} if self.keepRepoPath: option_keys['keepRepoPath'] = 1
@@ -3396,17 +3425,22 @@ def importChanges(self, changes, origin_revision=0): try: if self.detectBranches: branches = self.splitFilesIntoBranches(description) for branch in branches.keys(): - ## HACK --hwn - branchPrefix = self.depotPaths[0] + branch + "/" - self.branchPrefixes = [ branchPrefix ] + # hack branch prefix and file list for alien branches + if branches[branch] and "alienPath" in branches[branch][0]: + branchPrefix = self.depotPaths[0] + branches[branch][0]["alienPath"] + "/" + filesForCommit = [] + else: + ## HACK --hwn + branchPrefix = self.depotPaths[0] + branch + "/" + filesForCommit = branches[branch] - parent = "" + self.branchPrefixes = [branchPrefix] - filesForCommit = branches[branch] + parent = "" if self.verbose: print("branch is %s" % branch) self.updatedBranches.add(branch)
@@ -3715,10 +3749,11 @@ def run(self, args): if self.hasOrigin: self.getBranchMappingFromGitBranches() else: self.getBranchMapping() + self.getAlienLabelBranchMapping() if self.verbose: print("p4-git branches: %s" % self.p4BranchesInGit) print("initial parents: %s" % self.initialParents) for b in self.p4BranchesInGit: if b != "master":
Signature · 1 line
2.19.2
Re: [RFC PATCH 1/2] git-p4: introduce alien branch mappings
Quoted 24 lines · Andrey
Labels in Perforce are not global, but can be placed on a particular view/subdirectory. This might pose difficulties when importing only parts of Perforce depot into a git repository. For example:
1. Depot layout is as follows://depot/metaproject/branch1/subprojectA/... //depot/metaproject/branch1/subprojectB/... //depot/metaproject/branch2/subprojectA/... //depot/metaproject/branch2/subprojectB/...2. Labels are placed as follows:
* label 1A on //depot/metaproject/branch1/subprojectA/...
* label 1B on //depot/metaproject/branch1/subprojectB/...
* label 2A on //depot/metaproject/branch2/subprojectA/...
* label 2B on //depot/metaproject/branch2/subprojectB/...
3. The goal is to import
subprojectA into subprojectA.git and
subprojectB into subprojectB.git
preserving all the branches and labels.
4. Importing subprojectA.
Label 1A is imported fine because it's placed on certain commit on branch1. However, label 1B is not imported because it's placed on a commit in another subproject:
git-p4 says: "importing label 1B: could not find git commit for changelist ..." The same is with label 2A, which is imported; and 2B, which is not.
Quoted 3 lines
Currently, there is no easy way (that I'm aware of) to tell git-p4 to import an empty commit into a desired branch,
so that a label placed on that changelist could be imported as well,
So there is a file in subprojectA/foo.c@41.
And label 1B is against //depot/metaproject/branch1/subprojectB/bar.c@42.
And I suppose in Perforce you could still checkout subprojectA at change 42 and you would get change 41.
But with the way git-p4 works, the label just gets discarded.
You want to be able to checkout the subjectA with a tag called 1B and get the file contents as of 42.
I wonder if it would be easier to teach the code in importP4Labels to go searching harder for the next lower changelist number?
Where it currently says "could not find git commit"... could it do something like "p4 changes -m1 //depot/path/...@LABEL" and use that instead?
I'm not sure if that would work but it would mean you wouldn't need any extra configuration to maintain.
But perhaps I have misunderstood what you're trying to do here! Perhaps a failing test case might help explain it better?
Thanks
Luke
Quoted 209 lines
It might be possible to get a similar effect by importing both subprojectA and B in a single git repo, and then running
git filter-branch --subdirectory-filter subprojectA,
but this might produce way more irrelevant empty commits, than needed for labels. (although imported changelists can be limited with git-p4 --changesfile option)So, introduce a concept of an "alien" branch.
In the above example of importing subprojectA,
* branch1/subprojectB is an alien branch for branch1/subprojectA;
* branch2/subprojectB is an alien branch for branch2/subprojectA.
Any changelist for branch1/subprojectB will be imported into subprojectA.git branch1 as an empty commit for the sole purpose of being labeled with a tag later or just to preserve the history of changes across the branches.This relation between branches is specified in a similar way to branchList:
git config --add git-p4.alienLabelBranchMap alien_branch:real_branchFor the example of importing subprojectA above, config parameters are
``` git config --add git-p4.branchList branch1/subprojectA:branch2/subprojectA git config --add git-p4.alienLabelBranchMap branch1/subprojectB:branch1/subprojectA git config --add git-p4.alienLabelBranchMap branch2/subprojectB:branch2/subprojectA ```A similar use case, is when a label is placed on a changelist for an excluded path.
1. Depot layout is as follows://depot/branch1/... //depot/branch1/exclude_me/... 2. Labels are placed as follows: * label 1 on //depot/branch1/... * label 1E on //depot/branch1/exclude_me/... 3. The goal is to import //depot/... into depot.git excluding files under //depot/branch1/exclude_me/... and preserving all the branches and labels.4. Importing subprojectA.
Label 1 is imported fine because it's placed on certain commit on branch1. However, label 1E is not imported because it's placed on a commit which is excluded.For this use case, the config would be
``` git config --add git-p4.alienLabelBranchMap branch1/exclude_me:branch1 ```Note that the current implementation doesn't process alien branches when a clientspec is used.
Diff best viewed with --ignore-all-space .
Signed-off-by Andrey Mazo <[email protected]>---
Notes:
Documentation changes and tests are obviously missing, but I hoped to get some feedback on the idea overall before working on those.A better name for "alien" branches is very welcome.
git-p4.py | 59 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 12 deletions(-)git-p4.py +47
-13@@ -2610,10 +2610,12 @@ def __init__(self): # map from branch depot path to parent branch self.knownBranches = {} self.initialParents = {} + self.knownAlienLabelBranches = {} + self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60)) self.labels = {} # Force a checkpoint in fast-import and wait for it to finish def checkpoint(self):@@ -2705,14 +2707,37 @@ def splitFilesIntoBranches(self, commit): if self.clientSpecDirs: files = self.extractFilesFromCommit(commit) self.clientSpecDirs.update_client_spec_path_cache(files) branches = {} + alienBranches = {} fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum] found = self.isPathWanted(path) + + # start with the full relative path where this file would + # go in a p4 client + if self.useClientSpec: + # with a clientspec, we won't know where the file goes if it's excluded + if found: + relPath = self.clientSpecDirs.map_in_client(path) + else: + relPath = None + else: + # without a clientspec, we can guess a file path even if it's excluded + relPath = self.stripRepoPath(path, self.depotPaths) + + # Process alien branches before excludes (i.e. before respecting `found`) -- + # picking (empty) commits from excluded paths is one of the use-cases for alien branches. + # We don't commit any files for alien branches, so don't violate excludes. + for (alienPath, ourPath) in self.knownAlienLabelBranches.items(): + if relPath is not None and p4PathStartsWith(relPath, alienPath + '/') and ourPath in self.knownBranches: + # we don't put any files since they are under the paths, we're not interested in. + # however, we still want the commit message and etc. + alienBranches[ourPath] = [{"alienPath": alienPath}] + if not found: fnum = fnum + 1 continue file = {}@@ -2720,26 +2745,23 @@ def splitFilesIntoBranches(self, commit): file["rev"] = commit["rev%s" % fnum] file["action"] = commit["action%s" % fnum] file["type"] = commit["type%s" % fnum] fnum = fnum + 1 - # 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, e.g. if p4PathStartsWith(relPath, branch + "/"): if branch not in branches: branches[branch] = [] branches[branch].append(file) break + # not on any branch, check out our alien mapping + if not branches: + branches = alienBranches + return branches def writeToGitStream(self, gitMode, relPath, contents): self.gitStream.write('M %s inline %s\n' % (gitMode, relPath)) self.gitStream.write('data %d\n' % sum(len(d) for d in contents))@@ -3283,10 +3305,17 @@ def getBranchMappingFromGitBranches(self): branch = "main" else: branch = branch[len(self.projectName):] self.knownBranches[branch] = branch + def getAlienLabelBranchMapping(self): + alienLabelBranches = gitConfigList("git-p4.alienLabelBranchMap") + for mapping in alienLabelBranches: + if mapping: + (alien, ours) = mapping.split(":") + self.knownAlienLabelBranches[alien] = ours + def updateOptionDict(self, d): option_keys = {} if self.keepRepoPath: option_keys['keepRepoPath'] = 1@@ -3396,17 +3425,22 @@ def importChanges(self, changes, origin_revision=0): try: if self.detectBranches: branches = self.splitFilesIntoBranches(description) for branch in branches.keys(): - ## HACK --hwn - branchPrefix = self.depotPaths[0] + branch + "/" - self.branchPrefixes = [ branchPrefix ] + # hack branch prefix and file list for alien branches + if branches[branch] and "alienPath" in branches[branch][0]: + branchPrefix = self.depotPaths[0] + branches[branch][0]["alienPath"] + "/" + filesForCommit = [] + else: + ## HACK --hwn + branchPrefix = self.depotPaths[0] + branch + "/" + filesForCommit = branches[branch] - parent = "" + self.branchPrefixes = [branchPrefix] - filesForCommit = branches[branch] + parent = "" if self.verbose: print("branch is %s" % branch) self.updatedBranches.add(branch)@@ -3715,10 +3749,11 @@ def run(self, args): if self.hasOrigin: self.getBranchMappingFromGitBranches() else: self.getBranchMapping() + self.getAlienLabelBranchMapping() if self.verbose: print("p4-git branches: %s" % self.p4BranchesInGit) print("initial parents: %s" % self.initialParents) for b in self.p4BranchesInGit: if b != "master": --2.19.2
Re: [RFC PATCH 1/2] git-p4: introduce alien branch mappings
Quoted 27 lines
Quoted 23 lines
Labels in Perforce are not global, but can be placed on a particular view/subdirectory. This might pose difficulties when importing only parts of Perforce depot into a git repository. For example:
1. Depot layout is as follows://depot/metaproject/branch1/subprojectA/... //depot/metaproject/branch1/subprojectB/... //depot/metaproject/branch2/subprojectA/... //depot/metaproject/branch2/subprojectB/...2. Labels are placed as follows:
* label 1A on //depot/metaproject/branch1/subprojectA/...
* label 1B on //depot/metaproject/branch1/subprojectB/...
* label 2A on //depot/metaproject/branch2/subprojectA/...
* label 2B on //depot/metaproject/branch2/subprojectB/...
3. The goal is to import
subprojectA into subprojectA.git and
subprojectB into subprojectB.git
preserving all the branches and labels.
4. Importing subprojectA.
Label 1A is imported fine because it's placed on certain commit on branch1. However, label 1B is not imported because it's placed on a commit in another subproject:
git-p4 says: "importing label 1B: could not find git commit for changelist ..." The same is with label 2A, which is imported; and 2B, which is not.Quoted 3 lines
Currently, there is no easy way (that I'm aware of) to tell git-p4 to import an empty commit into a desired branch,
so that a label placed on that changelist could be imported as well,
Quoted 7 lines
So there is a file in subprojectA/foo.c@41.
And label 1B is against //depot/metaproject/branch1/subprojectB/bar.c@42.And I suppose in Perforce you could still checkout subprojectA at change 42 and you would get change 41.
But with the way git-p4 works, the label just gets discarded.
Yes, exactly.
Quoted 6 lines
You want to be able to checkout the subjectA with a tag called 1B and get the file contents as of 42.
I wonder if it would be easier to teach the code in importP4Labels to go searching harder for the next lower changelist number?
Quoted 6 lines
Where it currently says "could not find git commit"... could it do something like "p4 changes -m1 //depot/path/...@LABEL" and use that instead?
I'm not sure if that would work but it would mean you wouldn't need any extra configuration to maintain.
Yeah, that's a great idea!
I think, it should work pretty well in simpler cases for sure. Initially, I was thinking, that I needed an explicit configuration option to choose the proper branch/subproject in a more complicated case, but let me give it a try to your idea -- hopefully it just works.
Some new option like git-p4.allowInexactLabels to enable this behavior? Don't think it should be enabled by default unless git-p4.labelImportRegexp is set, right?
Quoted 2 lines
But perhaps I have misunderstood what you're trying to do here! Perhaps a failing test case might help explain it better?
No, I think, you got it right!
Thank you for the great suggestion!
Yeah, let me see if I can get a simple but representative test case.
Quoted 3 lines
Thanks
Luke
RFC PATCH 1/1 git-p4: inexact label detection
Labels in Perforce are not global, but can be placed on a particular view/subdirectory. This might pose difficulties when importing only parts of Perforce depot into a git repository. For example:
1. Depot layout is as follows:
//depot/metaproject/branch1/subprojectA/...
//depot/metaproject/branch1/subprojectB/...
//depot/metaproject/branch2/subprojectA/...
//depot/metaproject/branch2/subprojectB/... 2. Labels are placed as follows:
* label 1A on //depot/metaproject/branch1/subprojectA/...
* label 1B on //depot/metaproject/branch1/subprojectB/...
* label 2A on //depot/metaproject/branch2/subprojectA/...
* label 2B on //depot/metaproject/branch2/subprojectB/...
3. The goal is to import
subprojectA into subprojectA.git and
subprojectB into subprojectB.git
preserving all the branches and labels.
4. Importing subprojectA.
Label 1A is imported fine because it's placed on certain commit on branch1. However, label 1B is not imported because it's placed on a commit in another subproject:
git-p4 says: "importing label 1B: could not find git commit for changelist ..." The same is with label 2A, which is imported; and 2B, which is not.
Currently, there is no easy way (that I'm aware of) to tell git-p4 to
import an empty commit into a desired branch,
so that a label placed on that changelist could be imported as well,
It might be possible to get a similar effect by importing both subprojectA and B in a single git repo, and then running git filter-branch --subdirectory-filter subprojectA,
but this might produce way more irrelevant empty commits, than needed for labels. (although imported changelists can be limited with git-p4 --changesfile option)
Also, git filter-branch is harder to use for incremental imports
or when changes are submitted from git back to Perforce.
As suggested by Luke,
instead of creating an empty commit for the sole purpose of being tagged later, teach git-p4 to search harder for the next lower changelist, corresponding to the label in question.
Do this by finding the highest changelist up to the label under all known branches, (branches are finalized by the time importP4Labels() runs)
and using it instead of a depot-wide changelist corresponding to the label.
This new behavior may not be desired for people,
who want exact label <-> changelist relationship.
So, add a new boolean config parameter git-p4.allowInexactLabels (defaults to false) to explicitly enable it if needed.
Also, this behavior only appears to be useful in case of multiple branches, (otherwise, every Perforce changelist should appear in git)
so it's not engaged when running without branch detection.
Detect and report (--verbose) "inexact" tags,
i.e. tags placed on a lower changelist than was in Perforce. Implement this by comparing a changelist for which a commit was found with a changelist corresponding to the label on the whole depot.
Note, that the new "inexact" logic works slower
than the original code in case of numerous branches,
because p4 needs to calculate the most recent change for each branch path instead of just one.
This is an alternative solution to "alien" branches concept proposed earlier:
https://public-inbox.org/git/b02df749b9266ac8c73707617a171122156621ab.1553283214.git.amazo@checkvideo.com/
--- git-p4.py | 45 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-)
git-p4.py +39 -6
@@ -3186,17 +3186,40 @@ def importP4Labels(self, stream, p4Labels): if name in ignoredP4Labels: continue labelDetails = p4CmdList(['label', "-o", name])[0] - # get the most recent changelist for each file in this label - change = p4Cmd(["changes", "-m", "1"] + ["%s...@%s" % (p, name) + if self.detectBranches and gitConfigBool("git-p4.allowInexactLabels"): + doInexactLabels = True + else: + doInexactLabels = False + + # get the most recent changelist in this label for the whole depot + depot_wide_changelist = p4Cmd(["changes", "-m", "1"] + ["%s...@%s" % (p, name) for p in self.depotPaths]) + if 'change' in depot_wide_changelist: + depot_wide_changelist = int(depot_wide_changelist['change']) + else: + depot_wide_changelist = None + + # get the most recent changelist for each file under branches of interest in this label + if doInexactLabels: + paths = ["%s...@%s" % (self.depotPaths[0] + p + '/', name) for p in self.knownBranches] + changes = p4CmdList(["changes", "-m", "1"] + paths) + changes = [int(c['change']) for c in changes if 'change' in c] + + # there may be different "most recent" changelists for different paths. + # take the newest since some paths were just modified later than others. + if changes: + changelist = max(changes) + else: + changelist = None + else: + changelist = depot_wide_changelist - if 'change' in change: + if changelist: # find the corresponding git commit; take the oldest commit - changelist = int(change['change']) if changelist in self.committedChanges: gitCommit = ":%d" % changelist # use a fast-import mark commitFound = True else: gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
@@ -3216,14 +3239,24 @@ def importP4Labels(self, stream, p4Labels): tmwhen = 1 when = int(time.mktime(tmwhen)) self.streamTag(stream, name, labelDetails, gitCommit, when) if verbose: - print("p4 label %s mapped to git commit %s" % (name, gitCommit)) + if depot_wide_changelist == changelist: + isExact = "" + else: + isExact = " inexactly" + print("p4 label %s mapped%s to git commit %s" % (name, isExact, gitCommit)) else: if verbose: - print("Label %s has no changelists - possibly deleted?" % name) + if depot_wide_changelist: + # there is a changelist corresponding to this label, + # but it's not under any branches of interest. + print("Label %s has no changelists under detected branches -- ignoring" % name) + else: + # there is no changelist corresponding to this label in the whole depot + print("Label %s has no changelists - possibly deleted?" % name) if not commitFound: # We can't import this label; don't try again as it will get very # expensive repeatedly fetching all the files for labels that will # never be imported. If the label is moved in the future, the
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Signature · 1 line
2.19.2
RFC PATCH 2/2 git-p4: support loading changelist descriptions from files
In reply to Mazo, AndreyOur Perforce server experienced some kind of database corruption a few years ago. While the file data and revision history are mostly intact,
some metadata for several changesets got lost.
For example, inspecting certain changelists produces errors.
"""
$ p4 describe -s 12345 Date 2019/02/26 16:46:17: Operation: user-describe Operation 'user-describe' failed. Change 12345 description missing! """
While some metadata (like changeset descriptions) is obviously lost, most of it can be reconstructed via other commands:
* p4 changes -l -t //...@12345,12345 --
to obtain date+time, author, beginning of changeset description;
* p4 files -a //...@12345,12345 --
to obtain file revisions, file types, file actions;
* p4 diff2 -u //...@12344 //...@12345 --
to get a unified diff of text files in a changeset;
* p4 print -o binary.blob@12345 //depot/binary.blob@12345 -- to get a revision of a binary file.
It might be possible to teach git-p4 to fallback to other methods if p4 describe fails, but it's probably too special-cased (really depends on kind and scale of DB corruption), so some manual intervention is perhaps acceptable.
So, with some manual work, it's possible to reconstruct p4 -G describe ... output manually. In our case, once git-p4 passes p4 describe stage,
it can proceed further just fine.
Thus, it's tempting to feed resurrected metadata to git-p4 when a normal p4 describe would fail.
This functionality may be useful to cache changelist information, or to make some changes to changelist info before feeding it to git-p4.
A new config parameter is introduced to tell git-p4
to load certain changelist descriptions from files instead of from a server. For simplicity, it's one pickled file per changelist.
``` git config --add git-p4.damagedChangelists 12345.pickled git config --add git-p4.damagedChangelists 12346.pickled ```
The following trivial script may be used to produce pickled p4 -G describe-compatible output. """
#!/usr/bin/python2
import pickle
import time
# recovered commits of interest
changes = [
{
'change': '12345',
'status': 'submitted',
'code': 'stat',
'user': 'username1',
'time': str(int(time.mktime(time.strptime('2019/02/28 16:00:30', '%Y/%m/%d %H:%M:%S')))),
'client': 'username1_hostname1',
'desc': 'A bug is fixed.\nDetails are below:<lost>\n',
'depotFile0': '//depot/branch1/foo.sh',
'action0': 'edit',
'rev0': '28',
'type0': 'xtext',
'depotFile1': '//depot/branch1/bar.py',
'action1': 'edit',
'rev1': '43',
'type1': 'text',
'depotFile2': '//depot/branch1/baz.doc',
'action2': 'edit',
'rev2': '8',
'type2': 'binary',
'depotFile3': '//depot/branch1/qqq.c',
'action3': 'edit',
'rev3': '6',
'type3': 'ktext',
},
]for change in changes:
pickle.dump(change, open('{0}.pickled'.format(change['change']), 'wb'))"""
---
Notes:
Documentation changes and tests are obviously missing, but I hoped to get some feedback on the idea overall before working on those.
git-p4.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
git-p4.py +24 -1
@@ -24,10 +24,11 @@ import stat import zipfile import zlib import ctypes import errno +import pickle # support basestring in python3 try: unicode = unicode except NameError:
@@ -2615,10 +2616,12 @@ def __init__(self): self.knownAlienLabelBranches = {} self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60)) self.labels = {} + self.damagedChangelists = {} + # Force a checkpoint in fast-import and wait for it to finish def checkpoint(self): self.gitStream.write("checkpoint\n\n") self.gitStream.write("progress checkpoint\n\n") out = self.gitOutput.readline()
@@ -3312,10 +3315,25 @@ def getAlienLabelBranchMapping(self): for mapping in alienLabelBranches: if mapping: (alien, ours) = mapping.split(":") self.knownAlienLabelBranches[alien] = ours + def loadDamagedChangelists(self): + damagedChangelists = gitConfigList("git-p4.damagedChangelists") + for clPickled in damagedChangelists: + if not clPickled: + continue + + try: + clDesc = pickle.load(open(clPickled, 'rb')) + if not ("status" in clDesc and "user" in clDesc and "time" in clDesc and "change" in clDesc): + die("Changelist description read from {0} doesn't have required fields".format(clPickled)) + except (IOError, TypeError) as e: + die("Can't read changelist description dict from {0}: {1}".format(clPickled, str(e))) + + self.damagedChangelists[int(clDesc["change"])] = clDesc + def updateOptionDict(self, d): option_keys = {} if self.keepRepoPath: option_keys['keepRepoPath'] = 1
@@ -3413,11 +3431,14 @@ def searchParent(self, parent, branch, target): return None def importChanges(self, changes, origin_revision=0): cnt = 1 for change in changes: - description = p4_describe(change) + if change in self.damagedChangelists: + description = self.damagedChangelists[change] + else: + description = p4_describe(change) self.updateOptionDict(description) if not self.silent: sys.stdout.write("\rImporting revision %s (%s%%)" % (change, cnt * 100 / len(changes))) sys.stdout.flush()
@@ -3704,10 +3725,12 @@ def run(self, args): bad_changesfile = True break if bad_changesfile: die("Option --changesfile is incompatible with revision specifiers") + self.loadDamagedChangelists() + newPaths = [] for p in self.depotPaths: if p.find("@") != -1: atIdx = p.index("@") self.changeRange = p[atIdx:]
Signature · 1 line
2.19.2
Re: [RFC PATCH 2/2] git-p4: support loading changelist descriptions from files
Quoted 4 lines · Andrey
Our Perforce server experienced some kind of database corruption a few years ago. While the file data and revision history are mostly intact,
some metadata for several changesets got lost.
I think it's not unheard of for P4 databases to end up being corrupt, as in your case.
It looks like the RCS files get updated, but the database files (i.e. the metadata) do not, after which you have a bit of a problem.
So I guess this change could be quite useful, but it really needs some documentation and tests to support it - git-p4 is already complicated enough!
Your example script should probably use the same magic that the git-p4 script uses to pick the path to Python.
And perhaps come up with a nicer name than "damaged" - as you say, it could also be used for other purposes.
Quoted 178 lines
For example, inspecting certain changelists produces errors. """
$ p4 describe -s 12345 Date 2019/02/26 16:46:17: Operation: user-describe Operation 'user-describe' failed. Change 12345 description missing! """While some metadata (like changeset descriptions) is obviously lost, most of it can be reconstructed via other commands:
*p4 changes -l -t //...@12345,12345--
to obtain date+time, author, beginning of changeset description;
*p4 files -a //...@12345,12345--
to obtain file revisions, file types, file actions;
*p4 diff2 -u //...@12344 //...@12345--
to get a unified diff of text files in a changeset;
*p4 print -o binary.blob@12345 //depot/binary.blob@12345-- to get a revision of a binary file.It might be possible to teach git-p4 to fallback to other methods if
p4 describefails, but it's probably too special-cased (really depends on kind and scale of DB corruption), so some manual intervention is perhaps acceptable.So, with some manual work, it's possible to reconstruct
p4 -G describe ...output manually. In our case, once git-p4 passesp4 describestage,
it can proceed further just fine.
Thus, it's tempting to feed resurrected metadata to git-p4 when a normalp4 describewould fail.This functionality may be useful to cache changelist information, or to make some changes to changelist info before feeding it to git-p4.
A new config parameter is introduced to tell git-p4
to load certain changelist descriptions from files instead of from a server. For simplicity, it's one pickled file per changelist.``` git config --add git-p4.damagedChangelists 12345.pickled git config --add git-p4.damagedChangelists 12346.pickled ```The following trivial script may be used to produce pickled
p4 -G describe-compatible output. """
#!/usr/bin/python2import pickle
import time# recovered commits of interest changes = [ { 'change': '12345', 'status': 'submitted', 'code': 'stat', 'user': 'username1', 'time': str(int(time.mktime(time.strptime('2019/02/28 16:00:30', '%Y/%m/%d %H:%M:%S')))), 'client': 'username1_hostname1', 'desc': 'A bug is fixed.\nDetails are below:<lost>\n', 'depotFile0': '//depot/branch1/foo.sh', 'action0': 'edit', 'rev0': '28', 'type0': 'xtext', 'depotFile1': '//depot/branch1/bar.py', 'action1': 'edit', 'rev1': '43', 'type1': 'text', 'depotFile2': '//depot/branch1/baz.doc', 'action2': 'edit', 'rev2': '8', 'type2': 'binary', 'depotFile3': '//depot/branch1/qqq.c', 'action3': 'edit', 'rev3': '6', 'type3': 'ktext', }, ]for change in changes:
pickle.dump(change, open('{0}.pickled'.format(change['change']), 'wb'))"""
Signed-off-by Andrey Mazo <[email protected]>---
Notes:
Documentation changes and tests are obviously missing, but I hoped to get some feedback on the idea overall before working on those.git-p4.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)git-p4.py +24
-2@@ -24,10 +24,11 @@ import stat import zipfile import zlib import ctypes import errno +import pickle # support basestring in python3 try: unicode = unicode except NameError:@@ -2615,10 +2616,12 @@ def __init__(self): self.knownAlienLabelBranches = {} self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60)) self.labels = {} + self.damagedChangelists = {} + # Force a checkpoint in fast-import and wait for it to finish def checkpoint(self): self.gitStream.write("checkpoint\n\n") self.gitStream.write("progress checkpoint\n\n") out = self.gitOutput.readline()@@ -3312,10 +3315,25 @@ def getAlienLabelBranchMapping(self): for mapping in alienLabelBranches: if mapping: (alien, ours) = mapping.split(":") self.knownAlienLabelBranches[alien] = ours + def loadDamagedChangelists(self): + damagedChangelists = gitConfigList("git-p4.damagedChangelists") + for clPickled in damagedChangelists: + if not clPickled: + continue + + try: + clDesc = pickle.load(open(clPickled, 'rb')) + if not ("status" in clDesc and "user" in clDesc and "time" in clDesc and "change" in clDesc): + die("Changelist description read from {0} doesn't have required fields".format(clPickled)) + except (IOError, TypeError) as e: + die("Can't read changelist description dict from {0}: {1}".format(clPickled, str(e))) + + self.damagedChangelists[int(clDesc["change"])] = clDesc + def updateOptionDict(self, d): option_keys = {} if self.keepRepoPath: option_keys['keepRepoPath'] = 1@@ -3413,11 +3431,14 @@ def searchParent(self, parent, branch, target): return None def importChanges(self, changes, origin_revision=0): cnt = 1 for change in changes: - description = p4_describe(change) + if change in self.damagedChangelists: + description = self.damagedChangelists[change] + else: + description = p4_describe(change) self.updateOptionDict(description) if not self.silent: sys.stdout.write("\rImporting revision %s (%s%%)" % (change, cnt * 100 / len(changes))) sys.stdout.flush()@@ -3704,10 +3725,12 @@ def run(self, args): bad_changesfile = True break if bad_changesfile: die("Option --changesfile is incompatible with revision specifiers") + self.loadDamagedChangelists() + newPaths = [] for p in self.depotPaths: if p.find("@") != -1: atIdx = p.index("@") self.changeRange = p[atIdx:] --2.19.2
Re: [RFC PATCH 2/2] git-p4: support loading changelist descriptions
23.03.2019, 04:44, "Luke Diamand" <[email protected]>:
Quoted 10 lines · Andrey
Quoted 3 lines
Our Perforce server experienced some kind of database corruption a few years ago. While the file data and revision history are mostly intact,
some metadata for several changesets got lost.I think it's not unheard of for P4 databases to end up being corrupt, as in your case.
It looks like the RCS files get updated, but the database files (i.e. the metadata) do not, after which you have a bit of a problem.
Yeah, that's exactly what happened! :/
Quoted 3 lines
So I guess this change could be quite useful, but it really needs some documentation and tests to support it - git-p4 is already complicated enough!
Great, thanks for positive feedback.
Let me start working on these.
Luckily, the actual git-p4 changes are fairly straightforward.
Quoted 2 lines
Your example script should probably use the same magic that the git-p4 script uses to pick the path to Python.
Oh, you mean "#!/usr/bin/env python"?
Sure!
Actually, I also have a smarter "example script", which reconstructs p4 describe output from p4 files and p4 change without manual intervention. It's still short enough to be posted inside commit message, but would it make sense to put it under contrib/ and reference from the docs?
Quoted 2 lines
And perhaps come up with a nicer name than "damaged" - as you say, it could also be used for other purposes.
Does anything from the list below sound reasonable?
1. git-p4.loadChangelistInfo
2. git-p4.changelistInfoOverride
3. git-p4.cachedChangelistInfo
Quoted 1 line
Quoted 1 line
For simplicity, it's one pickled file per changelist.
(talking to myself)
I realized p4 -G describe produces a marshalled output, not pickled one.
Don't know why I decided to use pickle instead of marshal back when I wrote it.
I'll switch to marshal for serialization/deserialization in the next reroll.
It'll be easier for testing as well, since p4 -G describe output could be fed directly to damagedChangelists machinery without the need to convert it from marshal to pickle.
Thank you,
Andrey
v3 cover git-p4: a few assorted fixes for branches, excludes
In reply to Mazo, AndreyThis series fixes a few cases with branch detection and handling of excludes by git-p4.
This is the third iteration of the patch series. Changes since v2 [2]:
* Added new test cases for case-insensitive branch detection.
Changes since v1 [1]:
* Added new test case for excluded paths when detecting branches;
* Added a new fix for excluded paths when detecting branches.
[1] https://public-inbox.org/git/[email protected]
[2] https://public-inbox.org/git/[email protected]/
Range-diff vs v2:
1: 3ac39171d4 = 1: bd009a5ca5 git-p4: detect/prevent infinite loop in gitCommitByP4Change() 2: e644a8ab49 < -: ---------- git-p4: match branches case insensitively if configured -: ---------- > 2: 68b68ce1e4 git-p4: add failing test for "git-p4: match branches case insensitively if configured" -: ---------- > 3: 6eaad2582c git-p4: match branches case insensitively if configured 3: 44fed954dc = 4: 1bd5e170e0 git-p4: don't groom exclude path list on every commit 4: a0d3fa6add = 5: b657967154 git-p4: add failing test for "don't exclude other files with same prefix" 5: 3330f88a0d = 6: 035abfff2a git-p4: don't exclude other files with same prefix 6: 6170d45951 = 7: 2bde24b7e4 git-p4: add failing test for "git-p4: respect excluded paths when detecting branches" 7: 758d8e8486 = 8: 6d3ffb98a7 git-p4: respect excluded paths when detecting branches
Andrey Mazo (8):
git-p4: detect/prevent infinite loop in gitCommitByP4Change()
git-p4: add failing test for "git-p4: match branches case insensitively if configured"
git-p4: match branches case insensitively if configured
git-p4: don't groom exclude path list on every commit
git-p4: add failing test for "don't exclude other files with same prefix"
git-p4: don't exclude other files with same prefix
git-p4: add failing test for "git-p4: respect excluded paths when detecting branches"
git-p4: respect excluded paths when detecting branches
git-p4.py | 44 ++++++++----- t/t9801-git-p4-branch.sh | 132 ++++++++++++++++++++++++++++++++++++++ t/t9817-git-p4-exclude.sh | 51 +++++++++++++-- 3 files changed, 205 insertions(+), 22 deletions(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Signature · 1 line
2.19.2
PATCH v3 1/8 git-p4: detect/prevent infinite loop in gitCommitByP4Change()
Under certain circumstances, gitCommitByP4Change() can enter an infinite loop resulting in git p4 sync hanging forever.
The problem is thatgit rev-list --bisect <latest> ^<earliest> can return <latest>, which would result in reinspecting <latest> and potentially an infinite loop.
This can happen when importing just a subset of P4 repository and/or with explicit "--changesfile" option.
A real-life example:
"""
looking in ref refs/remotes/p4/mybranch for change 26894 using bisect...
Reading pipe: git rev-parse refs/remotes/p4/mybranch
trying: earliest latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git cat-file commit 147f5d3292af2e1cc4a56a7b96db845144c68486
current change 25339
trying: earliest ^147f5d3292af2e1cc4a56a7b96db845144c68486 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^147f5d3292af2e1cc4a56a7b96db845144c68486
Reading pipe: git cat-file commit 51db83df9d588010d0bd995641c85aa0408a5bb9
current change 25420
trying: earliest ^51db83df9d588010d0bd995641c85aa0408a5bb9 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^51db83df9d588010d0bd995641c85aa0408a5bb9
Reading pipe: git cat-file commit e8f83909ceb570f5a7e48c2853f3c5d8207cea52
current change 25448
trying: earliest ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52
Reading pipe: git cat-file commit 09a48eb7acd594dce52e06681be9c366e1844d66
current change 25521
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1
Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66
Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1
current change 26907
..."""
The fix is two-fold:
* detect an infinite loop and die right away
instead of looping forever;
* make sure, git rev-list --bisect can't return "latestCommit" again by excluding it from the rev-list range explicitly.
---
Notes:
I don't have a simple test-case for this yet,
and I was able to perform a few complex initial git p4 sync runs without hitting this problem.
I suspect, I had somehow messed up with branch definitions and --changesfile option at some point.
git-p4.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
git-p4.py +3 -1
@@ -3323,11 +3323,13 @@ def gitCommitByP4Change(self, ref, change): return next if currentChange < change: earliestCommit = "^%s" % next else: - latestCommit = "%s" % next + if next == latestCommit: + die("Infinite loop while looking in ref %s for change %s. Check your branch mappings" % (ref, change)) + latestCommit = "%s^@" % next return "" def importNewBranch(self, branch, maxChange): # make fast-import flush all changes to disk and update the refs using the checkpoint
Signature · 1 line
2.19.2
PATCH v3 2/8 git-p4: add failing test for "git-p4: match branches case insensitively if configured"
In reply to Mazo, AndreyIn preparation for a fix, add a failing test case to test that git-p4 doesn't fold the case in file paths
when doing branch detection case insensitively. (i.e. when core.ignorecase is set)
--- t/t9801-git-p4-branch.sh | 92 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+)
t/t9801-git-p4-branch.sh +92 -0
@@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie cd branch1 && grep "client spec" file1 ) ' +test_expect_success 'restart p4d (case folding enabled)' ' + kill_p4d && + start_p4d -C1 +' + +# +# 1: //depot/main/mf1 +# 2: integrate //depot/main/... -> //depot/branch1/... +# 3: //depot/main/mf2 +# 4: //depot/BRANCH1/B1f3 +# 5: //depot/branch1/b1f4 +# +test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' ' + ( + cd "$cli" && + mkdir -p main && + + echo mf1 >main/mf1 && + p4 add main/mf1 && + p4 submit -d "main/mf1" && + + p4 integrate //depot/main/... //depot/branch1/... && + p4 submit -d "integrate main to branch1" && + + echo mf2 >main/mf2 && + p4 add main/mf2 && + p4 submit -d "main/mf2" && + + mkdir BRANCH1 && + echo B1f3 >BRANCH1/B1f3 && + p4 add BRANCH1/B1f3 && + p4 submit -d "BRANCH1/B1f3" && + + echo b1f4 >branch1/b1f4 && + p4 add branch1/b1f4 && + p4 submit -d "branch1/b1f4" + ) +' + +# Check that files are properly split across branches when ignorecase is set +test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) +' + +# Check that files are properly split across branches when ignorecase is set, use-client-spec case +test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' ' + client_view "//depot/... //client/..." && + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --use-client-spec --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' test_done
Signature · 1 line
2.19.2
Re: [PATCH v3 2/8] git-p4: add failing test for "git-p4: match branches case insensitively if configured"
Hi Junio and Andrey,
Quoted 5 lines · Andrey
t/t9801-git-p4-branch.sh +0
-0
t/t9801-git-p4-branch.sh test_expect_success 'Update a file in git side and submit to P4 using client vie · line 616
Quoted 9 lines
t/t9801-git-p4-branch.sh +4
-0@@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie cd branch1 && grep "client spec" file1 ) ' +test_expect_success 'restart p4d (case folding enabled)' ' + kill_p4d && + start_p4d -C1 +'
There is a semantic conflict between this patch and commit 07353d9042 (git p4 test: clean up the p4d cleanup functions, 2019-03-13) in 'sg/test-atexit' currently cooking in 'pu': this patch adds a new callsite of 'kill_p4d', but that commit renamed that function to 'stop_and_cleanup_p4d'. Consequently, t9801 on 'pu' now fails with:
+kill_p4d
t9801-git-p4-branch.sh: 4: eval: kill_p4d: not found
error: last command exited with $?=127
not ok 28 - restart p4d (case folding enabled)
https://travis-ci.org/git/git/jobs/514513463#L5827
I wonder whether it would be worth amending 07353d9042 to keep 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the time being.
Re: [PATCH v3 2/8] git-p4: add failing test for "git-p4: match branches case insensitively if configured"
Quoted 27 lines · Andrey
Quoted 5 lines
t/t9801-git-p4-branch.sh +0
-0t/t9801-git-p4-branch.sh test_expect_success 'Update a file in git side and submit to P4 using client vie · line 616
Quoted 9 lines
t/t9801-git-p4-branch.sh +4
-0@@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie cd branch1 && grep "client spec" file1 ) ' +test_expect_success 'restart p4d (case folding enabled)' ' + kill_p4d && + start_p4d -C1 +'There is a semantic conflict between this patch and commit
07353d9042(git p4 test: clean up the p4d cleanup functions, 2019-03-13) in 'sg/test-atexit' currently cooking in 'pu': this patch adds a new callsite of 'kill_p4d', but that commit renamed that function to 'stop_and_cleanup_p4d'. Consequently, t9801 on 'pu' now fails with:+kill_p4d
t9801-git-p4-branch.sh: 4: eval:kill_p4d: not found
error: last command exited with $?=127
not ok 28 - restart p4d (case folding enabled)
Quoted 8 lines
https://travis-ci.org/git/git/jobs/514513463#L5827
I wonder whether it would be worth amending
07353d9042to keep 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the time being.
Good catch!
Don't know what's the proper workflow here, but I see 2 more options:
* Resolve the conflict in t/t9801-git-p4-branch.sh while merging am/p4-branches-excludes commit d15068a650 ("git-p4: respect excluded paths when detecting branches", 2019-04-01)
* I can rebase my git-p4 changes on top of sg/test-atexit branch
commit 74ec8cf674 ("t9811-git-p4-label-import: fix pipeline negation", 2019-03-13)
In case this might be helpful,
I did a conflict resolution locally,
(by doing git checkout d15068a650; git merge 74ec8cf674) and here's the patch of the merge.
Basically,
* a newly added "kill_p4d" is replaced with "stop_and_cleanup_p4d"; and
* "kill_p4d" in the end of the script is removed.
t/t9801-git-p4-branch.sh +0 -0
@@@ -610,4 -650,100 +650,96 @@@ test_expect_success 'Update a file in g ) ' + test_expect_success 'restart p4d (case folding enabled)' ' - kill_p4d && ++ stop_and_cleanup_p4d && + start_p4d -C1 + ' + + # + # 1: //depot/main/mf1 + # 2: integrate //depot/main/... -> //depot/branch1/... + # 3: //depot/main/mf2 + # 4: //depot/BRANCH1/B1f3 + # 5: //depot/branch1/b1f4 + # + test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' ' + ( + cd "$cli" && + mkdir -p main && + + echo mf1 >main/mf1 && + p4 add main/mf1 && + p4 submit -d "main/mf1" && + + p4 integrate //depot/main/... //depot/branch1/... && + p4 submit -d "integrate main to branch1" && + + echo mf2 >main/mf2 && + p4 add main/mf2 && + p4 submit -d "main/mf2" && + + mkdir BRANCH1 && + echo B1f3 >BRANCH1/B1f3 && + p4 add BRANCH1/B1f3 && + p4 submit -d "BRANCH1/B1f3" && + + echo b1f4 >branch1/b1f4 && + p4 add branch1/b1f4 && + p4 submit -d "branch1/b1f4" + ) + ' + + # Check that files are properly split across branches when ignorecase is set + test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) + ' + + # Check that files are properly split across branches when ignorecase is set, use-client-spec case + test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' ' + client_view "//depot/... //client/..." && + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList main:branch1 && + git config --type=bool core.ignoreCase true && + git p4 clone --dest=. --use-client-spec --detect-branches //depot@all && + + git log --all --graph --decorate --stat && + + git reset --hard p4/master && + test_path_is_file mf1 && + test_path_is_file mf2 && + test_path_is_missing B1f3 && + test_path_is_missing b1f4 && + + git reset --hard p4/depot/branch1 && + test_path_is_file mf1 && + test_path_is_missing mf2 && + test_path_is_file B1f3 && + test_path_is_file b1f4 + ) + ' + -test_expect_success 'kill p4d' ' - kill_p4d -' - test_done
Re: [PATCH v3 2/8] git-p4: add failing test for "git-p4: match branches case insensitively if configured"
In reply to SZEDER GáborQuoted 3 lines · SZEDER Gábor
I wonder whether it would be worth amending
07353d9042to keep 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the time being.
I think renaming was the right thing to do; "show --cc" shows that the post-image lives in the new world order where calling kill_p4d is not usually needed clearly with a hunk that replaces kill_p4d (which only existed in the old world) with stop_and_cleanup (which exists only in the new world).
Will redo the merge and teach my rerere database.
Thanks.
PATCH v3 3/8 git-p4: match branches case insensitively if configured
In reply to Mazo, Andreygit-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.
--- git-p4.py | 4 ++-- t/t9801-git-p4-branch.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
git-p4.py +2 -2
@@ -2666,11 +2666,11 @@ def stripRepoPath(self, path, prefixes): # 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 + "/"): + if p4PathStartsWith(path, 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
@@ -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
t/t9801-git-p4-branch.sh +2 -2
@@ -648,11 +648,11 @@ test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' ' p4 submit -d "branch1/b1f4" ) ' # Check that files are properly split across branches when ignorecase is set -test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' ' +test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' ' test_when_finished cleanup_git && test_create_repo "$git" && ( cd "$git" && git config git-p4.branchList main:branch1 &&
@@ -674,11 +674,11 @@ test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch defini test_path_is_file b1f4 ) ' # Check that files are properly split across branches when ignorecase is set, use-client-spec case -test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' ' +test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' ' client_view "//depot/... //client/..." && test_when_finished cleanup_git && test_create_repo "$git" && ( cd "$git" &&
Signature · 1 line
2.19.2
PATCH v3 4/8 git-p4: don't groom exclude path list on every commit
In reply to Mazo, AndreyCurrently, cloneExclude array is being groomed (by removing trailing "...") on every changeset.
(since extractFilesFromCommit() is called on every imported changeset)
As a micro-optimization, do it once while parsing arguments. Also, prepend "/" and remove trailing "..." at the same time.
--- git-p4.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
git-p4.py +7 -5
@@ -1314,11 +1314,11 @@ class Command: def __init__(self): self.usage = "usage: %prog [options]" self.needsGit = True self.verbose = False - # This is required for the "append" cloneExclude action + # This is required for the "append" update_shelve action def ensure_value(self, attr, value): if not hasattr(self, attr) or getattr(self, attr) is None: setattr(self, attr, value) return getattr(self, attr)
@@ -2528,10 +2528,15 @@ def map_in_client(self, depot_path): return self.client_spec_path_cache[depot_path] die( "Error: %s is not found in client spec path" % depot_path ) return "" +def cloneExcludeCallback(option, opt_str, value, parser): + # prepend "/" because the first "/" was consumed as part of the option itself. + # ("-//depot/A/..." becomes "/depot/A/..." after option parsing) + parser.values.cloneExclude += ["/" + re.sub(r"\.\.\.$", "", value)] + class P4Sync(Command, P4UserMap): def __init__(self): Command.__init__(self) P4UserMap.__init__(self)
@@ -2551,11 +2556,11 @@ def __init__(self): optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true', help="Keep entire BRANCH/DIR/SUBDIR prefix during import"), optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true', help="Only sync files that are included in the Perforce Client Spec"), optparse.make_option("-/", dest="cloneExclude", - action="append", type="string", + action="callback", callback=cloneExcludeCallback, type="string", help="exclude depot path"), ] self.description = """Imports from Perforce into a git repository.\n example: //depot/my/project/ -- to import the current head
@@ -2617,12 +2622,10 @@ def checkpoint(self): out = self.gitOutput.readline() if self.verbose: print("checkpoint finished: " + out) def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0): - self.cloneExclude = [re.sub(r"\.\.\.$", "", path) - for path in self.cloneExclude] files = [] fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum]
@@ -3888,11 +3891,10 @@ def run(self, args): if not self.cloneDestination and len(depotPaths) > 1: self.cloneDestination = depotPaths[-1] depotPaths = depotPaths[:-1] - self.cloneExclude = ["/"+p for p in self.cloneExclude] for p in depotPaths: if not p.startswith("//"): sys.stderr.write('Depot paths must start with "//": %s\n' % p) return False
Signature · 1 line
2.19.2
PATCH v3 5/8 git-p4: add failing test for "don't exclude other files with same prefix"
In reply to Mazo, AndreyIn preparation for a fix, add a failing test case to test that git-p4 doesn't exclude files with the same prefix unintentionally when exclude paths are specified without a trailing /.
I.e., don't exclude "//depot/file_dont_exclude" if run with "-//depot/file". or don't exclude "//depot/discard_file_not" if run with "-//depot/discard_file".
--- t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 5 deletions(-)
t/t9817-git-p4-exclude.sh +46 -5
@@ -20,49 +20,90 @@ test_expect_success 'create exclude repo' ' ( cd "$cli" && mkdir -p wanted discard && echo wanted >wanted/foo && echo discard >discard/foo && - p4 add wanted/foo discard/foo && + echo discard_file >discard_file && + echo discard_file_not >discard_file_not && + p4 add wanted/foo discard/foo discard_file discard_file_not && p4 submit -d "initial revision" ) ' test_expect_success 'check the repo was created correctly' ' test_when_finished cleanup_git && git p4 clone --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo && - test_path_is_file discard/foo + test_path_is_file discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'clone, excluding part of repo' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo && - test_path_is_missing discard/foo + test_path_is_missing discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not + ) +' + +test_expect_failure 'clone, excluding single file, no trailing /' ' + test_when_finished cleanup_git && + git p4 clone -//depot/discard_file --dest="$git" //depot/...@all && + ( + cd "$git" && + test_path_is_file wanted/foo && + test_path_is_file discard/foo && + test_path_is_missing discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'clone, then sync with exclude' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... --dest="$git" //depot/...@all && ( cd "$cli" && - p4 edit wanted/foo discard/foo && + p4 edit wanted/foo discard/foo discard_file_not && date >>wanted/foo && date >>discard/foo && + date >>discard_file_not && p4 submit -d "updating" && cd "$git" && git p4 sync -//depot/discard/... && test_path_is_file wanted/foo && - test_path_is_missing discard/foo + test_path_is_missing discard/foo && + test_path_is_file discard_file && + test_path_is_file discard_file_not + ) +' + +test_expect_failure 'clone, then sync with exclude, no trailing /' ' + test_when_finished cleanup_git && + git p4 clone -//depot/discard/... -//depot/discard_file --dest="$git" //depot/...@all && + ( + cd "$cli" && + p4 edit wanted/foo discard/foo discard_file_not && + date >>wanted/foo && + date >>discard/foo && + date >>discard_file_not && + p4 submit -d "updating" && + + cd "$git" && + git p4 sync -//depot/discard/... -//depot/discard_file && + test_path_is_file wanted/foo && + test_path_is_missing discard/foo && + test_path_is_missing discard_file && + test_path_is_file discard_file_not ) ' test_expect_success 'kill p4d' ' kill_p4d
Signature · 1 line
2.19.2
PATCH v3 6/8 git-p4: don't exclude other files with same prefix
In reply to Mazo, AndreyMake sure not to exclude files unintentionally
if exclude paths are specified without a trailing /.
I.e., don't exclude "//depot/file_dont_exclude" if run with "-//depot/file".
Do this by ensuring that paths without a trailing "/" are only matched completely.
Also, abort path search on the first match as a micro-optimization.
--- git-p4.py | 21 ++++++++++++++------- t/t9817-git-p4-exclude.sh | 4 ++-- 2 files changed, 16 insertions(+), 9 deletions(-)
git-p4.py +14 -7
@@ -2621,22 +2621,29 @@ def checkpoint(self): self.gitStream.write("progress checkpoint\n\n") out = self.gitOutput.readline() if self.verbose: print("checkpoint finished: " + out) + def isPathWanted(self, path): + for p in self.cloneExclude: + if p.endswith("/"): + if p4PathStartsWith(path, p): + return False + # "-//depot/file1" without a trailing "/" should only exclude "file1", but not "file111" or "file1_dir/file2" + elif path.lower() == p.lower(): + return False + for p in self.depotPaths: + if p4PathStartsWith(path, p): + return True + return False + def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0): files = [] fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum] - - if [p for p in self.cloneExclude - if p4PathStartsWith(path, p)]: - found = False - else: - found = [p for p in self.depotPaths - if p4PathStartsWith(path, p)] + found = self.isPathWanted(path) if not found: fnum = fnum + 1 continue file = {}
t/t9817-git-p4-exclude.sh +2 -2
@@ -51,11 +51,11 @@ test_expect_success 'clone, excluding part of repo' ' test_path_is_file discard_file && test_path_is_file discard_file_not ) ' -test_expect_failure 'clone, excluding single file, no trailing /' ' +test_expect_success 'clone, excluding single file, no trailing /' ' test_when_finished cleanup_git && git p4 clone -//depot/discard_file --dest="$git" //depot/...@all && ( cd "$git" && test_path_is_file wanted/foo &&
@@ -83,11 +83,11 @@ test_expect_success 'clone, then sync with exclude' ' test_path_is_file discard_file && test_path_is_file discard_file_not ) ' -test_expect_failure 'clone, then sync with exclude, no trailing /' ' +test_expect_success 'clone, then sync with exclude, no trailing /' ' test_when_finished cleanup_git && git p4 clone -//depot/discard/... -//depot/discard_file --dest="$git" //depot/...@all && ( cd "$cli" && p4 edit wanted/foo discard/foo discard_file_not &&
Signature · 1 line
2.19.2
PATCH v3 7/8 git-p4: add failing test for "git-p4: respect excluded paths when detecting branches"
In reply to Mazo, AndreyIn preparation for a fix, add a failing test case to test that git-p4 doesn't exclude files despite being told to when handling multiple branches.
I.e., it should exclude //depot/branch2/file2 when run with -//depot/branch2/file2, but doesn't do this right now.
The test is based on 'git p4 clone complex branches' test with the following changes:
* account for file3 moved from branch3 to branch4 in test 'git p4 submit to two branches in a single changelist';
* account for branch6 created in test 'git p4 clone file subset branch';
* file2 is expected to be missing from all branches due to explicit exclude.
--- t/t9801-git-p4-branch.sh | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
t/t9801-git-p4-branch.sh +40 -0
@@ -409,10 +409,50 @@ test_expect_failure 'git p4 clone file subset branch' ' test_path_is_missing file2 && test_path_is_missing file3 ) ' +# Check that excluded files are omitted during import +test_expect_failure 'git p4 clone complex branches with excluded files' ' + test_when_finished cleanup_git && + test_create_repo "$git" && + ( + cd "$git" && + git config git-p4.branchList branch1:branch2 && + git config --add git-p4.branchList branch1:branch3 && + git config --add git-p4.branchList branch1:branch4 && + git config --add git-p4.branchList branch1:branch5 && + git config --add git-p4.branchList branch1:branch6 && + git p4 clone --dest=. --detect-branches -//depot/branch1/file2 -//depot/branch2/file2 -//depot/branch3/file2 -//depot/branch4/file2 -//depot/branch5/file2 -//depot/branch6/file2 //depot@all && + git log --all --graph --decorate --stat && + git reset --hard p4/depot/branch1 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_file file3 && + git reset --hard p4/depot/branch2 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_missing file3 && + git reset --hard p4/depot/branch3 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_missing file3 && + git reset --hard p4/depot/branch4 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_file file3 && + git reset --hard p4/depot/branch5 && + test_path_is_file file1 && + test_path_is_missing file2 && + test_path_is_file file3 && + git reset --hard p4/depot/branch6 && + test_path_is_file file1 && + test_path_is_missing file2 && + 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" &&
Signature · 1 line
2.19.2
PATCH v3 8/8 git-p4: respect excluded paths when detecting branches
In reply to Mazo, AndreyCurrently, excluded paths are only handled in the following cases:
* no branch detection;
* branch detection with using clientspec.
However, excluded paths are not respected in case of branch detection without using clientspec.
Fix this by consulting the list of excluded paths
when splitting files across branches.
--- git-p4.py | 3 +-- t/t9801-git-p4-branch.sh | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-)
git-p4.py +1 -2
@@ -2708,12 +2708,11 @@ def splitFilesIntoBranches(self, commit): branches = {} fnum = 0 while "depotFile%s" % fnum in commit: path = commit["depotFile%s" % fnum] - found = [p for p in self.depotPaths - if p4PathStartsWith(path, p)] + found = self.isPathWanted(path) if not found: fnum = fnum + 1 continue file = {}
t/t9801-git-p4-branch.sh +1 -1
@@ -410,11 +410,11 @@ test_expect_failure 'git p4 clone file subset branch' ' test_path_is_missing file3 ) ' # Check that excluded files are omitted during import -test_expect_failure 'git p4 clone complex branches with excluded files' ' +test_expect_success 'git p4 clone complex branches with excluded files' ' test_when_finished cleanup_git && test_create_repo "$git" && ( cd "$git" && git config git-p4.branchList branch1:branch2 &&
Signature · 1 line
2.19.2
Quoted 6 lines
This series fixes a few cases with branch detection and handling of excludes by git-p4.
This is the third iteration of the patch series. Changes since v2 [2]:
* Added new test cases for case-insensitive branch detection.
Forgot to add:
* Added a fix for case-insensitive branch detection when running with useClientSpec enabled. (range diff below correctly shows the e644a8ab49 vs 6eaad2582c difference)
Quoted 8 lines
Changes since v1 [1]:
* Added new test case for excluded paths when detecting branches;
* Added a new fix for excluded paths when detecting branches.[1] https://public-inbox.org/git/[email protected]
[2] https://public-inbox.org/git/[email protected]/Range-diff vs v2:
Quoted 9 lines
1: 3ac39171d4 = 1: bd009a5ca5 git-p4: detect/prevent infinite loop in gitCommitByP4Change() 2: e644a8ab49 < -: ---------- git-p4: match branches case insensitively if configured -: ---------- > 2: 68b68ce1e4 git-p4: add failing test for "git-p4: match branches case insensitively if configured" -: ---------- > 3: 6eaad2582c git-p4: match branches case insensitively if configured 3: 44fed954dc = 4: 1bd5e170e0 git-p4: don't groom exclude path list on every commit 4: a0d3fa6add = 5: b657967154 git-p4: add failing test for "don't exclude other files with same prefix" 5: 3330f88a0d = 6: 035abfff2a git-p4: don't exclude other files with same prefix 6: 6170d45951 = 7: 2bde24b7e4 git-p4: add failing test for "git-p4: respect excluded paths when detecting branches" 7: 758d8e8486 = 8: 6d3ffb98a7 git-p4: respect excluded paths when detecting branches
--
Andrey
v2 cover git-p4: inexact labels and load changelist description from file
In reply to Mazo, AndreyThis patch series introduces two experimental features to git-p4, unrelated to each other.
1. The first patch adds support for "inexact" label detection.
The feature lets git-p4 find a git commit for a Perforce label even if there is no git commit with exact same changelist number as in Perforce. It is particularly useful when splitting a large Perforce depot into multiple git repositories
or when importing just a subset of a depot into git.
2. The second patch adds support for loading changelist description from a file. (p4 -G describe equivalent)
The original use case is to be able to migrate a Perforce depot, which database got a little bit corrupted, into git.
This patch series should be applied on top of
"[PATCH v3 0/8] git-p4: a few assorted fixes for branches, excludes" [1]
This is the second iteration of the patch series.
Changes since the v1 [1]:
* Dropped "alien" branch feature;
* Added "inexact" label feature by suggestion from Luke;
* Added minimal documentation;
* Changed "damaged"-oriented narrative to more generic one. (renamed "git-p4.damagedChangelists" to "git-p4.changelistDescriptionFile", functions and variables correspondingly)
Range-diff vs v1:
1: b02df749b9 < -: ---------- git-p4: introduce alien branch mappings -: ---------- > 1: 54ef897fcf git-p4: inexact label detection 2: bb3e14a389 ! 2: 83b0034538 git-p4: support loading changelist descriptions from files
[1] https://public-inbox.org/git/[email protected]/
Andrey Mazo (2):
git-p4: inexact label detection
git-p4: support loading changelist descriptions from files
Documentation/git-p4.txt | 41 +++++++++++++++++++++++ git-p4.py | 72 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 106 insertions(+), 7 deletions(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
prerequisite-patch-id: 23e039fec7a1f5c51c98326a14d788adb1ecb5ba
prerequisite-patch-id: 9840851ffca6f00126c9c91da5a8828c7d0dcaed
prerequisite-patch-id: 32a738b41fb3dccfbbfb4d382a9748e36dcdfa8b
prerequisite-patch-id: 10661f77392f4131d2375976c77a7cd231fdf9ab
prerequisite-patch-id: a55360c904eba1b9e9c934405d3141eb96c5ad30
prerequisite-patch-id: 46357586199c02d956d53d782a12f1ee0c991302
prerequisite-patch-id: c683e7d6017580df9385a1544af409ca615d770c
prerequisite-patch-id: 411dcb5e95aff036e0cb3e850ea75f2424b260a6
Signature · 1 line
2.19.2
RFC PATCH v2 1/2 git-p4: inexact label detection
Labels in Perforce are not global, but can be placed on a particular view/subdirectory. This might pose difficulties when importing only parts of Perforce depot into a git repository. For example:
1. Depot layout is as follows:
//depot/metaproject/branch1/subprojectA/...
//depot/metaproject/branch1/subprojectB/...
//depot/metaproject/branch2/subprojectA/...
//depot/metaproject/branch2/subprojectB/... 2. Labels are placed as follows:
* label 1A on //depot/metaproject/branch1/subprojectA/...
* label 1B on //depot/metaproject/branch1/subprojectB/...
* label 2A on //depot/metaproject/branch2/subprojectA/...
* label 2B on //depot/metaproject/branch2/subprojectB/...
3. The goal is to import
subprojectA into subprojectA.git and
subprojectB into subprojectB.git
preserving all the branches and labels.
4. Importing subprojectA.
Label 1A is imported fine because it's placed on certain commit on branch1. However, label 1B is not imported because it's placed on a commit in another subproject:
git-p4 says: "importing label 1B: could not find git commit for changelist ..." The same is with label 2A, which is imported; and 2B, which is not.
Currently, there is no easy way (that I'm aware of) to tell git-p4 to
import an empty commit into a desired branch,
so that a label placed on that changelist could be imported as well,
It might be possible to get a similar effect by importing both subprojectA and B in a single git repo, and then running git filter-branch --subdirectory-filter subprojectA,
but this might produce way more irrelevant empty commits, than needed for labels. (although imported changelists can be limited with git-p4 --changesfile option)
Also, git filter-branch is harder to use for incremental imports
or when changes are submitted from git back to Perforce.
As suggested by Luke,
instead of creating an empty commit for the sole purpose of being tagged later, teach git-p4 to search harder for the next lower changelist, corresponding to the label in question.
Do this by finding the highest changelist up to the label under all known branches, (branches are finalized by the time importP4Labels() runs)
and using it instead of a depot-wide changelist corresponding to the label.
This new behavior may not be desired for people,
who want exact label <-> changelist relationship.
So, add a new boolean config parameter git-p4.allowInexactLabels (defaults to false) to explicitly enable it if needed.
Also, this behavior only appears to be useful in case of multiple branches, (otherwise, every Perforce changelist should appear in git)
so it's not engaged when running without branch detection.
Detect and report (--verbose) "inexact" tags,
i.e. tags placed on a lower changelist than was in Perforce. Implement this by comparing a changelist for which a commit was found with a changelist corresponding to the label on the whole depot.
Note, that the new "inexact" logic works slower
than the original code in case of numerous branches,
because p4 needs to calculate the most recent change for each branch path instead of just one.
This is an alternative solution to "alien" branches concept proposed earlier:
https://public-inbox.org/git/b02df749b9266ac8c73707617a171122156621ab.1553283214.git.amazo@checkvideo.com/
--- Documentation/git-p4.txt | 14 ++++++++++++ git-p4.py | 48 +++++++++++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 6 deletions(-)
Documentation/git-p4.txt +14 -0
@@ -582,10 +582,24 @@ git-p4.importLabels:: git-p4.labelImportRegexp:: Only p4 labels matching this regular expression will be imported. The default value is '[a-zA-Z0-9_\-.]+$'. +git-p4.allowInexactLabels:: + Only has an effect if run with `--detect-branches`. + By default, when performing p4 label import, + 'git p4' finds a changelist number of every label, + then finds a git commit corresponding to the found changelist number, + and then places an annotated tag on the found git commit. + If a git commit is not found, the label is considered unimportable + and is added to 'ignoredP4Labels' list. + If 'allowInexactLabels' is set to true, + 'git p4' only considers changelists under branches being imported. + This has an effect that a tag in git might be placed on a lower changelist compared to p4. + This might be useful when importing just a subset of the depot into git, + if a label would be discarded otherwise. + git-p4.useClientSpec:: Specify that the p4 client spec should be used to identify p4 depot paths of interest. This is equivalent to specifying the option `--use-client-spec`. See the "CLIENT SPEC" section above. This variable is a boolean, not the name of a p4 client.
git-p4.py +42 -6
73 lines not on this page · raw message
Signature · 1 line
2.19.2
RFC PATCH v2 2/2 git-p4: support loading changelist descriptions from files
In reply to Mazo, AndreyOur Perforce server experienced some kind of database corruption a few years ago. While the file data and revision history are mostly intact,
some metadata for several changesets got lost.
For example, inspecting certain changelists produces errors.
"""
$ p4 describe -s 12345 Date 2019/02/26 16:46:17: Operation: user-describe Operation 'user-describe' failed. Change 12345 description missing! """
While some metadata (like changeset descriptions) is obviously lost, most of it can be reconstructed via other commands:
* p4 changes -l -t //...@12345,12345 --
to obtain date+time, author, beginning of changeset description;
* p4 files -a //...@12345,12345 --
to obtain file revisions, file types, file actions;
* p4 diff2 -u //...@12344 //...@12345 --
to get a unified diff of text files in a changeset;
* p4 print -o binary.blob@12345 //depot/binary.blob@12345 -- to get a revision of a binary file.
It might be possible to teach git-p4 to fallback to other methods if p4 describe fails, but it's probably too special-cased (really depends on kind and scale of DB corruption), so some manual intervention is perhaps acceptable.
So, with some manual work, it's possible to reconstruct p4 -G describe ... output manually. In our case, once git-p4 passes p4 describe stage,
it can proceed further just fine.
Thus, it's tempting to feed resurrected metadata to git-p4 when a normal p4 describe would fail.
This functionality may be useful to cache changelist information, or to make some changes to changelist info before feeding it to git-p4.
A new config parameter is introduced to tell git-p4
to load certain changelist descriptions from files instead of from a server. For simplicity, it's one marshalled file per changelist.
``` git config --add git-p4.changelistDescriptionFile 12345.marshal git config --add git-p4.changelistDescriptionFile 12346.marshal ```
The following trivial script may be used to produce marshalled p4 -G describe-compatible output. """
#!/usr/bin/env python
import marshal
import time
# recovered commits of interest
changes = [
{
'change': '12345',
'status': 'submitted',
'code': 'stat',
'user': 'username1',
'time': str(int(time.mktime(time.strptime('2019/02/28 16:00:30', '%Y/%m/%d %H:%M:%S')))),
'client': 'username1_hostname1',
'desc': 'A bug is fixed.\nDetails are below:<lost>\n',
'depotFile0': '//depot/branch1/foo.sh',
'action0': 'edit',
'rev0': '28',
'type0': 'xtext',
'depotFile1': '//depot/branch1/bar.py',
'action1': 'edit',
'rev1': '43',
'type1': 'text',
'depotFile2': '//depot/branch1/baz.doc',
'action2': 'edit',
'rev2': '8',
'type2': 'binary',
'depotFile3': '//depot/branch1/qqq.c',
'action3': 'edit',
'rev3': '6',
'type3': 'ktext',
},
]for change in changes:
marshal.dump(change, open('{0}.marshal'.format(change['change']), 'wb'))"""
Or, the following script may be used to produce marshalled p4 -G describe-compatible output for our particular database corruption.
"""
#!/usr/bin/env python
import itertools
import marshal
import subprocess
import tempfile
def p4_unmarshal(fileobj):
result = []
while True:
try:
result += [marshal.load(fileobj)]
except EOFError:
break
return result def p4_describe_fallback(cl):
with tempfile.TemporaryFile() as p4_changes_output:
with tempfile.TemporaryFile() as p4_files_output:
subprocess.check_call(['p4', '-G', 'changes', '-l', '-t', '//...@{0},{0}'.format(cl)], stdout=p4_changes_output)
subprocess.check_call(['p4', '-G', 'files', '-a', '//...@{0},{0}'.format(cl)], stdout=p4_files_output)
p4_changes_output.seek(0)
p4_files_output.seek(0)
p4_changes_unmarshalled = p4_unmarshal(p4_changes_output)
p4_files_unmarshalled = p4_unmarshal(p4_files_output)
described_cl = p4_changes_unmarshalled[0] # there is usually only one entry
described_cl['desc'] += '<lost>\n'
assert described_cl['change'] == str(cl)
for (file_info, i) in itertools.izip(p4_files_unmarshalled, itertools.count()):
for f in ('depotFile', 'action', 'rev', 'type'):
described_cl['{}{}'.format(f, i)] = file_info[f]
assert file_info['change'] == described_cl['change']
return described_cl cls_wanted = ( 12345, 12346 )
for cl in cls_wanted:
with open('{0}.marshal'.format(cl), 'wb') as f:
cl_info = p4_describe_fallback(cl) marshal.dump(cl_info, f)
"""
--- Documentation/git-p4.txt | 27 +++++++++++++++++++++++++++ git-p4.py | 24 +++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-)
Documentation/git-p4.txt +27 -0
@@ -654,10 +654,37 @@ git config --add git-p4.mapUser "p4user = First Last <[email protected]>" ------------- + A mapping will override any user information from P4. Mappings for multiple P4 user can be defined. +git-p4.changelistDescriptionFile:: + This config variable points 'git p4' to a file, + containing a serialized (marshalled) changelist description. + 'git p4' loads a description from such a file + instead of asking Perforce server about it. + The file format is the same as produced by 'p4 -G describe -s <changelist>'. + This option can be specified multiple times + to feed multiple changelist descriptions to 'git p4'. + The path is relative to git work tree, + file names or extensions don't matter. + This example loads 2 changelist descriptions: ++ +------------- +git config --add git-p4.changelistDescriptionFile cl-12345.marshal +git config --add git-p4.changelistDescriptionFile cl-12347.marshal +------------- ++ +Under some circumstances (for example, Perforce database corruption) +this option is useful to supply changelist description to 'git p4' bypassing 'p4'. +Also, it can be used for caching of changelist descriptions +to reduce load on the Perforce server in case of successive imports +(say, when splitting the depot into multiple Git repositories) +or for overriding some changelist information. +This config variable is generally not needed after the initial import, +so it can be removed from the config file +together with corresponding description files after the import. + Submit variables ~~~~~~~~~~~~~~~~ git-p4.detectRenames:: Detect renames. See linkgit:git-diff[1]. This can be true, false, or a score as expected by 'git diff -M'.
git-p4.py +23 -1
@@ -2613,10 +2613,12 @@ def __init__(self): self.initialParents = {} self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60)) self.labels = {} + self.loadedChangelistDescriptions = {} + # Force a checkpoint in fast-import and wait for it to finish def checkpoint(self): self.gitStream.write("checkpoint\n\n") self.gitStream.write("progress checkpoint\n\n") out = self.gitOutput.readline()
@@ -3319,10 +3321,25 @@ def getBranchMappingFromGitBranches(self): branch = "main" else: branch = branch[len(self.projectName):] self.knownBranches[branch] = branch + def loadChangelistDescFromFile(self): + changelistDescriptionFiles = gitConfigList("git-p4.changelistDescriptionFile") + for clMarshalledDescFile in changelistDescriptionFiles: + if not clMarshalledDescFile: + continue + + try: + with open(clMarshalledDescFile, 'rb') as clFileObj: + clDesc = marshal.load(clFileObj) + if not ("status" in clDesc and "user" in clDesc and "time" in clDesc and "change" in clDesc): + die("Changelist description read from {0} doesn't have required fields".format(clMarshalledDescFile)) + self.loadedChangelistDescriptions[int(clDesc["change"])] = clDesc + except (IOError, TypeError, ValueError, EOFError) as e: + die("Can't read changelist description from {0}: {1}".format(clMarshalledDescFile, str(e))) + def updateOptionDict(self, d): option_keys = {} if self.keepRepoPath: option_keys['keepRepoPath'] = 1
@@ -3420,11 +3437,14 @@ def searchParent(self, parent, branch, target): return None def importChanges(self, changes, origin_revision=0): cnt = 1 for change in changes: - description = p4_describe(change) + if change in self.loadedChangelistDescriptions: + description = self.loadedChangelistDescriptions[change] + else: + description = p4_describe(change) self.updateOptionDict(description) if not self.silent: sys.stdout.write("\rImporting revision %s (%s%%)" % (change, cnt * 100 / len(changes))) sys.stdout.flush()
@@ -3706,10 +3726,12 @@ def run(self, args): bad_changesfile = True break if bad_changesfile: die("Option --changesfile is incompatible with revision specifiers") + self.loadChangelistDescFromFile() + newPaths = [] for p in self.depotPaths: if p.find("@") != -1: atIdx = p.index("@") self.changeRange = p[atIdx:]
Signature · 1 line
2.19.2