{"thread":{"id":"50645","subject":"[PATCH 0/5] git-p4: a few assorted fixes for branches, excludes","startedAt":"2019-03-04T17:34:45Z","lastAt":"2019-04-03T07:10:55Z","messageCount":40,"participants":["Mazo, Andrey","Luke Diamand","SZEDER Gábor","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"370619","messageId":"cover.1551485349.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":null,"subject":"[PATCH 0/5] git-p4: a few assorted fixes for branches, excludes","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-04T17:34:38Z","receivedAt":"2019-03-04T17:34:45Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"This series fixes a couple of corner cases with branch detection\nand handling of excludes by git-p4.\n\nAndrey Mazo (5):\n git-p4: detect/prevent infinite loop in gitCommitByP4Change()\n git-p4: match branches case insensitively if configured\n git-p4: don't groom exclude path list on every commit\n git-p4: add failing test for \"don't exclude other files with same prefix\"\n git-p4: don't exclude other files with same prefix\n\n git-p4.py | 39 +++++++++++++++++++-----------\n t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++----\n 2 files changed, 71 insertions(+), 19 deletions(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\n-- \n2.19.2\n\n"},{"id":"370620","messageId":"3ac39171d441b84a20d5e918a9995e8d8de627c5.1551485349.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1551485349.git.amazo@checkvideo.com","subject":"[PATCH 1/5] git-p4: detect/prevent infinite loop in gitCommitByP4Change()","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-04T17:34:42Z","receivedAt":"2019-03-04T17:34:47Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Under certain circumstances, gitCommitByP4Change() can enter an infinite\nloop resulting in `git p4 sync` hanging forever.\n\nThe problem is that\n`git rev-list --bisect ^` can return ``,\nwhich would result in reinspecting and potentially an infinite loop.\n\nThis can happen when importing just a subset of P4 repository\nand/or with explicit \"--changesfile\" option.\n\nA real-life example:\n\"\"\"\n looking in ref refs/remotes/p4/mybranch for change 26894 using bisect...\n Reading pipe: git rev-parse refs/remotes/p4/mybranch\n trying: earliest latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git cat-file commit 147f5d3292af2e1cc4a56a7b96db845144c68486\n current change 25339\n trying: earliest ^147f5d3292af2e1cc4a56a7b96db845144c68486 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^147f5d3292af2e1cc4a56a7b96db845144c68486\n Reading pipe: git cat-file commit 51db83df9d588010d0bd995641c85aa0408a5bb9\n current change 25420\n trying: earliest ^51db83df9d588010d0bd995641c85aa0408a5bb9 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^51db83df9d588010d0bd995641c85aa0408a5bb9\n Reading pipe: git cat-file commit e8f83909ceb570f5a7e48c2853f3c5d8207cea52\n current change 25448\n trying: earliest ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52\n Reading pipe: git cat-file commit 09a48eb7acd594dce52e06681be9c366e1844d66\n current change 25521\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n ...\n\"\"\"\n\nThe fix is two-fold:\n * detect an infinite loop and die right away\n instead of looping forever;\n * make sure, `git rev-list --bisect` can't return \"latestCommit\" again\n by excluding it from the rev-list range explicitly.\n\nSigned-off-by: Andrey Mazo \n---\n\nNotes:\n I don't have a simple test-case for this yet,\n and I was able to perform a few complex initial `git p4 sync` runs\n without hitting this problem.\n \n I suspect, I had somehow messed up with branch definitions\n and --changesfile option at some point.\n\n git-p4.py | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 5b79920f46..c0a3068b6f 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3323,11 +3323,13 @@ def gitCommitByP4Change(self, ref, change):\n return next\n \n if currentChange < change:\n earliestCommit = \"^%s\" % next\n else:\n- latestCommit = \"%s\" % next\n+ if next == latestCommit:\n+ die(\"Infinite loop while looking in ref %s for change %s. Check your branch mappings\" % (ref, change))\n+ latestCommit = \"%s^@\" % next\n \n return \"\"\n \n def importNewBranch(self, branch, maxChange):\n # make fast-import flush all changes to disk and update the refs using the checkpoint\n-- \n2.19.2\n\n"},{"id":"370621","messageId":"e644a8ab4928349ed83ac9ab6ffdbcafc3a3a7b5.1551485349.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1551485349.git.amazo@checkvideo.com","subject":"[PATCH 2/5] git-p4: match branches case insensitively if configured","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-04T17:34:44Z","receivedAt":"2019-03-04T17:34:49Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"git-p4 knows how to handle case insensitivity in file paths\nif core.ignorecase is set.\nHowever, when determining a branch for a file,\nit still does a case-sensitive prefix match.\nThis may result in some file changes to be lost on import.\n\nFor example, given the following commits\n 1. add //depot/main/file1\n 2. add //depot/DirA/file2\n 3. add //depot/dira/file3\n 4. add //depot/DirA/file4\nand \"branchList = main:DirA\" branch mapping,\ncommit 3 will be lost.\n\nSo, do branch search case insensitively if running with core.ignorecase set.\nTeach splitFilesIntoBranches() to use the p4PathStartsWith() function\nfor path prefix matches instead of always case-sensitive match.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c0a3068b6f..91c610f960 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2721,11 +2721,11 @@ def splitFilesIntoBranches(self, commit):\n relPath = self.stripRepoPath(path, self.depotPaths)\n \n for branch in self.knownBranches.keys():\n # add a trailing slash so that a commit into qt/4.2foo\n # doesn't end up in qt/4.2, e.g.\n- if relPath.startswith(branch + \"/\"):\n+ if p4PathStartsWith(relPath, branch + \"/\"):\n if branch not in branches:\n branches[branch] = []\n branches[branch].append(file)\n break\n \n-- \n2.19.2\n\n"},{"id":"370622","messageId":"44fed954dc4ee7d98ce518c0665cc71a0751dd3b.1551485349.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1551485349.git.amazo@checkvideo.com","subject":"[PATCH 3/5] git-p4: don't groom exclude path list on every commit","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-04T17:34:46Z","receivedAt":"2019-03-04T17:34:51Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Currently, `cloneExclude` array is being groomed (by removing trailing \"...\")\non every changeset.\n(since `extractFilesFromCommit()` is called on every imported changeset)\n\nAs a micro-optimization, do it once while parsing arguments.\nAlso, prepend \"/\" and remove trailing \"...\" at the same time.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 91c610f960..a9f53e5b88 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1314,11 +1314,11 @@ class Command:\n def __init__(self):\n self.usage = \"usage: %prog [options]\"\n self.needsGit = True\n self.verbose = False\n \n- # This is required for the \"append\" cloneExclude action\n+ # This is required for the \"append\" update_shelve action\n def ensure_value(self, attr, value):\n if not hasattr(self, attr) or getattr(self, attr) is None:\n setattr(self, attr, value)\n return getattr(self, attr)\n \n@@ -2528,10 +2528,15 @@ def map_in_client(self, depot_path):\n return self.client_spec_path_cache[depot_path]\n \n die( \"Error: %s is not found in client spec path\" % depot_path )\n return \"\"\n \n+def cloneExcludeCallback(option, opt_str, value, parser):\n+ # prepend \"/\" because the first \"/\" was consumed as part of the option itself.\n+ # (\"-//depot/A/...\" becomes \"/depot/A/...\" after option parsing)\n+ parser.values.cloneExclude += [\"/\" + re.sub(r\"\\.\\.\\.$\", \"\", value)]\n+\n class P4Sync(Command, P4UserMap):\n \n def __init__(self):\n Command.__init__(self)\n P4UserMap.__init__(self)\n@@ -2551,11 +2556,11 @@ def __init__(self):\n optparse.make_option(\"--keep-path\", dest=\"keepRepoPath\", action='store_true',\n help=\"Keep entire BRANCH/DIR/SUBDIR prefix during import\"),\n optparse.make_option(\"--use-client-spec\", dest=\"useClientSpec\", action='store_true',\n help=\"Only sync files that are included in the Perforce Client Spec\"),\n optparse.make_option(\"-/\", dest=\"cloneExclude\",\n- action=\"append\", type=\"string\",\n+ action=\"callback\", callback=cloneExcludeCallback, type=\"string\",\n help=\"exclude depot path\"),\n ]\n self.description = \"\"\"Imports from Perforce into a git repository.\\n\n example:\n //depot/my/project/ -- to import the current head\n@@ -2617,12 +2622,10 @@ def checkpoint(self):\n out = self.gitOutput.readline()\n if self.verbose:\n print(\"checkpoint finished: \" + out)\n \n def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n- self.cloneExclude = [re.sub(r\"\\.\\.\\.$\", \"\", path)\n- for path in self.cloneExclude]\n files = []\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n \n@@ -3888,11 +3891,10 @@ def run(self, args):\n \n if not self.cloneDestination and len(depotPaths) > 1:\n self.cloneDestination = depotPaths[-1]\n depotPaths = depotPaths[:-1]\n \n- self.cloneExclude = [\"/\"+p for p in self.cloneExclude]\n for p in depotPaths:\n if not p.startswith(\"//\"):\n sys.stderr.write('Depot paths must start with \"//\": %s\\n' % p)\n return False\n \n-- \n2.19.2\n\n"},{"id":"370623","messageId":"a0d3fa6add2e9284167457e95d316e689ad798d5.1551485349.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1551485349.git.amazo@checkvideo.com","subject":"[PATCH 4/5] git-p4: add failing test for \"don't exclude other files with same prefix\"","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-04T17:34:49Z","receivedAt":"2019-03-04T17:34:54Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"In preparation for a fix, add a failing test case to test that\ngit-p4 doesn't exclude files with the same prefix unintentionally\nwhen exclude paths are specified without a trailing /.\nI.e., don't exclude \"//depot/file_dont_exclude\" if run with \"-//depot/file\".\nor don't exclude \"//depot/discard_file_not\" if run with \"-//depot/discard_file\".\n\nSigned-off-by: Andrey Mazo \n---\n t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++----\n 1 file changed, 46 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh\nindex aac568eadf..1c22570797 100755\n--- a/t/t9817-git-p4-exclude.sh\n+++ b/t/t9817-git-p4-exclude.sh\n@@ -20,49 +20,90 @@ test_expect_success 'create exclude repo' '\n \t(\n \t\tcd \"$cli\" &&\n \t\tmkdir -p wanted discard &&\n \t\techo wanted >wanted/foo &&\n \t\techo discard >discard/foo &&\n-\t\tp4 add wanted/foo discard/foo &&\n+\t\techo discard_file >discard_file &&\n+\t\techo discard_file_not >discard_file_not &&\n+\t\tp4 add wanted/foo discard/foo discard_file discard_file_not &&\n \t\tp4 submit -d \"initial revision\"\n \t)\n '\n \n test_expect_success 'check the repo was created correctly' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_file discard/foo\n+\t\ttest_path_is_file discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'clone, excluding part of repo' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_missing discard/foo\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n+\t)\n+'\n+\n+test_expect_failure 'clone, excluding single file, no trailing /' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_path_is_file wanted/foo &&\n+\t\ttest_path_is_file discard/foo &&\n+\t\ttest_path_is_missing discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'clone, then sync with exclude' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$cli\" &&\n-\t\tp4 edit wanted/foo discard/foo &&\n+\t\tp4 edit wanted/foo discard/foo discard_file_not &&\n \t\tdate >>wanted/foo &&\n \t\tdate >>discard/foo &&\n+\t\tdate >>discard_file_not &&\n \t\tp4 submit -d \"updating\" &&\n \n \t\tcd \"$git\" &&\n \t\tgit p4 sync -//depot/discard/... &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_missing discard/foo\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n+\t)\n+'\n+\n+test_expect_failure 'clone, then sync with exclude, no trailing /' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone -//depot/discard/... -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 edit wanted/foo discard/foo discard_file_not &&\n+\t\tdate >>wanted/foo &&\n+\t\tdate >>discard/foo &&\n+\t\tdate >>discard_file_not &&\n+\t\tp4 submit -d \"updating\" &&\n+\n+\t\tcd \"$git\" &&\n+\t\tgit p4 sync -//depot/discard/... -//depot/discard_file &&\n+\t\ttest_path_is_file wanted/foo &&\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_missing discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'kill p4d' '\n \tkill_p4d\n-- \n2.19.2\n\n"},{"id":"370624","messageId":"3330f88a0d1ccd8aa1a376ee8c543690ac983958.1551485349.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1551485349.git.amazo@checkvideo.com","subject":"[PATCH 5/5] git-p4: don't exclude other files with same prefix","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-04T17:34:51Z","receivedAt":"2019-03-04T17:34:56Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Make sure not to exclude files unintentionally\nif exclude paths are specified without a trailing /.\nI.e., don't exclude \"//depot/file_dont_exclude\" if run with \"-//depot/file\".\n\nDo this by ensuring that paths without a trailing \"/\" are only matched completely.\n\nAlso, abort path search on the first match as a micro-optimization.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 21 ++++++++++++++-------\n t/t9817-git-p4-exclude.sh | 4 ++--\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex a9f53e5b88..162877aa82 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2621,22 +2621,29 @@ def checkpoint(self):\n self.gitStream.write(\"progress checkpoint\\n\\n\")\n out = self.gitOutput.readline()\n if self.verbose:\n print(\"checkpoint finished: \" + out)\n \n+ def isPathWanted(self, path):\n+ for p in self.cloneExclude:\n+ if p.endswith(\"/\"):\n+ if p4PathStartsWith(path, p):\n+ return False\n+ # \"-//depot/file1\" without a trailing \"/\" should only exclude \"file1\", but not \"file111\" or \"file1_dir/file2\"\n+ elif path.lower() == p.lower():\n+ return False\n+ for p in self.depotPaths:\n+ if p4PathStartsWith(path, p):\n+ return True\n+ return False\n+\n def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n files = []\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n-\n- if [p for p in self.cloneExclude\n- if p4PathStartsWith(path, p)]:\n- found = False\n- else:\n- found = [p for p in self.depotPaths\n- if p4PathStartsWith(path, p)]\n+ found = self.isPathWanted(path)\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\ndiff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh\nindex 1c22570797..275dd30425 100755\n--- a/t/t9817-git-p4-exclude.sh\n+++ b/t/t9817-git-p4-exclude.sh\n@@ -51,11 +51,11 @@ test_expect_success 'clone, excluding part of repo' '\n \t\ttest_path_is_file discard_file &&\n \t\ttest_path_is_file discard_file_not\n \t)\n '\n \n-test_expect_failure 'clone, excluding single file, no trailing /' '\n+test_expect_success 'clone, excluding single file, no trailing /' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n@@ -83,11 +83,11 @@ test_expect_success 'clone, then sync with exclude' '\n \t\ttest_path_is_file discard_file &&\n \t\ttest_path_is_file discard_file_not\n \t)\n '\n \n-test_expect_failure 'clone, then sync with exclude, no trailing /' '\n+test_expect_success 'clone, then sync with exclude, no trailing /' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$cli\" &&\n \t\tp4 edit wanted/foo discard/foo discard_file_not &&\n-- \n2.19.2\n\n"},{"id":"372182","messageId":"cover.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1551485349.git.amazo@checkvideo.com","subject":"[PATCH v2 0/7] git-p4: a few assorted fixes for branches, excludes","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:32:35Z","receivedAt":"2019-03-21T22:32:48Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"This series fixes a few cases with branch detection\nand handling of excludes by git-p4.\n\nThis is the second iteration of the patch series.\nChanges since the v1 [1]:\n * Added new test case for excluded paths when detecting branches;\n * Added a new fix for excluded paths when detecting branches.\n\n[1] https://public-inbox.org/git/cover.1551485349.git.amazo@checkvideo.com\n\nRange-diff vs v1:\n1: 3ac39171d4 = 1: 3ac39171d4 git-p4: detect/prevent infinite loop in gitCommitByP4Change()\n2: e644a8ab49 = 2: e644a8ab49 git-p4: match branches case insensitively if configured\n3: 44fed954dc = 3: 44fed954dc git-p4: don't groom exclude path list on every commit\n4: a0d3fa6add = 4: a0d3fa6add git-p4: add failing test for \"don't exclude other files with same prefix\"\n5: 3330f88a0d = 5: 3330f88a0d git-p4: don't exclude other files with same prefix\n-: ---------- > 6: 6170d45951 git-p4: add failing test for \"git-p4: respect excluded paths when detecting branches\"\n-: ---------- > 7: 758d8e8486 git-p4: respect excluded paths when detecting branches\n\nAndrey Mazo (7):\n git-p4: detect/prevent infinite loop in gitCommitByP4Change()\n git-p4: match branches case insensitively if configured\n git-p4: don't groom exclude path list on every commit\n git-p4: add failing test for \"don't exclude other files with same prefix\"\n git-p4: don't exclude other files with same prefix\n git-p4: add failing test for \"git-p4: respect excluded paths when detecting branches\"\n git-p4: respect excluded paths when detecting branches\n\n git-p4.py | 42 ++++++++++++++++++++------------\n t/t9801-git-p4-branch.sh | 40 ++++++++++++++++++++++++++++++\n t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++----\n 3 files changed, 112 insertions(+), 21 deletions(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\n-- \n2.19.2\n\n"},{"id":"372183","messageId":"e644a8ab4928349ed83ac9ab6ffdbcafc3a3a7b5.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v2 2/7] git-p4: match branches case insensitively if configured","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:32:54Z","receivedAt":"2019-03-21T22:32:58Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"git-p4 knows how to handle case insensitivity in file paths\nif core.ignorecase is set.\nHowever, when determining a branch for a file,\nit still does a case-sensitive prefix match.\nThis may result in some file changes to be lost on import.\n\nFor example, given the following commits\n 1. add //depot/main/file1\n 2. add //depot/DirA/file2\n 3. add //depot/dira/file3\n 4. add //depot/DirA/file4\nand \"branchList = main:DirA\" branch mapping,\ncommit 3 will be lost.\n\nSo, do branch search case insensitively if running with core.ignorecase set.\nTeach splitFilesIntoBranches() to use the p4PathStartsWith() function\nfor path prefix matches instead of always case-sensitive match.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c0a3068b6f..91c610f960 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2721,11 +2721,11 @@ def splitFilesIntoBranches(self, commit):\n relPath = self.stripRepoPath(path, self.depotPaths)\n \n for branch in self.knownBranches.keys():\n # add a trailing slash so that a commit into qt/4.2foo\n # doesn't end up in qt/4.2, e.g.\n- if relPath.startswith(branch + \"/\"):\n+ if p4PathStartsWith(relPath, branch + \"/\"):\n if branch not in branches:\n branches[branch] = []\n branches[branch].append(file)\n break\n \n-- \n2.19.2\n\n"},{"id":"372184","messageId":"3ac39171d441b84a20d5e918a9995e8d8de627c5.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v2 1/7] git-p4: detect/prevent infinite loop in gitCommitByP4Change()","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:32:45Z","receivedAt":"2019-03-21T22:33:31Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Under certain circumstances, gitCommitByP4Change() can enter an infinite\nloop resulting in `git p4 sync` hanging forever.\n\nThe problem is that\n`git rev-list --bisect ^` can return ``,\nwhich would result in reinspecting and potentially an infinite loop.\n\nThis can happen when importing just a subset of P4 repository\nand/or with explicit \"--changesfile\" option.\n\nA real-life example:\n\"\"\"\n looking in ref refs/remotes/p4/mybranch for change 26894 using bisect...\n Reading pipe: git rev-parse refs/remotes/p4/mybranch\n trying: earliest latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git cat-file commit 147f5d3292af2e1cc4a56a7b96db845144c68486\n current change 25339\n trying: earliest ^147f5d3292af2e1cc4a56a7b96db845144c68486 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^147f5d3292af2e1cc4a56a7b96db845144c68486\n Reading pipe: git cat-file commit 51db83df9d588010d0bd995641c85aa0408a5bb9\n current change 25420\n trying: earliest ^51db83df9d588010d0bd995641c85aa0408a5bb9 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^51db83df9d588010d0bd995641c85aa0408a5bb9\n Reading pipe: git cat-file commit e8f83909ceb570f5a7e48c2853f3c5d8207cea52\n current change 25448\n trying: earliest ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52\n Reading pipe: git cat-file commit 09a48eb7acd594dce52e06681be9c366e1844d66\n current change 25521\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n ...\n\"\"\"\n\nThe fix is two-fold:\n * detect an infinite loop and die right away\n instead of looping forever;\n * make sure, `git rev-list --bisect` can't return \"latestCommit\" again\n by excluding it from the rev-list range explicitly.\n\nSigned-off-by: Andrey Mazo \n---\n\nNotes:\n I don't have a simple test-case for this yet,\n and I was able to perform a few complex initial `git p4 sync` runs\n without hitting this problem.\n \n I suspect, I had somehow messed up with branch definitions\n and --changesfile option at some point.\n\n git-p4.py | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 5b79920f46..c0a3068b6f 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3323,11 +3323,13 @@ def gitCommitByP4Change(self, ref, change):\n return next\n \n if currentChange < change:\n earliestCommit = \"^%s\" % next\n else:\n- latestCommit = \"%s\" % next\n+ if next == latestCommit:\n+ die(\"Infinite loop while looking in ref %s for change %s. Check your branch mappings\" % (ref, change))\n+ latestCommit = \"%s^@\" % next\n \n return \"\"\n \n def importNewBranch(self, branch, maxChange):\n # make fast-import flush all changes to disk and update the refs using the checkpoint\n-- \n2.19.2\n\n"},{"id":"372185","messageId":"44fed954dc4ee7d98ce518c0665cc71a0751dd3b.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v2 3/7] git-p4: don't groom exclude path list on every commit","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:32:57Z","receivedAt":"2019-03-21T22:33:34Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Currently, `cloneExclude` array is being groomed (by removing trailing \"...\")\non every changeset.\n(since `extractFilesFromCommit()` is called on every imported changeset)\n\nAs a micro-optimization, do it once while parsing arguments.\nAlso, prepend \"/\" and remove trailing \"...\" at the same time.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 91c610f960..a9f53e5b88 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1314,11 +1314,11 @@ class Command:\n def __init__(self):\n self.usage = \"usage: %prog [options]\"\n self.needsGit = True\n self.verbose = False\n \n- # This is required for the \"append\" cloneExclude action\n+ # This is required for the \"append\" update_shelve action\n def ensure_value(self, attr, value):\n if not hasattr(self, attr) or getattr(self, attr) is None:\n setattr(self, attr, value)\n return getattr(self, attr)\n \n@@ -2528,10 +2528,15 @@ def map_in_client(self, depot_path):\n return self.client_spec_path_cache[depot_path]\n \n die( \"Error: %s is not found in client spec path\" % depot_path )\n return \"\"\n \n+def cloneExcludeCallback(option, opt_str, value, parser):\n+ # prepend \"/\" because the first \"/\" was consumed as part of the option itself.\n+ # (\"-//depot/A/...\" becomes \"/depot/A/...\" after option parsing)\n+ parser.values.cloneExclude += [\"/\" + re.sub(r\"\\.\\.\\.$\", \"\", value)]\n+\n class P4Sync(Command, P4UserMap):\n \n def __init__(self):\n Command.__init__(self)\n P4UserMap.__init__(self)\n@@ -2551,11 +2556,11 @@ def __init__(self):\n optparse.make_option(\"--keep-path\", dest=\"keepRepoPath\", action='store_true',\n help=\"Keep entire BRANCH/DIR/SUBDIR prefix during import\"),\n optparse.make_option(\"--use-client-spec\", dest=\"useClientSpec\", action='store_true',\n help=\"Only sync files that are included in the Perforce Client Spec\"),\n optparse.make_option(\"-/\", dest=\"cloneExclude\",\n- action=\"append\", type=\"string\",\n+ action=\"callback\", callback=cloneExcludeCallback, type=\"string\",\n help=\"exclude depot path\"),\n ]\n self.description = \"\"\"Imports from Perforce into a git repository.\\n\n example:\n //depot/my/project/ -- to import the current head\n@@ -2617,12 +2622,10 @@ def checkpoint(self):\n out = self.gitOutput.readline()\n if self.verbose:\n print(\"checkpoint finished: \" + out)\n \n def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n- self.cloneExclude = [re.sub(r\"\\.\\.\\.$\", \"\", path)\n- for path in self.cloneExclude]\n files = []\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n \n@@ -3888,11 +3891,10 @@ def run(self, args):\n \n if not self.cloneDestination and len(depotPaths) > 1:\n self.cloneDestination = depotPaths[-1]\n depotPaths = depotPaths[:-1]\n \n- self.cloneExclude = [\"/\"+p for p in self.cloneExclude]\n for p in depotPaths:\n if not p.startswith(\"//\"):\n sys.stderr.write('Depot paths must start with \"//\": %s\\n' % p)\n return False\n \n-- \n2.19.2\n\n"},{"id":"372186","messageId":"a0d3fa6add2e9284167457e95d316e689ad798d5.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v2 4/7] git-p4: add failing test for \"don't exclude other files with same prefix\"","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:33:00Z","receivedAt":"2019-03-21T22:33:36Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"In preparation for a fix, add a failing test case to test that\ngit-p4 doesn't exclude files with the same prefix unintentionally\nwhen exclude paths are specified without a trailing /.\nI.e., don't exclude \"//depot/file_dont_exclude\" if run with \"-//depot/file\".\nor don't exclude \"//depot/discard_file_not\" if run with \"-//depot/discard_file\".\n\nSigned-off-by: Andrey Mazo \n---\n t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++----\n 1 file changed, 46 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh\nindex aac568eadf..1c22570797 100755\n--- a/t/t9817-git-p4-exclude.sh\n+++ b/t/t9817-git-p4-exclude.sh\n@@ -20,49 +20,90 @@ test_expect_success 'create exclude repo' '\n \t(\n \t\tcd \"$cli\" &&\n \t\tmkdir -p wanted discard &&\n \t\techo wanted >wanted/foo &&\n \t\techo discard >discard/foo &&\n-\t\tp4 add wanted/foo discard/foo &&\n+\t\techo discard_file >discard_file &&\n+\t\techo discard_file_not >discard_file_not &&\n+\t\tp4 add wanted/foo discard/foo discard_file discard_file_not &&\n \t\tp4 submit -d \"initial revision\"\n \t)\n '\n \n test_expect_success 'check the repo was created correctly' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_file discard/foo\n+\t\ttest_path_is_file discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'clone, excluding part of repo' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_missing discard/foo\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n+\t)\n+'\n+\n+test_expect_failure 'clone, excluding single file, no trailing /' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_path_is_file wanted/foo &&\n+\t\ttest_path_is_file discard/foo &&\n+\t\ttest_path_is_missing discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'clone, then sync with exclude' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$cli\" &&\n-\t\tp4 edit wanted/foo discard/foo &&\n+\t\tp4 edit wanted/foo discard/foo discard_file_not &&\n \t\tdate >>wanted/foo &&\n \t\tdate >>discard/foo &&\n+\t\tdate >>discard_file_not &&\n \t\tp4 submit -d \"updating\" &&\n \n \t\tcd \"$git\" &&\n \t\tgit p4 sync -//depot/discard/... &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_missing discard/foo\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n+\t)\n+'\n+\n+test_expect_failure 'clone, then sync with exclude, no trailing /' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone -//depot/discard/... -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 edit wanted/foo discard/foo discard_file_not &&\n+\t\tdate >>wanted/foo &&\n+\t\tdate >>discard/foo &&\n+\t\tdate >>discard_file_not &&\n+\t\tp4 submit -d \"updating\" &&\n+\n+\t\tcd \"$git\" &&\n+\t\tgit p4 sync -//depot/discard/... -//depot/discard_file &&\n+\t\ttest_path_is_file wanted/foo &&\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_missing discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'kill p4d' '\n \tkill_p4d\n-- \n2.19.2\n\n"},{"id":"372187","messageId":"3330f88a0d1ccd8aa1a376ee8c543690ac983958.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v2 5/7] git-p4: don't exclude other files with same prefix","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:33:02Z","receivedAt":"2019-03-21T22:33:40Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Make sure not to exclude files unintentionally\nif exclude paths are specified without a trailing /.\nI.e., don't exclude \"//depot/file_dont_exclude\" if run with \"-//depot/file\".\n\nDo this by ensuring that paths without a trailing \"/\" are only matched completely.\n\nAlso, abort path search on the first match as a micro-optimization.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 21 ++++++++++++++-------\n t/t9817-git-p4-exclude.sh | 4 ++--\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex a9f53e5b88..162877aa82 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2621,22 +2621,29 @@ def checkpoint(self):\n self.gitStream.write(\"progress checkpoint\\n\\n\")\n out = self.gitOutput.readline()\n if self.verbose:\n print(\"checkpoint finished: \" + out)\n \n+ def isPathWanted(self, path):\n+ for p in self.cloneExclude:\n+ if p.endswith(\"/\"):\n+ if p4PathStartsWith(path, p):\n+ return False\n+ # \"-//depot/file1\" without a trailing \"/\" should only exclude \"file1\", but not \"file111\" or \"file1_dir/file2\"\n+ elif path.lower() == p.lower():\n+ return False\n+ for p in self.depotPaths:\n+ if p4PathStartsWith(path, p):\n+ return True\n+ return False\n+\n def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n files = []\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n-\n- if [p for p in self.cloneExclude\n- if p4PathStartsWith(path, p)]:\n- found = False\n- else:\n- found = [p for p in self.depotPaths\n- if p4PathStartsWith(path, p)]\n+ found = self.isPathWanted(path)\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\ndiff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh\nindex 1c22570797..275dd30425 100755\n--- a/t/t9817-git-p4-exclude.sh\n+++ b/t/t9817-git-p4-exclude.sh\n@@ -51,11 +51,11 @@ test_expect_success 'clone, excluding part of repo' '\n \t\ttest_path_is_file discard_file &&\n \t\ttest_path_is_file discard_file_not\n \t)\n '\n \n-test_expect_failure 'clone, excluding single file, no trailing /' '\n+test_expect_success 'clone, excluding single file, no trailing /' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n@@ -83,11 +83,11 @@ test_expect_success 'clone, then sync with exclude' '\n \t\ttest_path_is_file discard_file &&\n \t\ttest_path_is_file discard_file_not\n \t)\n '\n \n-test_expect_failure 'clone, then sync with exclude, no trailing /' '\n+test_expect_success 'clone, then sync with exclude, no trailing /' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$cli\" &&\n \t\tp4 edit wanted/foo discard/foo discard_file_not &&\n-- \n2.19.2\n\n"},{"id":"372188","messageId":"6170d45951d71171ea3ad502a3b2a5c5c55c12f8.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v2 6/7] git-p4: add failing test for \"git-p4: respect excluded paths when detecting branches\"","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:33:05Z","receivedAt":"2019-03-21T22:33:42Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"In preparation for a fix, add a failing test case to test that\ngit-p4 doesn't exclude files despite being told to\nwhen handling multiple branches.\n\nI.e., it should exclude //depot/branch2/file2 when run with -//depot/branch2/file2,\nbut doesn't do this right now.\n\nThe test is based on 'git p4 clone complex branches' test with the following changes:\n * account for file3 moved from branch3 to branch4 in test 'git p4 submit to two branches in a single changelist';\n * account for branch6 created in test 'git p4 clone file subset branch';\n * file2 is expected to be missing from all branches due to explicit exclude.\n\nSigned-off-by: Andrey Mazo \n---\n t/t9801-git-p4-branch.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 6a86d6996b..4729f470b2 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -409,10 +409,50 @@ test_expect_failure 'git p4 clone file subset branch' '\n \t\ttest_path_is_missing file2 &&\n \t\ttest_path_is_missing file3\n \t)\n '\n \n+# Check that excluded files are omitted during import\n+test_expect_failure 'git p4 clone complex branches with excluded files' '\n+\ttest_when_finished cleanup_git &&\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.branchList branch1:branch2 &&\n+\t\tgit config --add git-p4.branchList branch1:branch3 &&\n+\t\tgit config --add git-p4.branchList branch1:branch4 &&\n+\t\tgit config --add git-p4.branchList branch1:branch5 &&\n+\t\tgit config --add git-p4.branchList branch1:branch6 &&\n+\t\tgit 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 &&\n+\t\tgit log --all --graph --decorate --stat &&\n+\t\tgit reset --hard p4/depot/branch1 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\tgit reset --hard p4/depot/branch2 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_missing file3 &&\n+\t\tgit reset --hard p4/depot/branch3 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_missing file3 &&\n+\t\tgit reset --hard p4/depot/branch4 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\tgit reset --hard p4/depot/branch5 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\tgit reset --hard p4/depot/branch6 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_missing file3\n+\t)\n+'\n+\n # From a report in http://stackoverflow.com/questions/11893688\n # where --use-client-spec caused branch prefixes not to be removed;\n # every file in git appeared into a subdirectory of the branch name.\n test_expect_success 'use-client-spec detect-branches setup' '\n \trm -rf \"$cli\" &&\n-- \n2.19.2\n\n"},{"id":"372189","messageId":"758d8e84868edc4b53c382a74655048d69d187de.1553207234.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v2 7/7] git-p4: respect excluded paths when detecting branches","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-21T22:33:07Z","receivedAt":"2019-03-21T22:33:44Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Currently, excluded paths are only handled in the following cases:\n * no branch detection;\n * branch detection with using clientspec.\n\nHowever, excluded paths are not respected in case of\nbranch detection without using clientspec.\n\nFix this by consulting the list of excluded paths\nwhen splitting files across branches.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 3 +--\n t/t9801-git-p4-branch.sh | 2 +-\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 162877aa82..148ea6f1b0 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2708,12 +2708,11 @@ def splitFilesIntoBranches(self, commit):\n \n branches = {}\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n- found = [p for p in self.depotPaths\n- if p4PathStartsWith(path, p)]\n+ found = self.isPathWanted(path)\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 4729f470b2..62a3929d8e 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -410,11 +410,11 @@ test_expect_failure 'git p4 clone file subset branch' '\n \t\ttest_path_is_missing file3\n \t)\n '\n \n # Check that excluded files are omitted during import\n-test_expect_failure 'git p4 clone complex branches with excluded files' '\n+test_expect_success 'git p4 clone complex branches with excluded files' '\n \ttest_when_finished cleanup_git &&\n \ttest_create_repo \"$git\" &&\n \t(\n \t\tcd \"$git\" &&\n \t\tgit config git-p4.branchList branch1:branch2 &&\n-- \n2.19.2\n\n"},{"id":"372271","messageId":"cover.1553283214.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[RFC PATCH 0/2] git-p4: \"alien\" branches and load changelist info from file","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-22T19:54:12Z","receivedAt":"2019-03-22T19:54:17Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"\nThis patch series introduces two experimental features to git-p4,\nwhich unrelated to each other.\n 1. The first patch adds support for so-called \"alien\" branches.\n The feature lets git-p4 create empty commits\n to make the history or tags more accurate.\n It is particularly useful when splitting a large Perforce depot\n into multiple git repositories.\n 2. The second patch adds support for loading changelist information from a file.\n (`p4 -G describe` equivalent)\n The original use case is to be able to migrate a Perforce depot,\n which database got a little bit corrupted, into git.\n\nIt would be nice to get some feedback to see\nif these features are usable in general and are worth mainlining at all.\n\nThe patches don't contain documentation or test changes yet,\nbecause I wanted to get feedback first\nif there is interest in mainlining these features in the first place.\n\nThis patch series should be applied on top of\n\"[PATCH v2 0/7] git-p4: a few assorted fixes for branches, excludes\" [1]\n\n[1] https://public-inbox.org/git/cover.1551485349.git.amazo@checkvideo.com/t/#m965fb5895d25d6b42638dd8efbb96e9fa9182978\n\nAndrey Mazo (2):\n git-p4: introduce alien branch mappings\n git-p4: support loading changelist descriptions from files\n\n git-p4.py | 84 ++++++++++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 71 insertions(+), 13 deletions(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\nprerequisite-patch-id: 23e039fec7a1f5c51c98326a14d788adb1ecb5ba\nprerequisite-patch-id: 030a0acdce715ff99916fd412832e5a9471225c3\nprerequisite-patch-id: 10661f77392f4131d2375976c77a7cd231fdf9ab\nprerequisite-patch-id: a55360c904eba1b9e9c934405d3141eb96c5ad30\nprerequisite-patch-id: 46357586199c02d956d53d782a12f1ee0c991302\nprerequisite-patch-id: c683e7d6017580df9385a1544af409ca615d770c\nprerequisite-patch-id: 411dcb5e95aff036e0cb3e850ea75f2424b260a6\n-- \n2.19.2\n\n"},{"id":"372272","messageId":"b02df749b9266ac8c73707617a171122156621ab.1553283214.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553283214.git.amazo@checkvideo.com","subject":"[RFC PATCH 1/2] git-p4: introduce alien branch mappings","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-22T19:54:29Z","receivedAt":"2019-03-22T19:54:36Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Labels in Perforce are not global, but can be placed on a particular view/subdirectory.\nThis might pose difficulties when importing only parts of Perforce depot into a git repository.\nFor example:\n 1. Depot layout is as follows:\n //depot/metaproject/branch1/subprojectA/...\n //depot/metaproject/branch1/subprojectB/...\n //depot/metaproject/branch2/subprojectA/...\n //depot/metaproject/branch2/subprojectB/...\n 2. Labels are placed as follows:\n * label 1A on //depot/metaproject/branch1/subprojectA/...\n * label 1B on //depot/metaproject/branch1/subprojectB/...\n * label 2A on //depot/metaproject/branch2/subprojectA/...\n * label 2B on //depot/metaproject/branch2/subprojectB/...\n 3. The goal is to import\n subprojectA into subprojectA.git and\n subprojectB into subprojectB.git\n preserving all the branches and labels.\n 4. Importing subprojectA.\n Label 1A is imported fine because it's placed on certain commit on branch1.\n However, label 1B is not imported because it's placed on a commit in another subproject:\n git-p4 says: \"importing label 1B: could not find git commit for changelist ...\"\n The same is with label 2A, which is imported; and 2B, which is not.\n\nCurrently, there is no easy way (that I'm aware of) to tell git-p4 to\nimport an empty commit into a desired branch,\nso that a label placed on that changelist could be imported as well,\nIt might be possible to get a similar effect by importing both subprojectA and B in a single git repo,\nand then running `git filter-branch --subdirectory-filter subprojectA`,\nbut this might produce way more irrelevant empty commits, than needed for labels.\n(although imported changelists can be limited with git-p4 --changesfile option)\n\nSo, introduce a concept of an \"alien\" branch.\n\nIn the above example of importing subprojectA,\n * branch1/subprojectB is an alien branch for branch1/subprojectA;\n * branch2/subprojectB is an alien branch for branch2/subprojectA.\nAny changelist for branch1/subprojectB will be imported into subprojectA.git branch1\nas an empty commit for the sole purpose of being labeled with a tag later\nor just to preserve the history of changes across the branches.\n\nThis relation between branches is specified in a similar way to branchList:\n`git config --add git-p4.alienLabelBranchMap alien_branch:real_branch`\n\nFor the example of importing subprojectA above, config parameters are\n```\ngit config --add git-p4.branchList branch1/subprojectA:branch2/subprojectA\ngit config --add git-p4.alienLabelBranchMap branch1/subprojectB:branch1/subprojectA\ngit config --add git-p4.alienLabelBranchMap branch2/subprojectB:branch2/subprojectA\n```\n\nA similar use case, is when a label is placed on a changelist for an excluded path.\n 1. Depot layout is as follows:\n //depot/branch1/...\n //depot/branch1/exclude_me/...\n 2. Labels are placed as follows:\n * label 1 on //depot/branch1/...\n * label 1E on //depot/branch1/exclude_me/...\n 3. The goal is to import\n //depot/... into depot.git excluding files under\n //depot/branch1/exclude_me/...\n and preserving all the branches and labels.\n 4. Importing subprojectA.\n Label 1 is imported fine because it's placed on certain commit on branch1.\n However, label 1E is not imported because it's placed on a commit which is excluded.\n\nFor this use case, the config would be\n```\ngit config --add git-p4.alienLabelBranchMap branch1/exclude_me:branch1\n```\n\nNote that the current implementation doesn't process alien branches\nwhen a clientspec is used.\n\nDiff best viewed with --ignore-all-space .\n\nSigned-off-by: Andrey Mazo \n---\n\nNotes:\n Documentation changes and tests are obviously missing,\n but I hoped to get some feedback on the idea overall\n before working on those.\n \n A better name for \"alien\" branches is very welcome.\n\n git-p4.py | 59 ++++++++++++++++++++++++++++++++++++++++++++-----------\n 1 file changed, 47 insertions(+), 12 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 148ea6f1b0..40bc84573b 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2610,10 +2610,12 @@ def __init__(self):\n \n # map from branch depot path to parent branch\n self.knownBranches = {}\n self.initialParents = {}\n \n+ self.knownAlienLabelBranches = {}\n+\n self.tz = \"%+03d%02d\" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))\n self.labels = {}\n \n # Force a checkpoint in fast-import and wait for it to finish\n def checkpoint(self):\n@@ -2705,14 +2707,37 @@ def splitFilesIntoBranches(self, commit):\n if self.clientSpecDirs:\n files = self.extractFilesFromCommit(commit)\n self.clientSpecDirs.update_client_spec_path_cache(files)\n \n branches = {}\n+ alienBranches = {}\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n found = self.isPathWanted(path)\n+\n+ # start with the full relative path where this file would\n+ # go in a p4 client\n+ if self.useClientSpec:\n+ # with a clientspec, we won't know where the file goes if it's excluded\n+ if found:\n+ relPath = self.clientSpecDirs.map_in_client(path)\n+ else:\n+ relPath = None\n+ else:\n+ # without a clientspec, we can guess a file path even if it's excluded\n+ relPath = self.stripRepoPath(path, self.depotPaths)\n+\n+ # Process alien branches before excludes (i.e. before respecting `found`) --\n+ # picking (empty) commits from excluded paths is one of the use-cases for alien branches.\n+ # We don't commit any files for alien branches, so don't violate excludes.\n+ for (alienPath, ourPath) in self.knownAlienLabelBranches.items():\n+ if relPath is not None and p4PathStartsWith(relPath, alienPath + '/') and ourPath in self.knownBranches:\n+ # we don't put any files since they are under the paths, we're not interested in.\n+ # however, we still want the commit message and etc.\n+ alienBranches[ourPath] = [{\"alienPath\": alienPath}]\n+\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\n@@ -2720,26 +2745,23 @@ def splitFilesIntoBranches(self, commit):\n file[\"rev\"] = commit[\"rev%s\" % fnum]\n file[\"action\"] = commit[\"action%s\" % fnum]\n file[\"type\"] = commit[\"type%s\" % fnum]\n fnum = fnum + 1\n \n- # start with the full relative path where this file would\n- # go in a p4 client\n- if self.useClientSpec:\n- relPath = self.clientSpecDirs.map_in_client(path)\n- else:\n- relPath = self.stripRepoPath(path, self.depotPaths)\n-\n for branch in self.knownBranches.keys():\n # add a trailing slash so that a commit into qt/4.2foo\n # doesn't end up in qt/4.2, e.g.\n if p4PathStartsWith(relPath, branch + \"/\"):\n if branch not in branches:\n branches[branch] = []\n branches[branch].append(file)\n break\n \n+ # not on any branch, check out our alien mapping\n+ if not branches:\n+ branches = alienBranches\n+\n return branches\n \n def writeToGitStream(self, gitMode, relPath, contents):\n self.gitStream.write('M %s inline %s\\n' % (gitMode, relPath))\n self.gitStream.write('data %d\\n' % sum(len(d) for d in contents))\n@@ -3283,10 +3305,17 @@ def getBranchMappingFromGitBranches(self):\n branch = \"main\"\n else:\n branch = branch[len(self.projectName):]\n self.knownBranches[branch] = branch\n \n+ def getAlienLabelBranchMapping(self):\n+ alienLabelBranches = gitConfigList(\"git-p4.alienLabelBranchMap\")\n+ for mapping in alienLabelBranches:\n+ if mapping:\n+ (alien, ours) = mapping.split(\":\")\n+ self.knownAlienLabelBranches[alien] = ours\n+\n def updateOptionDict(self, d):\n option_keys = {}\n if self.keepRepoPath:\n option_keys['keepRepoPath'] = 1\n \n@@ -3396,17 +3425,22 @@ def importChanges(self, changes, origin_revision=0):\n \n try:\n if self.detectBranches:\n branches = self.splitFilesIntoBranches(description)\n for branch in branches.keys():\n- ## HACK --hwn\n- branchPrefix = self.depotPaths[0] + branch + \"/\"\n- self.branchPrefixes = [ branchPrefix ]\n+ # hack branch prefix and file list for alien branches\n+ if branches[branch] and \"alienPath\" in branches[branch][0]:\n+ branchPrefix = self.depotPaths[0] + branches[branch][0][\"alienPath\"] + \"/\"\n+ filesForCommit = []\n+ else:\n+ ## HACK --hwn\n+ branchPrefix = self.depotPaths[0] + branch + \"/\"\n+ filesForCommit = branches[branch]\n \n- parent = \"\"\n+ self.branchPrefixes = [branchPrefix]\n \n- filesForCommit = branches[branch]\n+ parent = \"\"\n \n if self.verbose:\n print(\"branch is %s\" % branch)\n \n self.updatedBranches.add(branch)\n@@ -3715,10 +3749,11 @@ def run(self, args):\n \n if self.hasOrigin:\n self.getBranchMappingFromGitBranches()\n else:\n self.getBranchMapping()\n+ self.getAlienLabelBranchMapping()\n if self.verbose:\n print(\"p4-git branches: %s\" % self.p4BranchesInGit)\n print(\"initial parents: %s\" % self.initialParents)\n for b in self.p4BranchesInGit:\n if b != \"master\":\n-- \n2.19.2\n\n"},{"id":"372273","messageId":"bb3e14a3897c98762b0e656d583eaa408a6aba60.1553283214.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553283214.git.amazo@checkvideo.com","subject":"[RFC PATCH 2/2] git-p4: support loading changelist descriptions from files","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-22T19:54:45Z","receivedAt":"2019-03-22T19:54:52Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Our Perforce server experienced some kind of database corruption a few years ago.\nWhile the file data and revision history are mostly intact,\nsome metadata for several changesets got lost.\nFor example, inspecting certain changelists produces errors.\n\"\"\"\n$ p4 describe -s 12345\nDate 2019/02/26 16:46:17:\nOperation: user-describe\nOperation 'user-describe' failed.\nChange 12345 description missing!\n\"\"\"\n\nWhile some metadata (like changeset descriptions) is obviously lost,\nmost of it can be reconstructed via other commands:\n * `p4 changes -l -t //...@12345,12345` --\n to obtain date+time, author, beginning of changeset description;\n * `p4 files -a //...@12345,12345` --\n to obtain file revisions, file types, file actions;\n * `p4 diff2 -u //...@12344 //...@12345` --\n to get a unified diff of text files in a changeset;\n * `p4 print -o binary.blob@12345 //depot/binary.blob@12345` --\n to get a revision of a binary file.\n\nIt might be possible to teach git-p4 to fallback to other methods if `p4 describe` fails,\nbut it's probably too special-cased (really depends on kind and scale of DB corruption),\nso some manual intervention is perhaps acceptable.\n\nSo, with some manual work, it's possible to reconstruct `p4 -G describe ...` output manually.\nIn our case, once git-p4 passes `p4 describe` stage,\nit can proceed further just fine.\nThus, it's tempting to feed resurrected metadata to git-p4 when a normal `p4 describe` would fail.\n\nThis functionality may be useful to cache changelist information,\nor to make some changes to changelist info before feeding it to git-p4.\n\nA new config parameter is introduced to tell git-p4\nto load certain changelist descriptions from files instead of from a server.\nFor simplicity, it's one pickled file per changelist.\n```\ngit config --add git-p4.damagedChangelists 12345.pickled\ngit config --add git-p4.damagedChangelists 12346.pickled\n```\n\nThe following trivial script may be used to produce pickled `p4 -G describe`-compatible output.\n\"\"\"\n #!/usr/bin/python2\n\n import pickle\n import time\n\n # recovered commits of interest\n changes = [\n {\n 'change': '12345',\n 'status': 'submitted',\n 'code': 'stat',\n 'user': 'username1',\n 'time': str(int(time.mktime(time.strptime('2019/02/28 16:00:30', '%Y/%m/%d %H:%M:%S')))),\n 'client': 'username1_hostname1',\n 'desc': 'A bug is fixed.\\nDetails are below:\\n',\n 'depotFile0': '//depot/branch1/foo.sh',\n 'action0': 'edit',\n 'rev0': '28',\n 'type0': 'xtext',\n 'depotFile1': '//depot/branch1/bar.py',\n 'action1': 'edit',\n 'rev1': '43',\n 'type1': 'text',\n 'depotFile2': '//depot/branch1/baz.doc',\n 'action2': 'edit',\n 'rev2': '8',\n 'type2': 'binary',\n 'depotFile3': '//depot/branch1/qqq.c',\n 'action3': 'edit',\n 'rev3': '6',\n 'type3': 'ktext',\n },\n ]\n\n for change in changes:\n pickle.dump(change, open('{0}.pickled'.format(change['change']), 'wb'))\n\"\"\"\n\nSigned-off-by: Andrey Mazo \n---\n\nNotes:\n Documentation changes and tests are obviously missing,\n but I hoped to get some feedback on the idea overall\n before working on those.\n\n git-p4.py | 25 ++++++++++++++++++++++++-\n 1 file changed, 24 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 40bc84573b..3133419280 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -24,10 +24,11 @@\n import stat\n import zipfile\n import zlib\n import ctypes\n import errno\n+import pickle\n \n # support basestring in python3\n try:\n unicode = unicode\n except NameError:\n@@ -2615,10 +2616,12 @@ def __init__(self):\n self.knownAlienLabelBranches = {}\n \n self.tz = \"%+03d%02d\" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))\n self.labels = {}\n \n+ self.damagedChangelists = {}\n+\n # Force a checkpoint in fast-import and wait for it to finish\n def checkpoint(self):\n self.gitStream.write(\"checkpoint\\n\\n\")\n self.gitStream.write(\"progress checkpoint\\n\\n\")\n out = self.gitOutput.readline()\n@@ -3312,10 +3315,25 @@ def getAlienLabelBranchMapping(self):\n for mapping in alienLabelBranches:\n if mapping:\n (alien, ours) = mapping.split(\":\")\n self.knownAlienLabelBranches[alien] = ours\n \n+ def loadDamagedChangelists(self):\n+ damagedChangelists = gitConfigList(\"git-p4.damagedChangelists\")\n+ for clPickled in damagedChangelists:\n+ if not clPickled:\n+ continue\n+\n+ try:\n+ clDesc = pickle.load(open(clPickled, 'rb'))\n+ if not (\"status\" in clDesc and \"user\" in clDesc and \"time\" in clDesc and \"change\" in clDesc):\n+ die(\"Changelist description read from {0} doesn't have required fields\".format(clPickled))\n+ except (IOError, TypeError) as e:\n+ die(\"Can't read changelist description dict from {0}: {1}\".format(clPickled, str(e)))\n+\n+ self.damagedChangelists[int(clDesc[\"change\"])] = clDesc\n+\n def updateOptionDict(self, d):\n option_keys = {}\n if self.keepRepoPath:\n option_keys['keepRepoPath'] = 1\n \n@@ -3413,11 +3431,14 @@ def searchParent(self, parent, branch, target):\n return None\n \n def importChanges(self, changes, origin_revision=0):\n cnt = 1\n for change in changes:\n- description = p4_describe(change)\n+ if change in self.damagedChangelists:\n+ description = self.damagedChangelists[change]\n+ else:\n+ description = p4_describe(change)\n self.updateOptionDict(description)\n \n if not self.silent:\n sys.stdout.write(\"\\rImporting revision %s (%s%%)\" % (change, cnt * 100 / len(changes)))\n sys.stdout.flush()\n@@ -3704,10 +3725,12 @@ def run(self, args):\n bad_changesfile = True\n break\n if bad_changesfile:\n die(\"Option --changesfile is incompatible with revision specifiers\")\n \n+ self.loadDamagedChangelists()\n+\n newPaths = []\n for p in self.depotPaths:\n if p.find(\"@\") != -1:\n atIdx = p.index(\"@\")\n self.changeRange = p[atIdx:]\n-- \n2.19.2\n\n"},{"id":"372300","messageId":"CAE5ih793TVn0NJ54CJTmOZ0Gr2Y4GSYwP-DAyRpjsgJsGc-NrA@mail.gmail.com","threadId":"50645","inReplyTo":"bb3e14a3897c98762b0e656d583eaa408a6aba60.1553283214.git.amazo@checkvideo.com","subject":"Re: [RFC PATCH 2/2] git-p4: support loading changelist descriptions from files","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-03-23T08:44:44Z","receivedAt":"2019-03-23T08:45:01Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Fri, 22 Mar 2019 at 19:54, Mazo, Andrey wrote:\n>\n> Our Perforce server experienced some kind of database corruption a few years ago.\n> While the file data and revision history are mostly intact,\n> some metadata for several changesets got lost.\n\nI think it's not unheard of for P4 databases to end up being corrupt,\nas in your case.\n\nIt looks like the RCS files get updated, but the database files (i.e.\nthe metadata) do not, after which you have a bit of a problem.\n\nSo I guess this change could be quite useful, but it really needs some\ndocumentation and tests to support it - git-p4 is already complicated\nenough!\n\nYour example script should probably use the same magic that the git-p4\nscript uses to pick the path to Python.\n\nAnd perhaps come up with a nicer name than \"damaged\" - as you say, it\ncould also be used for other purposes.\n\n> For example, inspecting certain changelists produces errors.\n> \"\"\"\n> $ p4 describe -s 12345\n> Date 2019/02/26 16:46:17:\n> Operation: user-describe\n> Operation 'user-describe' failed.\n> Change 12345 description missing!\n> \"\"\"\n>\n> While some metadata (like changeset descriptions) is obviously lost,\n> most of it can be reconstructed via other commands:\n> * `p4 changes -l -t //...@12345,12345` --\n> to obtain date+time, author, beginning of changeset description;\n> * `p4 files -a //...@12345,12345` --\n> to obtain file revisions, file types, file actions;\n> * `p4 diff2 -u //...@12344 //...@12345` --\n> to get a unified diff of text files in a changeset;\n> * `p4 print -o binary.blob@12345 //depot/binary.blob@12345` --\n> to get a revision of a binary file.\n>\n> It might be possible to teach git-p4 to fallback to other methods if `p4 describe` fails,\n> but it's probably too special-cased (really depends on kind and scale of DB corruption),\n> so some manual intervention is perhaps acceptable.\n>\n> So, with some manual work, it's possible to reconstruct `p4 -G describe ...` output manually.\n> In our case, once git-p4 passes `p4 describe` stage,\n> it can proceed further just fine.\n> Thus, it's tempting to feed resurrected metadata to git-p4 when a normal `p4 describe` would fail.\n>\n> This functionality may be useful to cache changelist information,\n> or to make some changes to changelist info before feeding it to git-p4.\n>\n> A new config parameter is introduced to tell git-p4\n> to load certain changelist descriptions from files instead of from a server.\n> For simplicity, it's one pickled file per changelist.\n> ```\n> git config --add git-p4.damagedChangelists 12345.pickled\n> git config --add git-p4.damagedChangelists 12346.pickled\n> ```\n>\n> The following trivial script may be used to produce pickled `p4 -G describe`-compatible output.\n> \"\"\"\n> #!/usr/bin/python2\n>\n> import pickle\n> import time\n>\n> # recovered commits of interest\n> changes = [\n> {\n> 'change': '12345',\n> 'status': 'submitted',\n> 'code': 'stat',\n> 'user': 'username1',\n> 'time': str(int(time.mktime(time.strptime('2019/02/28 16:00:30', '%Y/%m/%d %H:%M:%S')))),\n> 'client': 'username1_hostname1',\n> 'desc': 'A bug is fixed.\\nDetails are below:\\n',\n> 'depotFile0': '//depot/branch1/foo.sh',\n> 'action0': 'edit',\n> 'rev0': '28',\n> 'type0': 'xtext',\n> 'depotFile1': '//depot/branch1/bar.py',\n> 'action1': 'edit',\n> 'rev1': '43',\n> 'type1': 'text',\n> 'depotFile2': '//depot/branch1/baz.doc',\n> 'action2': 'edit',\n> 'rev2': '8',\n> 'type2': 'binary',\n> 'depotFile3': '//depot/branch1/qqq.c',\n> 'action3': 'edit',\n> 'rev3': '6',\n> 'type3': 'ktext',\n> },\n> ]\n>\n> for change in changes:\n> pickle.dump(change, open('{0}.pickled'.format(change['change']), 'wb'))\n> \"\"\"\n>\n> Signed-off-by: Andrey Mazo \n> ---\n>\n> Notes:\n> Documentation changes and tests are obviously missing,\n> but I hoped to get some feedback on the idea overall\n> before working on those.\n>\n> git-p4.py | 25 ++++++++++++++++++++++++-\n> 1 file changed, 24 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 40bc84573b..3133419280 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -24,10 +24,11 @@\n> import stat\n> import zipfile\n> import zlib\n> import ctypes\n> import errno\n> +import pickle\n>\n> # support basestring in python3\n> try:\n> unicode = unicode\n> except NameError:\n> @@ -2615,10 +2616,12 @@ def __init__(self):\n> self.knownAlienLabelBranches = {}\n>\n> self.tz = \"%+03d%02d\" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))\n> self.labels = {}\n>\n> + self.damagedChangelists = {}\n> +\n> # Force a checkpoint in fast-import and wait for it to finish\n> def checkpoint(self):\n> self.gitStream.write(\"checkpoint\\n\\n\")\n> self.gitStream.write(\"progress checkpoint\\n\\n\")\n> out = self.gitOutput.readline()\n> @@ -3312,10 +3315,25 @@ def getAlienLabelBranchMapping(self):\n> for mapping in alienLabelBranches:\n> if mapping:\n> (alien, ours) = mapping.split(\":\")\n> self.knownAlienLabelBranches[alien] = ours\n>\n> + def loadDamagedChangelists(self):\n> + damagedChangelists = gitConfigList(\"git-p4.damagedChangelists\")\n> + for clPickled in damagedChangelists:\n> + if not clPickled:\n> + continue\n> +\n> + try:\n> + clDesc = pickle.load(open(clPickled, 'rb'))\n> + if not (\"status\" in clDesc and \"user\" in clDesc and \"time\" in clDesc and \"change\" in clDesc):\n> + die(\"Changelist description read from {0} doesn't have required fields\".format(clPickled))\n> + except (IOError, TypeError) as e:\n> + die(\"Can't read changelist description dict from {0}: {1}\".format(clPickled, str(e)))\n> +\n> + self.damagedChangelists[int(clDesc[\"change\"])] = clDesc\n> +\n> def updateOptionDict(self, d):\n> option_keys = {}\n> if self.keepRepoPath:\n> option_keys['keepRepoPath'] = 1\n>\n> @@ -3413,11 +3431,14 @@ def searchParent(self, parent, branch, target):\n> return None\n>\n> def importChanges(self, changes, origin_revision=0):\n> cnt = 1\n> for change in changes:\n> - description = p4_describe(change)\n> + if change in self.damagedChangelists:\n> + description = self.damagedChangelists[change]\n> + else:\n> + description = p4_describe(change)\n> self.updateOptionDict(description)\n>\n> if not self.silent:\n> sys.stdout.write(\"\\rImporting revision %s (%s%%)\" % (change, cnt * 100 / len(changes)))\n> sys.stdout.flush()\n> @@ -3704,10 +3725,12 @@ def run(self, args):\n> bad_changesfile = True\n> break\n> if bad_changesfile:\n> die(\"Option --changesfile is incompatible with revision specifiers\")\n>\n> + self.loadDamagedChangelists()\n> +\n> newPaths = []\n> for p in self.depotPaths:\n> if p.find(\"@\") != -1:\n> atIdx = p.index(\"@\")\n> self.changeRange = p[atIdx:]\n> --\n> 2.19.2\n>\n"},{"id":"372302","messageId":"CAE5ih797T4vtuFsDhXuNGX+A89ZQ26GOae9Dt4PVaCwJ8C_GVg@mail.gmail.com","threadId":"50645","inReplyTo":"b02df749b9266ac8c73707617a171122156621ab.1553283214.git.amazo@checkvideo.com","subject":"Re: [RFC PATCH 1/2] git-p4: introduce alien branch mappings","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-03-23T09:08:59Z","receivedAt":"2019-03-23T09:09:14Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Fri, 22 Mar 2019 at 19:54, Mazo, Andrey wrote:\n>\n> Labels in Perforce are not global, but can be placed on a particular view/subdirectory.\n> This might pose difficulties when importing only parts of Perforce depot into a git repository.\n> For example:\n> 1. Depot layout is as follows:\n> //depot/metaproject/branch1/subprojectA/...\n> //depot/metaproject/branch1/subprojectB/...\n> //depot/metaproject/branch2/subprojectA/...\n> //depot/metaproject/branch2/subprojectB/...\n> 2. Labels are placed as follows:\n> * label 1A on //depot/metaproject/branch1/subprojectA/...\n> * label 1B on //depot/metaproject/branch1/subprojectB/...\n> * label 2A on //depot/metaproject/branch2/subprojectA/...\n> * label 2B on //depot/metaproject/branch2/subprojectB/...\n> 3. The goal is to import\n> subprojectA into subprojectA.git and\n> subprojectB into subprojectB.git\n> preserving all the branches and labels.\n> 4. Importing subprojectA.\n> Label 1A is imported fine because it's placed on certain commit on branch1.\n> However, label 1B is not imported because it's placed on a commit in another subproject:\n> git-p4 says: \"importing label 1B: could not find git commit for changelist ...\"\n> The same is with label 2A, which is imported; and 2B, which is not.\n>\n> Currently, there is no easy way (that I'm aware of) to tell git-p4 to\n> import an empty commit into a desired branch,\n> so that a label placed on that changelist could be imported as well,\n\nSo there is a file in subprojectA/foo.c@41.\nAnd label 1B is against //depot/metaproject/branch1/subprojectB/bar.c@42.\n\nAnd I suppose in Perforce you could still checkout subprojectA at\nchange 42 and you would get change 41.\n\nBut with the way git-p4 works, the label just gets discarded.\n\nYou want to be able to checkout the subjectA with a tag called 1B and\nget the file contents as of 42.\n\nI wonder if it would be easier to teach the code in importP4Labels to\ngo searching harder for the next lower changelist number?\n\nWhere it currently says \"could not find git commit\"... could it do\nsomething like \"p4 changes -m1 //depot/path/...@LABEL\" and use that\ninstead?\n\nI'm not sure if that would work but it would mean you wouldn't need\nany extra configuration to maintain.\n\nBut perhaps I have misunderstood what you're trying to do here!\nPerhaps a failing test case might help explain it better?\n\nThanks\nLuke\n\n\n\n> It might be possible to get a similar effect by importing both subprojectA and B in a single git repo,\n> and then running `git filter-branch --subdirectory-filter subprojectA`,\n> but this might produce way more irrelevant empty commits, than needed for labels.\n> (although imported changelists can be limited with git-p4 --changesfile option)\n>\n> So, introduce a concept of an \"alien\" branch.\n>\n> In the above example of importing subprojectA,\n> * branch1/subprojectB is an alien branch for branch1/subprojectA;\n> * branch2/subprojectB is an alien branch for branch2/subprojectA.\n> Any changelist for branch1/subprojectB will be imported into subprojectA.git branch1\n> as an empty commit for the sole purpose of being labeled with a tag later\n> or just to preserve the history of changes across the branches.\n>\n> This relation between branches is specified in a similar way to branchList:\n> `git config --add git-p4.alienLabelBranchMap alien_branch:real_branch`\n>\n> For the example of importing subprojectA above, config parameters are\n> ```\n> git config --add git-p4.branchList branch1/subprojectA:branch2/subprojectA\n> git config --add git-p4.alienLabelBranchMap branch1/subprojectB:branch1/subprojectA\n> git config --add git-p4.alienLabelBranchMap branch2/subprojectB:branch2/subprojectA\n> ```\n>\n> A similar use case, is when a label is placed on a changelist for an excluded path.\n> 1. Depot layout is as follows:\n> //depot/branch1/...\n> //depot/branch1/exclude_me/...\n> 2. Labels are placed as follows:\n> * label 1 on //depot/branch1/...\n> * label 1E on //depot/branch1/exclude_me/...\n> 3. The goal is to import\n> //depot/... into depot.git excluding files under\n> //depot/branch1/exclude_me/...\n> and preserving all the branches and labels.\n> 4. Importing subprojectA.\n> Label 1 is imported fine because it's placed on certain commit on branch1.\n> However, label 1E is not imported because it's placed on a commit which is excluded.\n>\n> For this use case, the config would be\n> ```\n> git config --add git-p4.alienLabelBranchMap branch1/exclude_me:branch1\n> ```\n>\n> Note that the current implementation doesn't process alien branches\n> when a clientspec is used.\n>\n> Diff best viewed with --ignore-all-space .\n>\n> Signed-off-by: Andrey Mazo \n> ---\n>\n> Notes:\n> Documentation changes and tests are obviously missing,\n> but I hoped to get some feedback on the idea overall\n> before working on those.\n>\n> A better name for \"alien\" branches is very welcome.\n>\n> git-p4.py | 59 ++++++++++++++++++++++++++++++++++++++++++++-----------\n> 1 file changed, 47 insertions(+), 12 deletions(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 148ea6f1b0..40bc84573b 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -2610,10 +2610,12 @@ def __init__(self):\n>\n> # map from branch depot path to parent branch\n> self.knownBranches = {}\n> self.initialParents = {}\n>\n> + self.knownAlienLabelBranches = {}\n> +\n> self.tz = \"%+03d%02d\" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))\n> self.labels = {}\n>\n> # Force a checkpoint in fast-import and wait for it to finish\n> def checkpoint(self):\n> @@ -2705,14 +2707,37 @@ def splitFilesIntoBranches(self, commit):\n> if self.clientSpecDirs:\n> files = self.extractFilesFromCommit(commit)\n> self.clientSpecDirs.update_client_spec_path_cache(files)\n>\n> branches = {}\n> + alienBranches = {}\n> fnum = 0\n> while \"depotFile%s\" % fnum in commit:\n> path = commit[\"depotFile%s\" % fnum]\n> found = self.isPathWanted(path)\n> +\n> + # start with the full relative path where this file would\n> + # go in a p4 client\n> + if self.useClientSpec:\n> + # with a clientspec, we won't know where the file goes if it's excluded\n> + if found:\n> + relPath = self.clientSpecDirs.map_in_client(path)\n> + else:\n> + relPath = None\n> + else:\n> + # without a clientspec, we can guess a file path even if it's excluded\n> + relPath = self.stripRepoPath(path, self.depotPaths)\n> +\n> + # Process alien branches before excludes (i.e. before respecting `found`) --\n> + # picking (empty) commits from excluded paths is one of the use-cases for alien branches.\n> + # We don't commit any files for alien branches, so don't violate excludes.\n> + for (alienPath, ourPath) in self.knownAlienLabelBranches.items():\n> + if relPath is not None and p4PathStartsWith(relPath, alienPath + '/') and ourPath in self.knownBranches:\n> + # we don't put any files since they are under the paths, we're not interested in.\n> + # however, we still want the commit message and etc.\n> + alienBranches[ourPath] = [{\"alienPath\": alienPath}]\n> +\n> if not found:\n> fnum = fnum + 1\n> continue\n>\n> file = {}\n> @@ -2720,26 +2745,23 @@ def splitFilesIntoBranches(self, commit):\n> file[\"rev\"] = commit[\"rev%s\" % fnum]\n> file[\"action\"] = commit[\"action%s\" % fnum]\n> file[\"type\"] = commit[\"type%s\" % fnum]\n> fnum = fnum + 1\n>\n> - # start with the full relative path where this file would\n> - # go in a p4 client\n> - if self.useClientSpec:\n> - relPath = self.clientSpecDirs.map_in_client(path)\n> - else:\n> - relPath = self.stripRepoPath(path, self.depotPaths)\n> -\n> for branch in self.knownBranches.keys():\n> # add a trailing slash so that a commit into qt/4.2foo\n> # doesn't end up in qt/4.2, e.g.\n> if p4PathStartsWith(relPath, branch + \"/\"):\n> if branch not in branches:\n> branches[branch] = []\n> branches[branch].append(file)\n> break\n>\n> + # not on any branch, check out our alien mapping\n> + if not branches:\n> + branches = alienBranches\n> +\n> return branches\n>\n> def writeToGitStream(self, gitMode, relPath, contents):\n> self.gitStream.write('M %s inline %s\\n' % (gitMode, relPath))\n> self.gitStream.write('data %d\\n' % sum(len(d) for d in contents))\n> @@ -3283,10 +3305,17 @@ def getBranchMappingFromGitBranches(self):\n> branch = \"main\"\n> else:\n> branch = branch[len(self.projectName):]\n> self.knownBranches[branch] = branch\n>\n> + def getAlienLabelBranchMapping(self):\n> + alienLabelBranches = gitConfigList(\"git-p4.alienLabelBranchMap\")\n> + for mapping in alienLabelBranches:\n> + if mapping:\n> + (alien, ours) = mapping.split(\":\")\n> + self.knownAlienLabelBranches[alien] = ours\n> +\n> def updateOptionDict(self, d):\n> option_keys = {}\n> if self.keepRepoPath:\n> option_keys['keepRepoPath'] = 1\n>\n> @@ -3396,17 +3425,22 @@ def importChanges(self, changes, origin_revision=0):\n>\n> try:\n> if self.detectBranches:\n> branches = self.splitFilesIntoBranches(description)\n> for branch in branches.keys():\n> - ## HACK --hwn\n> - branchPrefix = self.depotPaths[0] + branch + \"/\"\n> - self.branchPrefixes = [ branchPrefix ]\n> + # hack branch prefix and file list for alien branches\n> + if branches[branch] and \"alienPath\" in branches[branch][0]:\n> + branchPrefix = self.depotPaths[0] + branches[branch][0][\"alienPath\"] + \"/\"\n> + filesForCommit = []\n> + else:\n> + ## HACK --hwn\n> + branchPrefix = self.depotPaths[0] + branch + \"/\"\n> + filesForCommit = branches[branch]\n>\n> - parent = \"\"\n> + self.branchPrefixes = [branchPrefix]\n>\n> - filesForCommit = branches[branch]\n> + parent = \"\"\n>\n> if self.verbose:\n> print(\"branch is %s\" % branch)\n>\n> self.updatedBranches.add(branch)\n> @@ -3715,10 +3749,11 @@ def run(self, args):\n>\n> if self.hasOrigin:\n> self.getBranchMappingFromGitBranches()\n> else:\n> self.getBranchMapping()\n> + self.getAlienLabelBranchMapping()\n> if self.verbose:\n> print(\"p4-git branches: %s\" % self.p4BranchesInGit)\n> print(\"initial parents: %s\" % self.initialParents)\n> for b in self.p4BranchesInGit:\n> if b != \"master\":\n> --\n> 2.19.2\n>\n"},{"id":"372303","messageId":"CAE5ih7-W9vw4siwc=YQD36863LaCm1RzatAZ4Ajjk8MjYimOdA@mail.gmail.com","threadId":"50645","inReplyTo":"e644a8ab4928349ed83ac9ab6ffdbcafc3a3a7b5.1553207234.git.amazo@checkvideo.com","subject":"Re: [PATCH v2 2/7] git-p4: match branches case insensitively if configured","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2019-03-23T09:15:48Z","receivedAt":"2019-03-23T09:16:02Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Thu, 21 Mar 2019 at 22:32, Mazo, Andrey wrote:\n>\n> git-p4 knows how to handle case insensitivity in file paths\n> if core.ignorecase is set.\n> However, when determining a branch for a file,\n> it still does a case-sensitive prefix match.\n> This may result in some file changes to be lost on import.\n>\n> For example, given the following commits\n> 1. add //depot/main/file1\n> 2. add //depot/DirA/file2\n> 3. add //depot/dira/file3\n> 4. add //depot/DirA/file4\n> and \"branchList = main:DirA\" branch mapping,\n> commit 3 will be lost.\n>\n> So, do branch search case insensitively if running with core.ignorecase set.\n> Teach splitFilesIntoBranches() to use the p4PathStartsWith() function\n> for path prefix matches instead of always case-sensitive match.\n\nI wonder what other code paths break due to this problem!\n\nLooks reasonable but I fear there may be some other holes in there -\nquickly looking through the code suggests there are several other\nplaces this problem occurs.\n\nLuke\n\n>\n> Signed-off-by: Andrey Mazo \n> ---\n> git-p4.py | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index c0a3068b6f..91c610f960 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -2721,11 +2721,11 @@ def splitFilesIntoBranches(self, commit):\n> relPath = self.stripRepoPath(path, self.depotPaths)\n>\n> for branch in self.knownBranches.keys():\n> # add a trailing slash so that a commit into qt/4.2foo\n> # doesn't end up in qt/4.2, e.g.\n> - if relPath.startswith(branch + \"/\"):\n> + if p4PathStartsWith(relPath, branch + \"/\"):\n> if branch not in branches:\n> branches[branch] = []\n> branches[branch].append(file)\n> break\n>\n> --\n> 2.19.2\n>\n"},{"id":"372442","messageId":"20190325172037.32373-1-amazo@checkvideo.com","threadId":"50645","inReplyTo":"CAE5ih7-W9vw4siwc=YQD36863LaCm1RzatAZ4Ajjk8MjYimOdA@mail.gmail.com","subject":"Re: [PATCH v2 2/7] git-p4: match branches case insensitively if configured","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-25T17:20:55Z","receivedAt":"2019-03-25T17:21:00Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"From: \"Mazo, Andrey\" \n\n\n23.03.2019, 05:16, \"Luke Diamand\" :\n> On Thu, 21 Mar 2019 at 22:32, Mazo, Andrey wrote:\n>> git-p4 knows how to handle case insensitivity in file paths\n>> if core.ignorecase is set.\n>> However, when determining a branch for a file,\n>> it still does a case-sensitive prefix match.\n>> This may result in some file changes to be lost on import.\n>>\n>> For example, given the following commits\n>> 1. add //depot/main/file1\n>> 2. add //depot/DirA/file2\n>> 3. add //depot/dira/file3\n>> 4. add //depot/DirA/file4\n>> and \"branchList = main:DirA\" branch mapping,\n>> commit 3 will be lost.\n>>\n>> So, do branch search case insensitively if running with core.ignorecase set.\n>> Teach splitFilesIntoBranches() to use the p4PathStartsWith() function\n>> for path prefix matches instead of always case-sensitive match.\n>\n> I wonder what other code paths break due to this problem!\n>\n> Looks reasonable but I fear there may be some other holes in there -\n> quickly looking through the code suggests there are several other\n> places this problem occurs.\n\nFrom a quick search for .startswith(), I only see that stripRepoPath() might have a similar problem in useclientspec case.\nIf you see other apparent problematic places, could you, please, point them out?\n\nOr let me try to come up with a test case, and see what other places break.\n\nThank you,\nAndrey.\n"},{"id":"372443","messageId":"20190325174615.1231-1-amazo@checkvideo.com","threadId":"50645","inReplyTo":"CAE5ih793TVn0NJ54CJTmOZ0Gr2Y4GSYwP-DAyRpjsgJsGc-NrA@mail.gmail.com","subject":"Re: [RFC PATCH 2/2] git-p4: support loading changelist descriptions","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-25T17:46:25Z","receivedAt":"2019-03-25T17:46:31Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"23.03.2019, 04:44, \"Luke Diamand\" :\n> On Fri, 22 Mar 2019 at 19:54, Mazo, Andrey wrote:\n>> Our Perforce server experienced some kind of database corruption a few years ago.\n>> While the file data and revision history are mostly intact,\n>> some metadata for several changesets got lost.\n>\n> I think it's not unheard of for P4 databases to end up being corrupt,\n> as in your case.\n>\n> It looks like the RCS files get updated, but the database files (i.e.\n> the metadata) do not, after which you have a bit of a problem.\n\nYeah, that's exactly what happened! :/\n\n> So I guess this change could be quite useful, but it really needs some\n> documentation and tests to support it - git-p4 is already complicated\n> enough!\n\nGreat, thanks for positive feedback.\nLet me start working on these.\nLuckily, the actual git-p4 changes are fairly straightforward.\n\n> Your example script should probably use the same magic that the git-p4\n> script uses to pick the path to Python.\nOh, you mean \"#!/usr/bin/env python\"?\nSure!\n\nActually, I also have a smarter \"example script\", which reconstructs `p4 describe` output from `p4 files` and `p4 change` without manual intervention.\nIt'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?\n\n> And perhaps come up with a nicer name than \"damaged\" - as you say, it\n> could also be used for other purposes.\nDoes anything from the list below sound reasonable?\n 1. git-p4.loadChangelistInfo\n 2. git-p4.changelistInfoOverride\n 3. git-p4.cachedChangelistInfo\n\n>> For simplicity, it's one pickled file per changelist.\n(talking to myself)\nI realized `p4 -G describe` produces a marshalled output, not pickled one.\nDon't know why I decided to use pickle instead of marshal back when I wrote it.\nI'll switch to marshal for serialization/deserialization in the next reroll.\nIt'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.\n\nThank you,\nAndrey\n"},{"id":"372541","messageId":"20190326184327.28335-1-amazo@checkvideo.com","threadId":"50645","inReplyTo":"CAE5ih797T4vtuFsDhXuNGX+A89ZQ26GOae9Dt4PVaCwJ8C_GVg@mail.gmail.com","subject":"Re: [RFC PATCH 1/2] git-p4: introduce alien branch mappings","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-26T18:43:43Z","receivedAt":"2019-03-26T18:43:48Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":">> Labels in Perforce are not global, but can be placed on a particular view/subdirectory.\n>> This might pose difficulties when importing only parts of Perforce depot into a git repository.\n>> For example:\n>> 1. Depot layout is as follows:\n>> //depot/metaproject/branch1/subprojectA/...\n>> //depot/metaproject/branch1/subprojectB/...\n>> //depot/metaproject/branch2/subprojectA/...\n>> //depot/metaproject/branch2/subprojectB/...\n>> 2. Labels are placed as follows:\n>> * label 1A on //depot/metaproject/branch1/subprojectA/...\n>> * label 1B on //depot/metaproject/branch1/subprojectB/...\n>> * label 2A on //depot/metaproject/branch2/subprojectA/...\n>> * label 2B on //depot/metaproject/branch2/subprojectB/...\n>> 3. The goal is to import\n>> subprojectA into subprojectA.git and\n>> subprojectB into subprojectB.git\n>> preserving all the branches and labels.\n>> 4. Importing subprojectA.\n>> Label 1A is imported fine because it's placed on certain commit on branch1.\n>> However, label 1B is not imported because it's placed on a commit in another subproject:\n>> git-p4 says: \"importing label 1B: could not find git commit for changelist ...\"\n>> The same is with label 2A, which is imported; and 2B, which is not.\n>>\n>> Currently, there is no easy way (that I'm aware of) to tell git-p4 to\n>> import an empty commit into a desired branch,\n>> so that a label placed on that changelist could be imported as well,\n> \n> So there is a file in subprojectA/foo.c@41.\n> And label 1B is against //depot/metaproject/branch1/subprojectB/bar.c@42.\n> \n> And I suppose in Perforce you could still checkout subprojectA at\n> change 42 and you would get change 41.\n> \n> But with the way git-p4 works, the label just gets discarded.\n\nYes, exactly.\n\n> You want to be able to checkout the subjectA with a tag called 1B and\n> get the file contents as of 42.\n> \n> I wonder if it would be easier to teach the code in importP4Labels to\n> go searching harder for the next lower changelist number?\n> \n> Where it currently says \"could not find git commit\"... could it do\n> something like \"p4 changes -m1 //depot/path/...@LABEL\" and use that\n> instead?\n> \n> I'm not sure if that would work but it would mean you wouldn't need\n> any extra configuration to maintain.\n\nYeah, that's a great idea!\nI think, it should work pretty well in simpler cases for sure.\nInitially, I was thinking, that I needed an explicit configuration option\nto choose the proper branch/subproject in a more complicated case,\nbut let me give it a try to your idea -- hopefully it just works.\n\nSome new option like git-p4.allowInexactLabels to enable this behavior?\nDon't think it should be enabled by default unless git-p4.labelImportRegexp is set, right?\n\n> But perhaps I have misunderstood what you're trying to do here!\n> Perhaps a failing test case might help explain it better?\n\nNo, I think, you got it right!\nThank you for the great suggestion!\n\nYeah, let me see if I can get a simple but representative test case.\n\n> \n> Thanks\n> Luke\n"},{"id":"372594","messageId":"f695acc99834e01f0313c0cc9cb024f960da3ab1.1553727979.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"20190326184327.28335-1-amazo@checkvideo.com","subject":"[RFC PATCH 1/1] git-p4: inexact label detection","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-03-27T23:08:11Z","receivedAt":"2019-03-27T23:09:56Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Labels in Perforce are not global, but can be placed on a particular view/subdirectory.\nThis might pose difficulties when importing only parts of Perforce depot into a git repository.\nFor example:\n 1. Depot layout is as follows:\n //depot/metaproject/branch1/subprojectA/...\n //depot/metaproject/branch1/subprojectB/...\n //depot/metaproject/branch2/subprojectA/...\n //depot/metaproject/branch2/subprojectB/...\n 2. Labels are placed as follows:\n * label 1A on //depot/metaproject/branch1/subprojectA/...\n * label 1B on //depot/metaproject/branch1/subprojectB/...\n * label 2A on //depot/metaproject/branch2/subprojectA/...\n * label 2B on //depot/metaproject/branch2/subprojectB/...\n 3. The goal is to import\n subprojectA into subprojectA.git and\n subprojectB into subprojectB.git\n preserving all the branches and labels.\n 4. Importing subprojectA.\n Label 1A is imported fine because it's placed on certain commit on branch1.\n However, label 1B is not imported because it's placed on a commit in another subproject:\n git-p4 says: \"importing label 1B: could not find git commit for changelist ...\"\n The same is with label 2A, which is imported; and 2B, which is not.\n\nCurrently, there is no easy way (that I'm aware of) to tell git-p4 to\nimport an empty commit into a desired branch,\nso that a label placed on that changelist could be imported as well,\nIt might be possible to get a similar effect by importing both subprojectA and B in a single git repo,\nand then running `git filter-branch --subdirectory-filter subprojectA`,\nbut this might produce way more irrelevant empty commits, than needed for labels.\n(although imported changelists can be limited with git-p4 --changesfile option)\nAlso, `git filter-branch` is harder to use for incremental imports\nor when changes are submitted from git back to Perforce.\n\nAs suggested by Luke,\ninstead of creating an empty commit for the sole purpose of being tagged later,\nteach git-p4 to search harder for the next lower changelist,\ncorresponding to the label in question.\n\nDo this by finding the highest changelist up to the label under all known branches,\n(branches are finalized by the time importP4Labels() runs)\nand using it instead of a depot-wide changelist corresponding to the label.\n\nThis new behavior may not be desired for people,\nwho want exact label <-> changelist relationship.\nSo, add a new boolean config parameter git-p4.allowInexactLabels (defaults to false)\nto explicitly enable it if needed.\nAlso, this behavior only appears to be useful in case of multiple branches,\n(otherwise, every Perforce changelist should appear in git)\nso it's not engaged when running without branch detection.\n\nDetect and report (--verbose) \"inexact\" tags,\ni.e. tags placed on a lower changelist than was in Perforce.\nImplement this by comparing a changelist for which a commit was found\nwith a changelist corresponding to the label on the whole depot.\n\nNote, that the new \"inexact\" logic works slower\nthan the original code in case of numerous branches,\nbecause p4 needs to calculate the most recent change for each branch path instead of just one.\n\nThis is an alternative solution to \"alien\" branches concept proposed earlier:\nhttps://public-inbox.org/git/b02df749b9266ac8c73707617a171122156621ab.1553283214.git.amazo@checkvideo.com/\n\nSigned-off-by: Andrey Mazo \nSuggested-by: Luke Diamand \n---\n git-p4.py | 45 +++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 39 insertions(+), 6 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex b04f54b3d1..838c1b43d7 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3186,17 +3186,40 @@ def importP4Labels(self, stream, p4Labels):\n if name in ignoredP4Labels:\n continue\n \n labelDetails = p4CmdList(['label', \"-o\", name])[0]\n \n- # get the most recent changelist for each file in this label\n- change = p4Cmd([\"changes\", \"-m\", \"1\"] + [\"%s...@%s\" % (p, name)\n+ if self.detectBranches and gitConfigBool(\"git-p4.allowInexactLabels\"):\n+ doInexactLabels = True\n+ else:\n+ doInexactLabels = False\n+\n+ # get the most recent changelist in this label for the whole depot\n+ depot_wide_changelist = p4Cmd([\"changes\", \"-m\", \"1\"] + [\"%s...@%s\" % (p, name)\n for p in self.depotPaths])\n+ if 'change' in depot_wide_changelist:\n+ depot_wide_changelist = int(depot_wide_changelist['change'])\n+ else:\n+ depot_wide_changelist = None\n+\n+ # get the most recent changelist for each file under branches of interest in this label\n+ if doInexactLabels:\n+ paths = [\"%s...@%s\" % (self.depotPaths[0] + p + '/', name) for p in self.knownBranches]\n+ changes = p4CmdList([\"changes\", \"-m\", \"1\"] + paths)\n+ changes = [int(c['change']) for c in changes if 'change' in c]\n+\n+ # there may be different \"most recent\" changelists for different paths.\n+ # take the newest since some paths were just modified later than others.\n+ if changes:\n+ changelist = max(changes)\n+ else:\n+ changelist = None\n+ else:\n+ changelist = depot_wide_changelist\n \n- if 'change' in change:\n+ if changelist:\n # find the corresponding git commit; take the oldest commit\n- changelist = int(change['change'])\n if changelist in self.committedChanges:\n gitCommit = \":%d\" % changelist # use a fast-import mark\n commitFound = True\n else:\n gitCommit = read_pipe([\"git\", \"rev-list\", \"--max-count=1\",\n@@ -3216,14 +3239,24 @@ def importP4Labels(self, stream, p4Labels):\n tmwhen = 1\n \n when = int(time.mktime(tmwhen))\n self.streamTag(stream, name, labelDetails, gitCommit, when)\n if verbose:\n- print(\"p4 label %s mapped to git commit %s\" % (name, gitCommit))\n+ if depot_wide_changelist == changelist:\n+ isExact = \"\"\n+ else:\n+ isExact = \" inexactly\"\n+ print(\"p4 label %s mapped%s to git commit %s\" % (name, isExact, gitCommit))\n else:\n if verbose:\n- print(\"Label %s has no changelists - possibly deleted?\" % name)\n+ if depot_wide_changelist:\n+ # there is a changelist corresponding to this label,\n+ # but it's not under any branches of interest.\n+ print(\"Label %s has no changelists under detected branches -- ignoring\" % name)\n+ else:\n+ # there is no changelist corresponding to this label in the whole depot\n+ print(\"Label %s has no changelists - possibly deleted?\" % name)\n \n if not commitFound:\n # We can't import this label; don't try again as it will get very\n # expensive repeatedly fetching all the files for labels that will\n # never be imported. If the label is moved in the future, the\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\n-- \n2.19.2\n\n"},{"id":"372923","messageId":"cover.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1553207234.git.amazo@checkvideo.com","subject":"[PATCH v3 0/8] git-p4: a few assorted fixes for branches, excludes","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:10Z","receivedAt":"2019-04-01T18:02:22Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"\nThis series fixes a few cases with branch detection\nand handling of excludes by git-p4.\n\nThis is the third iteration of the patch series.\nChanges since v2 [2]:\n * Added new test cases for case-insensitive branch detection.\nChanges since v1 [1]:\n * Added new test case for excluded paths when detecting branches;\n * Added a new fix for excluded paths when detecting branches.\n\n[1] https://public-inbox.org/git/cover.1551485349.git.amazo@checkvideo.com\n[2] https://public-inbox.org/git/cover.1553207234.git.amazo@checkvideo.com/\n\nRange-diff vs v2:\n1: 3ac39171d4 = 1: bd009a5ca5 git-p4: detect/prevent infinite loop in gitCommitByP4Change()\n2: e644a8ab49 < -: ---------- git-p4: match branches case insensitively if configured\n-: ---------- > 2: 68b68ce1e4 git-p4: add failing test for \"git-p4: match branches case insensitively if configured\"\n-: ---------- > 3: 6eaad2582c git-p4: match branches case insensitively if configured\n3: 44fed954dc = 4: 1bd5e170e0 git-p4: don't groom exclude path list on every commit\n4: a0d3fa6add = 5: b657967154 git-p4: add failing test for \"don't exclude other files with same prefix\"\n5: 3330f88a0d = 6: 035abfff2a git-p4: don't exclude other files with same prefix\n6: 6170d45951 = 7: 2bde24b7e4 git-p4: add failing test for \"git-p4: respect excluded paths when detecting branches\"\n7: 758d8e8486 = 8: 6d3ffb98a7 git-p4: respect excluded paths when detecting branches\n\nAndrey Mazo (8):\n git-p4: detect/prevent infinite loop in gitCommitByP4Change()\n git-p4: add failing test for \"git-p4: match branches case insensitively if configured\"\n git-p4: match branches case insensitively if configured\n git-p4: don't groom exclude path list on every commit\n git-p4: add failing test for \"don't exclude other files with same prefix\"\n git-p4: don't exclude other files with same prefix\n git-p4: add failing test for \"git-p4: respect excluded paths when detecting branches\"\n git-p4: respect excluded paths when detecting branches\n\n git-p4.py | 44 ++++++++-----\n t/t9801-git-p4-branch.sh | 132 ++++++++++++++++++++++++++++++++++++++\n t/t9817-git-p4-exclude.sh | 51 +++++++++++++--\n 3 files changed, 205 insertions(+), 22 deletions(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\n-- \n2.19.2\n\n"},{"id":"372924","messageId":"bd009a5ca5679883f9366a58fbfbdcff1cf5f8fa.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 1/8] git-p4: detect/prevent infinite loop in gitCommitByP4Change()","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:17Z","receivedAt":"2019-04-01T18:02:24Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Under certain circumstances, gitCommitByP4Change() can enter an infinite\nloop resulting in `git p4 sync` hanging forever.\n\nThe problem is that\n`git rev-list --bisect ^` can return ``,\nwhich would result in reinspecting and potentially an infinite loop.\n\nThis can happen when importing just a subset of P4 repository\nand/or with explicit \"--changesfile\" option.\n\nA real-life example:\n\"\"\"\n looking in ref refs/remotes/p4/mybranch for change 26894 using bisect...\n Reading pipe: git rev-parse refs/remotes/p4/mybranch\n trying: earliest latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git cat-file commit 147f5d3292af2e1cc4a56a7b96db845144c68486\n current change 25339\n trying: earliest ^147f5d3292af2e1cc4a56a7b96db845144c68486 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^147f5d3292af2e1cc4a56a7b96db845144c68486\n Reading pipe: git cat-file commit 51db83df9d588010d0bd995641c85aa0408a5bb9\n current change 25420\n trying: earliest ^51db83df9d588010d0bd995641c85aa0408a5bb9 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^51db83df9d588010d0bd995641c85aa0408a5bb9\n Reading pipe: git cat-file commit e8f83909ceb570f5a7e48c2853f3c5d8207cea52\n current change 25448\n trying: earliest ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^e8f83909ceb570f5a7e48c2853f3c5d8207cea52\n Reading pipe: git cat-file commit 09a48eb7acd594dce52e06681be9c366e1844d66\n current change 25521\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n trying: earliest ^09a48eb7acd594dce52e06681be9c366e1844d66 latest 4daff81c520a82678e1ef347f2b5e97258101ae1\n Reading pipe: git rev-list --bisect 4daff81c520a82678e1ef347f2b5e97258101ae1 ^09a48eb7acd594dce52e06681be9c366e1844d66\n Reading pipe: git cat-file commit 4daff81c520a82678e1ef347f2b5e97258101ae1\n current change 26907\n ...\n\"\"\"\n\nThe fix is two-fold:\n * detect an infinite loop and die right away\n instead of looping forever;\n * make sure, `git rev-list --bisect` can't return \"latestCommit\" again\n by excluding it from the rev-list range explicitly.\n\nSigned-off-by: Andrey Mazo \n---\n\nNotes:\n I don't have a simple test-case for this yet,\n and I was able to perform a few complex initial `git p4 sync` runs\n without hitting this problem.\n \n I suspect, I had somehow messed up with branch definitions\n and --changesfile option at some point.\n\n git-p4.py | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 5b79920f46..c0a3068b6f 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3323,11 +3323,13 @@ def gitCommitByP4Change(self, ref, change):\n return next\n \n if currentChange < change:\n earliestCommit = \"^%s\" % next\n else:\n- latestCommit = \"%s\" % next\n+ if next == latestCommit:\n+ die(\"Infinite loop while looking in ref %s for change %s. Check your branch mappings\" % (ref, change))\n+ latestCommit = \"%s^@\" % next\n \n return \"\"\n \n def importNewBranch(self, branch, maxChange):\n # make fast-import flush all changes to disk and update the refs using the checkpoint\n-- \n2.19.2\n\n"},{"id":"372925","messageId":"68b68ce1e4782bba552a016867bfc629f0d5e24f.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 2/8] git-p4: add failing test for \"git-p4: match branches case insensitively if configured\"","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:21Z","receivedAt":"2019-04-01T18:02:26Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"In preparation for a fix, add a failing test case to test that\ngit-p4 doesn't fold the case in file paths\nwhen doing branch detection case insensitively.\n(i.e. when core.ignorecase is set)\n\nSigned-off-by: Andrey Mazo \n---\n t/t9801-git-p4-branch.sh | 92 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 92 insertions(+)\n\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 6a86d6996b..c48532e12b 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie\n \t\tcd branch1 &&\n \t\tgrep \"client spec\" file1\n \t)\n '\n \n+test_expect_success 'restart p4d (case folding enabled)' '\n+\tkill_p4d &&\n+\tstart_p4d -C1\n+'\n+\n+#\n+# 1: //depot/main/mf1\n+# 2: integrate //depot/main/... -> //depot/branch1/...\n+# 3: //depot/main/mf2\n+# 4: //depot/BRANCH1/B1f3\n+# 5: //depot/branch1/b1f4\n+#\n+test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tmkdir -p main &&\n+\n+\t\techo mf1 >main/mf1 &&\n+\t\tp4 add main/mf1 &&\n+\t\tp4 submit -d \"main/mf1\" &&\n+\n+\t\tp4 integrate //depot/main/... //depot/branch1/... &&\n+\t\tp4 submit -d \"integrate main to branch1\" &&\n+\n+\t\techo mf2 >main/mf2 &&\n+\t\tp4 add main/mf2 &&\n+\t\tp4 submit -d \"main/mf2\" &&\n+\n+\t\tmkdir BRANCH1 &&\n+\t\techo B1f3 >BRANCH1/B1f3 &&\n+\t\tp4 add BRANCH1/B1f3 &&\n+\t\tp4 submit -d \"BRANCH1/B1f3\" &&\n+\n+\t\techo b1f4 >branch1/b1f4 &&\n+\t\tp4 add branch1/b1f4 &&\n+\t\tp4 submit -d \"branch1/b1f4\"\n+\t)\n+'\n+\n+# Check that files are properly split across branches when ignorecase is set\n+test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' '\n+\ttest_when_finished cleanup_git &&\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.branchList main:branch1 &&\n+\t\tgit config --type=bool core.ignoreCase true &&\n+\t\tgit p4 clone --dest=. --detect-branches //depot@all &&\n+\n+\t\tgit log --all --graph --decorate --stat &&\n+\n+\t\tgit reset --hard p4/master &&\n+\t\ttest_path_is_file mf1 &&\n+\t\ttest_path_is_file mf2 &&\n+\t\ttest_path_is_missing B1f3 &&\n+\t\ttest_path_is_missing b1f4 &&\n+\n+\t\tgit reset --hard p4/depot/branch1 &&\n+\t\ttest_path_is_file mf1 &&\n+\t\ttest_path_is_missing mf2 &&\n+\t\ttest_path_is_file B1f3 &&\n+\t\ttest_path_is_file b1f4\n+\t)\n+'\n+\n+# Check that files are properly split across branches when ignorecase is set, use-client-spec case\n+test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.branchList main:branch1 &&\n+\t\tgit config --type=bool core.ignoreCase true &&\n+\t\tgit p4 clone --dest=. --use-client-spec --detect-branches //depot@all &&\n+\n+\t\tgit log --all --graph --decorate --stat &&\n+\n+\t\tgit reset --hard p4/master &&\n+\t\ttest_path_is_file mf1 &&\n+\t\ttest_path_is_file mf2 &&\n+\t\ttest_path_is_missing B1f3 &&\n+\t\ttest_path_is_missing b1f4 &&\n+\n+\t\tgit reset --hard p4/depot/branch1 &&\n+\t\ttest_path_is_file mf1 &&\n+\t\ttest_path_is_missing mf2 &&\n+\t\ttest_path_is_file B1f3 &&\n+\t\ttest_path_is_file b1f4\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n \n test_done\n-- \n2.19.2\n\n"},{"id":"372926","messageId":"1bd5e170e00956ba131cf57b680102610a1b4aa2.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 4/8] git-p4: don't groom exclude path list on every commit","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:26Z","receivedAt":"2019-04-01T18:02:31Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Currently, `cloneExclude` array is being groomed (by removing trailing \"...\")\non every changeset.\n(since `extractFilesFromCommit()` is called on every imported changeset)\n\nAs a micro-optimization, do it once while parsing arguments.\nAlso, prepend \"/\" and remove trailing \"...\" at the same time.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex f3e5ccb7af..7edcbad055 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1314,11 +1314,11 @@ class Command:\n def __init__(self):\n self.usage = \"usage: %prog [options]\"\n self.needsGit = True\n self.verbose = False\n \n- # This is required for the \"append\" cloneExclude action\n+ # This is required for the \"append\" update_shelve action\n def ensure_value(self, attr, value):\n if not hasattr(self, attr) or getattr(self, attr) is None:\n setattr(self, attr, value)\n return getattr(self, attr)\n \n@@ -2528,10 +2528,15 @@ def map_in_client(self, depot_path):\n return self.client_spec_path_cache[depot_path]\n \n die( \"Error: %s is not found in client spec path\" % depot_path )\n return \"\"\n \n+def cloneExcludeCallback(option, opt_str, value, parser):\n+ # prepend \"/\" because the first \"/\" was consumed as part of the option itself.\n+ # (\"-//depot/A/...\" becomes \"/depot/A/...\" after option parsing)\n+ parser.values.cloneExclude += [\"/\" + re.sub(r\"\\.\\.\\.$\", \"\", value)]\n+\n class P4Sync(Command, P4UserMap):\n \n def __init__(self):\n Command.__init__(self)\n P4UserMap.__init__(self)\n@@ -2551,11 +2556,11 @@ def __init__(self):\n optparse.make_option(\"--keep-path\", dest=\"keepRepoPath\", action='store_true',\n help=\"Keep entire BRANCH/DIR/SUBDIR prefix during import\"),\n optparse.make_option(\"--use-client-spec\", dest=\"useClientSpec\", action='store_true',\n help=\"Only sync files that are included in the Perforce Client Spec\"),\n optparse.make_option(\"-/\", dest=\"cloneExclude\",\n- action=\"append\", type=\"string\",\n+ action=\"callback\", callback=cloneExcludeCallback, type=\"string\",\n help=\"exclude depot path\"),\n ]\n self.description = \"\"\"Imports from Perforce into a git repository.\\n\n example:\n //depot/my/project/ -- to import the current head\n@@ -2617,12 +2622,10 @@ def checkpoint(self):\n out = self.gitOutput.readline()\n if self.verbose:\n print(\"checkpoint finished: \" + out)\n \n def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n- self.cloneExclude = [re.sub(r\"\\.\\.\\.$\", \"\", path)\n- for path in self.cloneExclude]\n files = []\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n \n@@ -3888,11 +3891,10 @@ def run(self, args):\n \n if not self.cloneDestination and len(depotPaths) > 1:\n self.cloneDestination = depotPaths[-1]\n depotPaths = depotPaths[:-1]\n \n- self.cloneExclude = [\"/\"+p for p in self.cloneExclude]\n for p in depotPaths:\n if not p.startswith(\"//\"):\n sys.stderr.write('Depot paths must start with \"//\": %s\\n' % p)\n return False\n \n-- \n2.19.2\n\n"},{"id":"372927","messageId":"6eaad2582c14961ec682d299267b279ce16906ef.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 3/8] git-p4: match branches case insensitively if configured","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:24Z","receivedAt":"2019-04-01T18:02:34Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"git-p4 knows how to handle case insensitivity in file paths\nif core.ignorecase is set.\nHowever, when determining a branch for a file,\nit still does a case-sensitive prefix match.\nThis may result in some file changes to be lost on import.\n\nFor example, given the following commits\n 1. add //depot/main/file1\n 2. add //depot/DirA/file2\n 3. add //depot/dira/file3\n 4. add //depot/DirA/file4\nand \"branchList = main:DirA\" branch mapping,\ncommit 3 will be lost.\n\nSo, do branch search case insensitively if running with core.ignorecase set.\nTeach splitFilesIntoBranches() to use the p4PathStartsWith() function\nfor path prefix matches instead of always case-sensitive match.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 4 ++--\n t/t9801-git-p4-branch.sh | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c0a3068b6f..f3e5ccb7af 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2666,11 +2666,11 @@ def stripRepoPath(self, path, prefixes):\n # branch detection moves files up a level (the branch name)\n # from what client spec interpretation gives\n path = self.clientSpecDirs.map_in_client(path)\n if self.detectBranches:\n for b in self.knownBranches:\n- if path.startswith(b + \"/\"):\n+ if p4PathStartsWith(path, b + \"/\"):\n path = path[len(b)+1:]\n \n elif self.keepRepoPath:\n # Preserve everything in relative path name except leading\n # //depot/; just look at first prefix as they all should\n@@ -2721,11 +2721,11 @@ def splitFilesIntoBranches(self, commit):\n relPath = self.stripRepoPath(path, self.depotPaths)\n \n for branch in self.knownBranches.keys():\n # add a trailing slash so that a commit into qt/4.2foo\n # doesn't end up in qt/4.2, e.g.\n- if relPath.startswith(branch + \"/\"):\n+ if p4PathStartsWith(relPath, branch + \"/\"):\n if branch not in branches:\n branches[branch] = []\n branches[branch].append(file)\n break\n \ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex c48532e12b..4779448b4c 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -648,11 +648,11 @@ test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' '\n \t\tp4 submit -d \"branch1/b1f4\"\n \t)\n '\n \n # Check that files are properly split across branches when ignorecase is set\n-test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' '\n+test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' '\n \ttest_when_finished cleanup_git &&\n \ttest_create_repo \"$git\" &&\n \t(\n \t\tcd \"$git\" &&\n \t\tgit config git-p4.branchList main:branch1 &&\n@@ -674,11 +674,11 @@ test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch defini\n \t\ttest_path_is_file b1f4\n \t)\n '\n \n # Check that files are properly split across branches when ignorecase is set, use-client-spec case\n-test_expect_failure !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' '\n+test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' '\n \tclient_view \"//depot/... //client/...\" &&\n \ttest_when_finished cleanup_git &&\n \ttest_create_repo \"$git\" &&\n \t(\n \t\tcd \"$git\" &&\n-- \n2.19.2\n\n"},{"id":"372928","messageId":"b65796715480f0a859707e1bafb749a60ffb9d1d.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 5/8] git-p4: add failing test for \"don't exclude other files with same prefix\"","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:29Z","receivedAt":"2019-04-01T18:02:37Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"In preparation for a fix, add a failing test case to test that\ngit-p4 doesn't exclude files with the same prefix unintentionally\nwhen exclude paths are specified without a trailing /.\nI.e., don't exclude \"//depot/file_dont_exclude\" if run with \"-//depot/file\".\nor don't exclude \"//depot/discard_file_not\" if run with \"-//depot/discard_file\".\n\nSigned-off-by: Andrey Mazo \n---\n t/t9817-git-p4-exclude.sh | 51 +++++++++++++++++++++++++++++++++++----\n 1 file changed, 46 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh\nindex aac568eadf..1c22570797 100755\n--- a/t/t9817-git-p4-exclude.sh\n+++ b/t/t9817-git-p4-exclude.sh\n@@ -20,49 +20,90 @@ test_expect_success 'create exclude repo' '\n \t(\n \t\tcd \"$cli\" &&\n \t\tmkdir -p wanted discard &&\n \t\techo wanted >wanted/foo &&\n \t\techo discard >discard/foo &&\n-\t\tp4 add wanted/foo discard/foo &&\n+\t\techo discard_file >discard_file &&\n+\t\techo discard_file_not >discard_file_not &&\n+\t\tp4 add wanted/foo discard/foo discard_file discard_file_not &&\n \t\tp4 submit -d \"initial revision\"\n \t)\n '\n \n test_expect_success 'check the repo was created correctly' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_file discard/foo\n+\t\ttest_path_is_file discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'clone, excluding part of repo' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_missing discard/foo\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n+\t)\n+'\n+\n+test_expect_failure 'clone, excluding single file, no trailing /' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_path_is_file wanted/foo &&\n+\t\ttest_path_is_file discard/foo &&\n+\t\ttest_path_is_missing discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'clone, then sync with exclude' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$cli\" &&\n-\t\tp4 edit wanted/foo discard/foo &&\n+\t\tp4 edit wanted/foo discard/foo discard_file_not &&\n \t\tdate >>wanted/foo &&\n \t\tdate >>discard/foo &&\n+\t\tdate >>discard_file_not &&\n \t\tp4 submit -d \"updating\" &&\n \n \t\tcd \"$git\" &&\n \t\tgit p4 sync -//depot/discard/... &&\n \t\ttest_path_is_file wanted/foo &&\n-\t\ttest_path_is_missing discard/foo\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_file discard_file &&\n+\t\ttest_path_is_file discard_file_not\n+\t)\n+'\n+\n+test_expect_failure 'clone, then sync with exclude, no trailing /' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone -//depot/discard/... -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 edit wanted/foo discard/foo discard_file_not &&\n+\t\tdate >>wanted/foo &&\n+\t\tdate >>discard/foo &&\n+\t\tdate >>discard_file_not &&\n+\t\tp4 submit -d \"updating\" &&\n+\n+\t\tcd \"$git\" &&\n+\t\tgit p4 sync -//depot/discard/... -//depot/discard_file &&\n+\t\ttest_path_is_file wanted/foo &&\n+\t\ttest_path_is_missing discard/foo &&\n+\t\ttest_path_is_missing discard_file &&\n+\t\ttest_path_is_file discard_file_not\n \t)\n '\n \n test_expect_success 'kill p4d' '\n \tkill_p4d\n-- \n2.19.2\n\n"},{"id":"372929","messageId":"035abfff2a20516bc13f6b2e219ff158490ceced.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 6/8] git-p4: don't exclude other files with same prefix","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:32Z","receivedAt":"2019-04-01T18:02:38Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Make sure not to exclude files unintentionally\nif exclude paths are specified without a trailing /.\nI.e., don't exclude \"//depot/file_dont_exclude\" if run with \"-//depot/file\".\n\nDo this by ensuring that paths without a trailing \"/\" are only matched completely.\n\nAlso, abort path search on the first match as a micro-optimization.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 21 ++++++++++++++-------\n t/t9817-git-p4-exclude.sh | 4 ++--\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 7edcbad055..c47bd8c4d8 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2621,22 +2621,29 @@ def checkpoint(self):\n self.gitStream.write(\"progress checkpoint\\n\\n\")\n out = self.gitOutput.readline()\n if self.verbose:\n print(\"checkpoint finished: \" + out)\n \n+ def isPathWanted(self, path):\n+ for p in self.cloneExclude:\n+ if p.endswith(\"/\"):\n+ if p4PathStartsWith(path, p):\n+ return False\n+ # \"-//depot/file1\" without a trailing \"/\" should only exclude \"file1\", but not \"file111\" or \"file1_dir/file2\"\n+ elif path.lower() == p.lower():\n+ return False\n+ for p in self.depotPaths:\n+ if p4PathStartsWith(path, p):\n+ return True\n+ return False\n+\n def extractFilesFromCommit(self, commit, shelved=False, shelved_cl = 0):\n files = []\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n-\n- if [p for p in self.cloneExclude\n- if p4PathStartsWith(path, p)]:\n- found = False\n- else:\n- found = [p for p in self.depotPaths\n- if p4PathStartsWith(path, p)]\n+ found = self.isPathWanted(path)\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\ndiff --git a/t/t9817-git-p4-exclude.sh b/t/t9817-git-p4-exclude.sh\nindex 1c22570797..275dd30425 100755\n--- a/t/t9817-git-p4-exclude.sh\n+++ b/t/t9817-git-p4-exclude.sh\n@@ -51,11 +51,11 @@ test_expect_success 'clone, excluding part of repo' '\n \t\ttest_path_is_file discard_file &&\n \t\ttest_path_is_file discard_file_not\n \t)\n '\n \n-test_expect_failure 'clone, excluding single file, no trailing /' '\n+test_expect_success 'clone, excluding single file, no trailing /' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file wanted/foo &&\n@@ -83,11 +83,11 @@ test_expect_success 'clone, then sync with exclude' '\n \t\ttest_path_is_file discard_file &&\n \t\ttest_path_is_file discard_file_not\n \t)\n '\n \n-test_expect_failure 'clone, then sync with exclude, no trailing /' '\n+test_expect_success 'clone, then sync with exclude, no trailing /' '\n \ttest_when_finished cleanup_git &&\n \tgit p4 clone -//depot/discard/... -//depot/discard_file --dest=\"$git\" //depot/...@all &&\n \t(\n \t\tcd \"$cli\" &&\n \t\tp4 edit wanted/foo discard/foo discard_file_not &&\n-- \n2.19.2\n\n"},{"id":"372930","messageId":"2bde24b7e4d51e52614aaba1d38489cdf58c1543.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 7/8] git-p4: add failing test for \"git-p4: respect excluded paths when detecting branches\"","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:35Z","receivedAt":"2019-04-01T18:02:43Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"In preparation for a fix, add a failing test case to test that\ngit-p4 doesn't exclude files despite being told to\nwhen handling multiple branches.\n\nI.e., it should exclude //depot/branch2/file2 when run with -//depot/branch2/file2,\nbut doesn't do this right now.\n\nThe test is based on 'git p4 clone complex branches' test with the following changes:\n * account for file3 moved from branch3 to branch4 in test 'git p4 submit to two branches in a single changelist';\n * account for branch6 created in test 'git p4 clone file subset branch';\n * file2 is expected to be missing from all branches due to explicit exclude.\n\nSigned-off-by: Andrey Mazo \n---\n t/t9801-git-p4-branch.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 4779448b4c..7530d22de2 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -409,10 +409,50 @@ test_expect_failure 'git p4 clone file subset branch' '\n \t\ttest_path_is_missing file2 &&\n \t\ttest_path_is_missing file3\n \t)\n '\n \n+# Check that excluded files are omitted during import\n+test_expect_failure 'git p4 clone complex branches with excluded files' '\n+\ttest_when_finished cleanup_git &&\n+\ttest_create_repo \"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.branchList branch1:branch2 &&\n+\t\tgit config --add git-p4.branchList branch1:branch3 &&\n+\t\tgit config --add git-p4.branchList branch1:branch4 &&\n+\t\tgit config --add git-p4.branchList branch1:branch5 &&\n+\t\tgit config --add git-p4.branchList branch1:branch6 &&\n+\t\tgit 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 &&\n+\t\tgit log --all --graph --decorate --stat &&\n+\t\tgit reset --hard p4/depot/branch1 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\tgit reset --hard p4/depot/branch2 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_missing file3 &&\n+\t\tgit reset --hard p4/depot/branch3 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_missing file3 &&\n+\t\tgit reset --hard p4/depot/branch4 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\tgit reset --hard p4/depot/branch5 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_file file3 &&\n+\t\tgit reset --hard p4/depot/branch6 &&\n+\t\ttest_path_is_file file1 &&\n+\t\ttest_path_is_missing file2 &&\n+\t\ttest_path_is_missing file3\n+\t)\n+'\n+\n # From a report in http://stackoverflow.com/questions/11893688\n # where --use-client-spec caused branch prefixes not to be removed;\n # every file in git appeared into a subdirectory of the branch name.\n test_expect_success 'use-client-spec detect-branches setup' '\n \trm -rf \"$cli\" &&\n-- \n2.19.2\n\n"},{"id":"372931","messageId":"6d3ffb98a7c94f664acc1bd29a429c006d77a30c.1554141338.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[PATCH v3 8/8] git-p4: respect excluded paths when detecting branches","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T18:02:38Z","receivedAt":"2019-04-01T18:02:44Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Currently, excluded paths are only handled in the following cases:\n * no branch detection;\n * branch detection with using clientspec.\n\nHowever, excluded paths are not respected in case of\nbranch detection without using clientspec.\n\nFix this by consulting the list of excluded paths\nwhen splitting files across branches.\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 3 +--\n t/t9801-git-p4-branch.sh | 2 +-\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex c47bd8c4d8..96c4b78dc7 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2708,12 +2708,11 @@ def splitFilesIntoBranches(self, commit):\n \n branches = {}\n fnum = 0\n while \"depotFile%s\" % fnum in commit:\n path = commit[\"depotFile%s\" % fnum]\n- found = [p for p in self.depotPaths\n- if p4PathStartsWith(path, p)]\n+ found = self.isPathWanted(path)\n if not found:\n fnum = fnum + 1\n continue\n \n file = {}\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 7530d22de2..9654362052 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -410,11 +410,11 @@ test_expect_failure 'git p4 clone file subset branch' '\n \t\ttest_path_is_missing file3\n \t)\n '\n \n # Check that excluded files are omitted during import\n-test_expect_failure 'git p4 clone complex branches with excluded files' '\n+test_expect_success 'git p4 clone complex branches with excluded files' '\n \ttest_when_finished cleanup_git &&\n \ttest_create_repo \"$git\" &&\n \t(\n \t\tcd \"$git\" &&\n \t\tgit config git-p4.branchList branch1:branch2 &&\n-- \n2.19.2\n\n"},{"id":"372936","messageId":"20190401195342.17515-1-amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"Re: [PATCH v3 0/8] git-p4: a few assorted fixes for branches, excludes","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-01T19:54:02Z","receivedAt":"2019-04-01T19:54:08Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"> This series fixes a few cases with branch detection\n> and handling of excludes by git-p4.\n> \n> This is the third iteration of the patch series.\n> Changes since v2 [2]:\n> * Added new test cases for case-insensitive branch detection.\n\nForgot to add:\n * Added a fix for case-insensitive branch detection when running with useClientSpec enabled.\n (range diff below correctly shows the e644a8ab49 vs 6eaad2582c difference)\n\n> Changes since v1 [1]:\n> * Added new test case for excluded paths when detecting branches;\n> * Added a new fix for excluded paths when detecting branches.\n> \n> [1] https://public-inbox.org/git/cover.1551485349.git.amazo@checkvideo.com\n> [2] https://public-inbox.org/git/cover.1553207234.git.amazo@checkvideo.com/\n> \n> Range-diff vs v2:\n> 1: 3ac39171d4 = 1: bd009a5ca5 git-p4: detect/prevent infinite loop in gitCommitByP4Change()\n> 2: e644a8ab49 < -: ---------- git-p4: match branches case insensitively if configured\n> -: ---------- > 2: 68b68ce1e4 git-p4: add failing test for \"git-p4: match branches case insensitively if configured\"\n> -: ---------- > 3: 6eaad2582c git-p4: match branches case insensitively if configured\n> 3: 44fed954dc = 4: 1bd5e170e0 git-p4: don't groom exclude path list on every commit\n> 4: a0d3fa6add = 5: b657967154 git-p4: add failing test for \"don't exclude other files with same prefix\"\n> 5: 3330f88a0d = 6: 035abfff2a git-p4: don't exclude other files with same prefix\n> 6: 6170d45951 = 7: 2bde24b7e4 git-p4: add failing test for \"git-p4: respect excluded paths when detecting branches\"\n> 7: 758d8e8486 = 8: 6d3ffb98a7 git-p4: respect excluded paths when detecting branches\n\n--\nAndrey\n"},{"id":"372952","messageId":"cover.1554162242.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554141338.git.amazo@checkvideo.com","subject":"[RFC PATCH v2 0/2] git-p4: inexact labels and load changelist description from file","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-02T00:13:37Z","receivedAt":"2019-04-02T00:13:43Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"This patch series introduces two experimental features to git-p4,\nunrelated to each other.\n 1. The first patch adds support for \"inexact\" label detection.\n The feature lets git-p4 find a git commit for a Perforce label\n even if there is no git commit with exact same changelist number as in Perforce.\n It is particularly useful when splitting a large Perforce depot\n into multiple git repositories\n or when importing just a subset of a depot into git.\n 2. The second patch adds support for loading changelist description from a file.\n (`p4 -G describe` equivalent)\n The original use case is to be able to migrate a Perforce depot,\n which database got a little bit corrupted, into git.\n\nThis patch series should be applied on top of\n\"[PATCH v3 0/8] git-p4: a few assorted fixes for branches, excludes\" [1]\n\nThis is the second iteration of the patch series.\nChanges since the v1 [1]:\n * Dropped \"alien\" branch feature;\n * Added \"inexact\" label feature by suggestion from Luke;\n * Added minimal documentation;\n * Changed \"damaged\"-oriented narrative to more generic one.\n (renamed \"git-p4.damagedChangelists\" to \"git-p4.changelistDescriptionFile\",\n functions and variables correspondingly)\n\nRange-diff vs v1:\n1: b02df749b9 < -: ---------- git-p4: introduce alien branch mappings\n-: ---------- > 1: 54ef897fcf git-p4: inexact label detection\n2: bb3e14a389 ! 2: 83b0034538 git-p4: support loading changelist descriptions from files\n\n[1] https://public-inbox.org/git/cover.1554141338.git.amazo@checkvideo.com/\n\nAndrey Mazo (2):\n git-p4: inexact label detection\n git-p4: support loading changelist descriptions from files\n\n Documentation/git-p4.txt | 41 +++++++++++++++++++++++\n git-p4.py | 72 ++++++++++++++++++++++++++++++++++++----\n 2 files changed, 106 insertions(+), 7 deletions(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\nprerequisite-patch-id: 23e039fec7a1f5c51c98326a14d788adb1ecb5ba\nprerequisite-patch-id: 9840851ffca6f00126c9c91da5a8828c7d0dcaed\nprerequisite-patch-id: 32a738b41fb3dccfbbfb4d382a9748e36dcdfa8b\nprerequisite-patch-id: 10661f77392f4131d2375976c77a7cd231fdf9ab\nprerequisite-patch-id: a55360c904eba1b9e9c934405d3141eb96c5ad30\nprerequisite-patch-id: 46357586199c02d956d53d782a12f1ee0c991302\nprerequisite-patch-id: c683e7d6017580df9385a1544af409ca615d770c\nprerequisite-patch-id: 411dcb5e95aff036e0cb3e850ea75f2424b260a6\n-- \n2.19.2\n\n"},{"id":"372953","messageId":"54ef897fcf645d241690ce3be6867cb60d829552.1554162242.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554162242.git.amazo@checkvideo.com","subject":"[RFC PATCH v2 1/2] git-p4: inexact label detection","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-02T00:13:43Z","receivedAt":"2019-04-02T00:13:52Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Labels in Perforce are not global, but can be placed on a particular view/subdirectory.\nThis might pose difficulties when importing only parts of Perforce depot into a git repository.\nFor example:\n 1. Depot layout is as follows:\n //depot/metaproject/branch1/subprojectA/...\n //depot/metaproject/branch1/subprojectB/...\n //depot/metaproject/branch2/subprojectA/...\n //depot/metaproject/branch2/subprojectB/...\n 2. Labels are placed as follows:\n * label 1A on //depot/metaproject/branch1/subprojectA/...\n * label 1B on //depot/metaproject/branch1/subprojectB/...\n * label 2A on //depot/metaproject/branch2/subprojectA/...\n * label 2B on //depot/metaproject/branch2/subprojectB/...\n 3. The goal is to import\n subprojectA into subprojectA.git and\n subprojectB into subprojectB.git\n preserving all the branches and labels.\n 4. Importing subprojectA.\n Label 1A is imported fine because it's placed on certain commit on branch1.\n However, label 1B is not imported because it's placed on a commit in another subproject:\n git-p4 says: \"importing label 1B: could not find git commit for changelist ...\"\n The same is with label 2A, which is imported; and 2B, which is not.\n\nCurrently, there is no easy way (that I'm aware of) to tell git-p4 to\nimport an empty commit into a desired branch,\nso that a label placed on that changelist could be imported as well,\nIt might be possible to get a similar effect by importing both subprojectA and B in a single git repo,\nand then running `git filter-branch --subdirectory-filter subprojectA`,\nbut this might produce way more irrelevant empty commits, than needed for labels.\n(although imported changelists can be limited with git-p4 --changesfile option)\nAlso, `git filter-branch` is harder to use for incremental imports\nor when changes are submitted from git back to Perforce.\n\nAs suggested by Luke,\ninstead of creating an empty commit for the sole purpose of being tagged later,\nteach git-p4 to search harder for the next lower changelist,\ncorresponding to the label in question.\n\nDo this by finding the highest changelist up to the label under all known branches,\n(branches are finalized by the time importP4Labels() runs)\nand using it instead of a depot-wide changelist corresponding to the label.\n\nThis new behavior may not be desired for people,\nwho want exact label <-> changelist relationship.\nSo, add a new boolean config parameter git-p4.allowInexactLabels (defaults to false)\nto explicitly enable it if needed.\nAlso, this behavior only appears to be useful in case of multiple branches,\n(otherwise, every Perforce changelist should appear in git)\nso it's not engaged when running without branch detection.\n\nDetect and report (--verbose) \"inexact\" tags,\ni.e. tags placed on a lower changelist than was in Perforce.\nImplement this by comparing a changelist for which a commit was found\nwith a changelist corresponding to the label on the whole depot.\n\nNote, that the new \"inexact\" logic works slower\nthan the original code in case of numerous branches,\nbecause p4 needs to calculate the most recent change for each branch path instead of just one.\n\nThis is an alternative solution to \"alien\" branches concept proposed earlier:\nhttps://public-inbox.org/git/b02df749b9266ac8c73707617a171122156621ab.1553283214.git.amazo@checkvideo.com/\n\nSigned-off-by: Andrey Mazo \nSuggested-by: Luke Diamand \n---\n Documentation/git-p4.txt | 14 ++++++++++++\n git-p4.py | 48 +++++++++++++++++++++++++++++++++++-----\n 2 files changed, 56 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 3494a1db3e..ceabab8b86 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -582,10 +582,24 @@ git-p4.importLabels::\n \n git-p4.labelImportRegexp::\n \tOnly p4 labels matching this regular expression will be imported. The\n \tdefault value is '[a-zA-Z0-9_\\-.]+$'.\n \n+git-p4.allowInexactLabels::\n+\tOnly has an effect if run with `--detect-branches`.\n+\tBy default, when performing p4 label import,\n+\t'git p4' finds a changelist number of every label,\n+\tthen finds a git commit corresponding to the found changelist number,\n+\tand then places an annotated tag on the found git commit.\n+\tIf a git commit is not found, the label is considered unimportable\n+\tand is added to 'ignoredP4Labels' list.\n+\tIf 'allowInexactLabels' is set to true,\n+\t'git p4' only considers changelists under branches being imported.\n+\tThis has an effect that a tag in git might be placed on a lower changelist compared to p4.\n+\tThis might be useful when importing just a subset of the depot into git,\n+\tif a label would be discarded otherwise.\n+\n git-p4.useClientSpec::\n \tSpecify that the p4 client spec should be used to identify p4\n \tdepot paths of interest. This is equivalent to specifying the\n \toption `--use-client-spec`. See the \"CLIENT SPEC\" section above.\n \tThis variable is a boolean, not the name of a p4 client.\ndiff --git a/git-p4.py b/git-p4.py\nindex 96c4b78dc7..98b2b7bbca 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3162,17 +3162,43 @@ def importP4Labels(self, stream, p4Labels):\n if name in ignoredP4Labels:\n continue\n \n labelDetails = p4CmdList(['label', \"-o\", name])[0]\n \n- # get the most recent changelist for each file in this label\n- change = p4Cmd([\"changes\", \"-m\", \"1\"] + [\"%s...@%s\" % (p, name)\n+ if self.detectBranches and gitConfigBool(\"git-p4.allowInexactLabels\"):\n+ doInexactLabels = True\n+ else:\n+ doInexactLabels = False\n+\n+ # get the most recent changelist in this label for the whole depot\n+ depot_wide_changelist = p4Cmd([\"changes\", \"-m\", \"1\"] + [\"%s...@%s\" % (p, name)\n for p in self.depotPaths])\n+ if 'change' in depot_wide_changelist:\n+ depot_wide_changelist = int(depot_wide_changelist['change'])\n+ else:\n+ depot_wide_changelist = None\n \n- if 'change' in change:\n+ # get the most recent changelist for each file under branches of interest in this label\n+ if doInexactLabels:\n+ if self.useClientSpec:\n+ paths = [\"%s...@%s\" % (self.clientSpecDirs.client_prefix + p + '/', name) for p in self.knownBranches]\n+ else:\n+ paths = [\"%s...@%s\" % (self.depotPaths[0] + p + '/', name) for p in self.knownBranches]\n+ changes = p4CmdList([\"changes\", \"-m\", \"1\"] + paths)\n+ changes = [int(c['change']) for c in changes if 'change' in c]\n+\n+ # there may be different \"most recent\" changelists for different paths.\n+ # take the newest since some paths were just modified later than others.\n+ if changes:\n+ changelist = max(changes)\n+ else:\n+ changelist = None\n+ else:\n+ changelist = depot_wide_changelist\n+\n+ if changelist:\n # find the corresponding git commit; take the oldest commit\n- changelist = int(change['change'])\n if changelist in self.committedChanges:\n gitCommit = \":%d\" % changelist # use a fast-import mark\n commitFound = True\n else:\n gitCommit = read_pipe([\"git\", \"rev-list\", \"--max-count=1\",\n@@ -3192,14 +3218,24 @@ def importP4Labels(self, stream, p4Labels):\n tmwhen = 1\n \n when = int(time.mktime(tmwhen))\n self.streamTag(stream, name, labelDetails, gitCommit, when)\n if verbose:\n- print(\"p4 label %s mapped to git commit %s\" % (name, gitCommit))\n+ if depot_wide_changelist == changelist:\n+ isExact = \"\"\n+ else:\n+ isExact = \" inexactly\"\n+ print(\"p4 label %s mapped%s to git commit %s\" % (name, isExact, gitCommit))\n else:\n if verbose:\n- print(\"Label %s has no changelists - possibly deleted?\" % name)\n+ if depot_wide_changelist:\n+ # there is a changelist corresponding to this label,\n+ # but it's not under any branches of interest.\n+ print(\"Label %s has no changelists under detected branches -- ignoring\" % name)\n+ else:\n+ # there is no changelist corresponding to this label in the whole depot\n+ print(\"Label %s has no changelists - possibly deleted?\" % name)\n \n if not commitFound:\n # We can't import this label; don't try again as it will get very\n # expensive repeatedly fetching all the files for labels that will\n # never be imported. If the label is moved in the future, the\n-- \n2.19.2\n\n"},{"id":"372954","messageId":"83b00345385bee36957f3812dda866826a8c2547.1554162242.git.amazo@checkvideo.com","threadId":"50645","inReplyTo":"cover.1554162242.git.amazo@checkvideo.com","subject":"[RFC PATCH v2 2/2] git-p4: support loading changelist descriptions from files","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-02T00:13:48Z","receivedAt":"2019-04-02T00:14:10Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Our Perforce server experienced some kind of database corruption a few years ago.\nWhile the file data and revision history are mostly intact,\nsome metadata for several changesets got lost.\nFor example, inspecting certain changelists produces errors.\n\"\"\"\n$ p4 describe -s 12345\nDate 2019/02/26 16:46:17:\nOperation: user-describe\nOperation 'user-describe' failed.\nChange 12345 description missing!\n\"\"\"\n\nWhile some metadata (like changeset descriptions) is obviously lost,\nmost of it can be reconstructed via other commands:\n * `p4 changes -l -t //...@12345,12345` --\n to obtain date+time, author, beginning of changeset description;\n * `p4 files -a //...@12345,12345` --\n to obtain file revisions, file types, file actions;\n * `p4 diff2 -u //...@12344 //...@12345` --\n to get a unified diff of text files in a changeset;\n * `p4 print -o binary.blob@12345 //depot/binary.blob@12345` --\n to get a revision of a binary file.\n\nIt might be possible to teach git-p4 to fallback to other methods if `p4 describe` fails,\nbut it's probably too special-cased (really depends on kind and scale of DB corruption),\nso some manual intervention is perhaps acceptable.\n\nSo, with some manual work, it's possible to reconstruct `p4 -G describe ...` output manually.\nIn our case, once git-p4 passes `p4 describe` stage,\nit can proceed further just fine.\nThus, it's tempting to feed resurrected metadata to git-p4 when a normal `p4 describe` would fail.\n\nThis functionality may be useful to cache changelist information,\nor to make some changes to changelist info before feeding it to git-p4.\n\nA new config parameter is introduced to tell git-p4\nto load certain changelist descriptions from files instead of from a server.\nFor simplicity, it's one marshalled file per changelist.\n```\ngit config --add git-p4.changelistDescriptionFile 12345.marshal\ngit config --add git-p4.changelistDescriptionFile 12346.marshal\n```\n\nThe following trivial script may be used to produce marshalled `p4 -G describe`-compatible output.\n\"\"\"\n #!/usr/bin/env python\n\n import marshal\n import time\n\n # recovered commits of interest\n changes = [\n {\n 'change': '12345',\n 'status': 'submitted',\n 'code': 'stat',\n 'user': 'username1',\n 'time': str(int(time.mktime(time.strptime('2019/02/28 16:00:30', '%Y/%m/%d %H:%M:%S')))),\n 'client': 'username1_hostname1',\n 'desc': 'A bug is fixed.\\nDetails are below:\\n',\n 'depotFile0': '//depot/branch1/foo.sh',\n 'action0': 'edit',\n 'rev0': '28',\n 'type0': 'xtext',\n 'depotFile1': '//depot/branch1/bar.py',\n 'action1': 'edit',\n 'rev1': '43',\n 'type1': 'text',\n 'depotFile2': '//depot/branch1/baz.doc',\n 'action2': 'edit',\n 'rev2': '8',\n 'type2': 'binary',\n 'depotFile3': '//depot/branch1/qqq.c',\n 'action3': 'edit',\n 'rev3': '6',\n 'type3': 'ktext',\n },\n ]\n\n for change in changes:\n marshal.dump(change, open('{0}.marshal'.format(change['change']), 'wb'))\n\"\"\"\n\nOr, the following script may be used to produce marshalled `p4 -G describe`-compatible output\nfor our particular database corruption.\n\"\"\"\n #!/usr/bin/env python\n\n import itertools\n import marshal\n import subprocess\n import tempfile\n\n def p4_unmarshal(fileobj):\n result = []\n while True:\n try:\n result += [marshal.load(fileobj)]\n except EOFError:\n break\n\n return result\n\n def p4_describe_fallback(cl):\n with tempfile.TemporaryFile() as p4_changes_output:\n with tempfile.TemporaryFile() as p4_files_output:\n subprocess.check_call(['p4', '-G', 'changes', '-l', '-t', '//...@{0},{0}'.format(cl)], stdout=p4_changes_output)\n subprocess.check_call(['p4', '-G', 'files', '-a', '//...@{0},{0}'.format(cl)], stdout=p4_files_output)\n\n p4_changes_output.seek(0)\n p4_files_output.seek(0)\n\n p4_changes_unmarshalled = p4_unmarshal(p4_changes_output)\n p4_files_unmarshalled = p4_unmarshal(p4_files_output)\n\n described_cl = p4_changes_unmarshalled[0] # there is usually only one entry\n described_cl['desc'] += '\\n'\n\n assert described_cl['change'] == str(cl)\n\n for (file_info, i) in itertools.izip(p4_files_unmarshalled, itertools.count()):\n for f in ('depotFile', 'action', 'rev', 'type'):\n described_cl['{}{}'.format(f, i)] = file_info[f]\n\n assert file_info['change'] == described_cl['change']\n\n return described_cl\n\n cls_wanted = ( 12345, 12346 )\n\n for cl in cls_wanted:\n with open('{0}.marshal'.format(cl), 'wb') as f:\n cl_info = p4_describe_fallback(cl)\n marshal.dump(cl_info, f)\n\"\"\"\n\nSigned-off-by: Andrey Mazo \n---\n Documentation/git-p4.txt | 27 +++++++++++++++++++++++++++\n git-p4.py | 24 +++++++++++++++++++++++-\n 2 files changed, 50 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex ceabab8b86..f751ae729f 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -654,10 +654,37 @@ git config --add git-p4.mapUser \"p4user = First Last \"\n -------------\n +\n A mapping will override any user information from P4. Mappings for\n multiple P4 user can be defined.\n \n+git-p4.changelistDescriptionFile::\n+\tThis config variable points 'git p4' to a file,\n+\tcontaining a serialized (marshalled) changelist description.\n+\t'git p4' loads a description from such a file\n+\tinstead of asking Perforce server about it.\n+\tThe file format is the same as produced by 'p4 -G describe -s '.\n+\tThis option can be specified multiple times\n+\tto feed multiple changelist descriptions to 'git p4'.\n+\tThe path is relative to git work tree,\n+\tfile names or extensions don't matter.\n+\tThis example loads 2 changelist descriptions:\n++\n+-------------\n+git config --add git-p4.changelistDescriptionFile cl-12345.marshal\n+git config --add git-p4.changelistDescriptionFile cl-12347.marshal\n+-------------\n++\n+Under some circumstances (for example, Perforce database corruption)\n+this option is useful to supply changelist description to 'git p4' bypassing 'p4'.\n+Also, it can be used for caching of changelist descriptions\n+to reduce load on the Perforce server in case of successive imports\n+(say, when splitting the depot into multiple Git repositories)\n+or for overriding some changelist information.\n+This config variable is generally not needed after the initial import,\n+so it can be removed from the config file\n+together with corresponding description files after the import.\n+\n Submit variables\n ~~~~~~~~~~~~~~~~\n git-p4.detectRenames::\n \tDetect renames. See linkgit:git-diff[1]. This can be true,\n \tfalse, or a score as expected by 'git diff -M'.\ndiff --git a/git-p4.py b/git-p4.py\nindex 98b2b7bbca..e65df92d75 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2613,10 +2613,12 @@ def __init__(self):\n self.initialParents = {}\n \n self.tz = \"%+03d%02d\" % (- time.timezone / 3600, ((- time.timezone % 3600) / 60))\n self.labels = {}\n \n+ self.loadedChangelistDescriptions = {}\n+\n # Force a checkpoint in fast-import and wait for it to finish\n def checkpoint(self):\n self.gitStream.write(\"checkpoint\\n\\n\")\n self.gitStream.write(\"progress checkpoint\\n\\n\")\n out = self.gitOutput.readline()\n@@ -3319,10 +3321,25 @@ def getBranchMappingFromGitBranches(self):\n branch = \"main\"\n else:\n branch = branch[len(self.projectName):]\n self.knownBranches[branch] = branch\n \n+ def loadChangelistDescFromFile(self):\n+ changelistDescriptionFiles = gitConfigList(\"git-p4.changelistDescriptionFile\")\n+ for clMarshalledDescFile in changelistDescriptionFiles:\n+ if not clMarshalledDescFile:\n+ continue\n+\n+ try:\n+ with open(clMarshalledDescFile, 'rb') as clFileObj:\n+ clDesc = marshal.load(clFileObj)\n+ if not (\"status\" in clDesc and \"user\" in clDesc and \"time\" in clDesc and \"change\" in clDesc):\n+ die(\"Changelist description read from {0} doesn't have required fields\".format(clMarshalledDescFile))\n+ self.loadedChangelistDescriptions[int(clDesc[\"change\"])] = clDesc\n+ except (IOError, TypeError, ValueError, EOFError) as e:\n+ die(\"Can't read changelist description from {0}: {1}\".format(clMarshalledDescFile, str(e)))\n+\n def updateOptionDict(self, d):\n option_keys = {}\n if self.keepRepoPath:\n option_keys['keepRepoPath'] = 1\n \n@@ -3420,11 +3437,14 @@ def searchParent(self, parent, branch, target):\n return None\n \n def importChanges(self, changes, origin_revision=0):\n cnt = 1\n for change in changes:\n- description = p4_describe(change)\n+ if change in self.loadedChangelistDescriptions:\n+ description = self.loadedChangelistDescriptions[change]\n+ else:\n+ description = p4_describe(change)\n self.updateOptionDict(description)\n \n if not self.silent:\n sys.stdout.write(\"\\rImporting revision %s (%s%%)\" % (change, cnt * 100 / len(changes)))\n sys.stdout.flush()\n@@ -3706,10 +3726,12 @@ def run(self, args):\n bad_changesfile = True\n break\n if bad_changesfile:\n die(\"Option --changesfile is incompatible with revision specifiers\")\n \n+ self.loadChangelistDescFromFile()\n+\n newPaths = []\n for p in self.depotPaths:\n if p.find(\"@\") != -1:\n atIdx = p.index(\"@\")\n self.changeRange = p[atIdx:]\n-- \n2.19.2\n\n"},{"id":"372978","messageId":"20190402120537.GK32732@szeder.dev","threadId":"50645","inReplyTo":"68b68ce1e4782bba552a016867bfc629f0d5e24f.1554141338.git.amazo@checkvideo.com","subject":"Re: [PATCH v3 2/8] git-p4: add failing test for \"git-p4: match branches case insensitively if configured\"","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-04-02T12:05:37Z","receivedAt":"2019-04-02T12:05:45Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi Junio and Andrey,\n\nOn Mon, Apr 01, 2019 at 06:02:21PM +0000, Mazo, Andrey wrote:\n> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\n> index 6a86d6996b..c48532e12b 100755\n> --- a/t/t9801-git-p4-branch.sh\n> +++ b/t/t9801-git-p4-branch.sh\n> @@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie\n> \t\tcd branch1 &&\n> \t\tgrep \"client spec\" file1\n> \t)\n> '\n> \n> +test_expect_success 'restart p4d (case folding enabled)' '\n> +\tkill_p4d &&\n> +\tstart_p4d -C1\n> +'\n\nThere is a semantic conflict between this patch and commit 07353d9042\n(git p4 test: clean up the p4d cleanup functions, 2019-03-13) in\n'sg/test-atexit' currently cooking in 'pu': this patch adds a new\ncallsite of 'kill_p4d', but that commit renamed that function to\n'stop_and_cleanup_p4d'. Consequently, t9801 on 'pu' now fails with:\n\n +kill_p4d\n t9801-git-p4-branch.sh: 4: eval: kill_p4d: not found\n error: last command exited with $?=127\n not ok 28 - restart p4d (case folding enabled)\n\nhttps://travis-ci.org/git/git/jobs/514513463#L5827\n\nI wonder whether it would be worth amending 07353d9042 to keep\n'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the\ntime being.\n\n\nhttps://public-inbox.org/git/20190313122419.2210-9-szeder.dev@gmail.com/\n\n"},{"id":"372986","messageId":"20190402171317.1364-1-amazo@checkvideo.com","threadId":"50645","inReplyTo":"20190402120537.GK32732@szeder.dev","subject":"Re: [PATCH v3 2/8] git-p4: add failing test for \"git-p4: match branches case insensitively if configured\"","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2019-04-02T17:13:33Z","receivedAt":"2019-04-02T17:13:40Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"> On Mon, Apr 01, 2019 at 06:02:21PM +0000, Mazo, Andrey wrote:\n>> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\n>> index 6a86d6996b..c48532e12b 100755\n>> --- a/t/t9801-git-p4-branch.sh\n>> +++ b/t/t9801-git-p4-branch.sh\n>> @@ -608,10 +608,102 @@ test_expect_success 'Update a file in git side and submit to P4 using client vie\n>> \t\tcd branch1 &&\n>> \t\tgrep \"client spec\" file1\n>> \t)\n>> '\n>>\n>> +test_expect_success 'restart p4d (case folding enabled)' '\n>> +\tkill_p4d &&\n>> +\tstart_p4d -C1\n>> +'\n> \n> There is a semantic conflict between this patch and commit 07353d9042\n> (git p4 test: clean up the p4d cleanup functions, 2019-03-13) in\n> 'sg/test-atexit' currently cooking in 'pu': this patch adds a new\n> callsite of 'kill_p4d', but that commit renamed that function to\n> 'stop_and_cleanup_p4d'. Consequently, t9801 on 'pu' now fails with:\n> \n> +kill_p4d\n> t9801-git-p4-branch.sh: 4: eval: kill_p4d: not found\n> error: last command exited with $?=127\n> not ok 28 - restart p4d (case folding enabled)\n> \n> https://travis-ci.org/git/git/jobs/514513463#L5827\n> \n> I wonder whether it would be worth amending 07353d9042 to keep\n> 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the\n> time being.\n> \n> \n> https://public-inbox.org/git/20190313122419.2210-9-szeder.dev@gmail.com/\n\nGood catch!\n\nDon't know what's the proper workflow here, but I see 2 more options:\n * Resolve the conflict in t/t9801-git-p4-branch.sh while merging am/p4-branches-excludes\n commit d15068a650 (\"git-p4: respect excluded paths when detecting branches\", 2019-04-01)\n * I can rebase my git-p4 changes on top of sg/test-atexit branch\n commit 74ec8cf674 (\"t9811-git-p4-label-import: fix pipeline negation\", 2019-03-13)\n\nIn case this might be helpful,\nI did a conflict resolution locally,\n(by doing `git checkout d15068a650; git merge 74ec8cf674`)\nand here's the patch of the merge.\n\nBasically,\n * a newly added \"kill_p4d\" is replaced with \"stop_and_cleanup_p4d\"; and\n * \"kill_p4d\" in the end of the script is removed.\n\ndiff --cc t/t9801-git-p4-branch.sh\nindex 38d6b9043b,9654362052..67ff2711f5\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@@ -610,4 -650,100 +650,96 @@@ test_expect_success 'Update a file in g\n \t)\n '\n \n+ test_expect_success 'restart p4d (case folding enabled)' '\n -\tkill_p4d &&\n++\tstop_and_cleanup_p4d &&\n+ \tstart_p4d -C1\n+ '\n+ \n+ #\n+ # 1: //depot/main/mf1\n+ # 2: integrate //depot/main/... -> //depot/branch1/...\n+ # 3: //depot/main/mf2\n+ # 4: //depot/BRANCH1/B1f3\n+ # 5: //depot/branch1/b1f4\n+ #\n+ test_expect_success !CASE_INSENSITIVE_FS 'basic p4 branches for case folding' '\n+ \t(\n+ \t\tcd \"$cli\" &&\n+ \t\tmkdir -p main &&\n+ \n+ \t\techo mf1 >main/mf1 &&\n+ \t\tp4 add main/mf1 &&\n+ \t\tp4 submit -d \"main/mf1\" &&\n+ \n+ \t\tp4 integrate //depot/main/... //depot/branch1/... &&\n+ \t\tp4 submit -d \"integrate main to branch1\" &&\n+ \n+ \t\techo mf2 >main/mf2 &&\n+ \t\tp4 add main/mf2 &&\n+ \t\tp4 submit -d \"main/mf2\" &&\n+ \n+ \t\tmkdir BRANCH1 &&\n+ \t\techo B1f3 >BRANCH1/B1f3 &&\n+ \t\tp4 add BRANCH1/B1f3 &&\n+ \t\tp4 submit -d \"BRANCH1/B1f3\" &&\n+ \n+ \t\techo b1f4 >branch1/b1f4 &&\n+ \t\tp4 add branch1/b1f4 &&\n+ \t\tp4 submit -d \"branch1/b1f4\"\n+ \t)\n+ '\n+ \n+ # Check that files are properly split across branches when ignorecase is set\n+ test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone, branchList branch definition, ignorecase' '\n+ \ttest_when_finished cleanup_git &&\n+ \ttest_create_repo \"$git\" &&\n+ \t(\n+ \t\tcd \"$git\" &&\n+ \t\tgit config git-p4.branchList main:branch1 &&\n+ \t\tgit config --type=bool core.ignoreCase true &&\n+ \t\tgit p4 clone --dest=. --detect-branches //depot@all &&\n+ \n+ \t\tgit log --all --graph --decorate --stat &&\n+ \n+ \t\tgit reset --hard p4/master &&\n+ \t\ttest_path_is_file mf1 &&\n+ \t\ttest_path_is_file mf2 &&\n+ \t\ttest_path_is_missing B1f3 &&\n+ \t\ttest_path_is_missing b1f4 &&\n+ \n+ \t\tgit reset --hard p4/depot/branch1 &&\n+ \t\ttest_path_is_file mf1 &&\n+ \t\ttest_path_is_missing mf2 &&\n+ \t\ttest_path_is_file B1f3 &&\n+ \t\ttest_path_is_file b1f4\n+ \t)\n+ '\n+ \n+ # Check that files are properly split across branches when ignorecase is set, use-client-spec case\n+ test_expect_success !CASE_INSENSITIVE_FS 'git p4 clone with client-spec, branchList branch definition, ignorecase' '\n+ \tclient_view \"//depot/... //client/...\" &&\n+ \ttest_when_finished cleanup_git &&\n+ \ttest_create_repo \"$git\" &&\n+ \t(\n+ \t\tcd \"$git\" &&\n+ \t\tgit config git-p4.branchList main:branch1 &&\n+ \t\tgit config --type=bool core.ignoreCase true &&\n+ \t\tgit p4 clone --dest=. --use-client-spec --detect-branches //depot@all &&\n+ \n+ \t\tgit log --all --graph --decorate --stat &&\n+ \n+ \t\tgit reset --hard p4/master &&\n+ \t\ttest_path_is_file mf1 &&\n+ \t\ttest_path_is_file mf2 &&\n+ \t\ttest_path_is_missing B1f3 &&\n+ \t\ttest_path_is_missing b1f4 &&\n+ \n+ \t\tgit reset --hard p4/depot/branch1 &&\n+ \t\ttest_path_is_file mf1 &&\n+ \t\ttest_path_is_missing mf2 &&\n+ \t\ttest_path_is_file B1f3 &&\n+ \t\ttest_path_is_file b1f4\n+ \t)\n+ '\n+ \n -test_expect_success 'kill p4d' '\n -\tkill_p4d\n -'\n -\n test_done\n\n\n"},{"id":"373003","messageId":"xmqq4l7ftvgl.fsf@gitster-ct.c.googlers.com","threadId":"50645","inReplyTo":"20190402120537.GK32732@szeder.dev","subject":"Re: [PATCH v3 2/8] git-p4: add failing test for \"git-p4: match branches case insensitively if configured\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-03T07:10:50Z","receivedAt":"2019-04-03T07:10:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> I wonder whether it would be worth amending 07353d9042 to keep\n> 'kill_p4d' around as a wrapper around 'stop_and_cleanup_p4d' for the\n> time being.\n\nI think renaming was the right thing to do; \"show --cc\" shows that\nthe post-image lives in the new world order where calling kill_p4d\nis not usually needed clearly with a hunk that replaces kill_p4d\n(which only existed in the old world) with stop_and_cleanup (which\nexists only in the new world).\n\nWill redo the merge and teach my rerere database.\n\nThanks.\n"}]}