{"thread":{"id":"57654","subject":"[PATCH] git-p4: support explicit sync of arbitrary existing git-p4 refs","startedAt":"2022-04-04T05:11:32Z","lastAt":"2022-04-04T05:11:32Z","messageCount":1,"participants":["Tao Klerks via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"453017","messageId":"pull.1202.git.1649049054600.gitgitgadget@gmail.com","threadId":"57654","inReplyTo":null,"subject":"[PATCH] git-p4: support explicit sync of arbitrary existing git-p4 refs","fromName":"Tao Klerks via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-04-04T05:10:54Z","receivedAt":"2022-04-04T05:11:32Z","isPatch":true,"sender":{"key":"tao@klerks.biz","avatar":"https://avatars.githubusercontent.com/u/531704?v=4"},"body":"From: Tao Klerks <tao@klerks.biz>\n\nWith the --branch argument of the \"sync\" subcommand, git-p4 enables\nyou to import a perforce branch/path to an arbitrary git ref, using\na full ref path, or to refs/remotes/p4/* or refs/heads/p4/*,\ndepending on --import-local, using a short ref name.\n\nHowever, when you later want to explicitly sync such a given ref to\npick up subsequent p4 changes, it only works if the ref was placed\nin the p4 path *and* has only one path component (no \"/\").\n\nThis limitation results from a bad assumption in the\nexisting-branch sync logic, and also means you cannot individually\nsync branches detected by --detect-branches, as these also get a\n\"/\" in their names.\n\nFix \"git p4 sync --branch\", when called with an existing ref, so\nthat it works correctly regardless of whether the ref is in the p4\npath or not, and (in the case of refs in the p4 path) regardless of\nwhether it has a \"/\" in its short name or not.\n\nAlso add tests to validate that these branch-specific syncs work\nas expected.\n\nSigned-off-by: Tao Klerks <tao@klerks.biz>\n---\n    git-p4: support explicit sync of arbitrary existing git-p4 refs\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1202%2FTaoK%2Fgit-p4-structured-paths-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1202/TaoK/git-p4-structured-paths-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1202\n\n git-p4.py                | 52 +++++++++++++++++-------\n t/t9800-git-p4-basic.sh  | 85 ++++++++++++++++++++++++++++++++++++++++\n t/t9801-git-p4-branch.sh | 10 +++++\n 3 files changed, 133 insertions(+), 14 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex a9b1f904410..48d5805ee41 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -893,6 +893,36 @@ def gitConfigList(key):\n             _gitConfig[key] = []\n     return _gitConfig[key]\n \n+def fullP4Ref(incomingRef, importIntoRemotes=True):\n+    \"\"\"Standardize a given provided p4 ref value to a full git ref:\n+         refs/foo/bar/branch -> use it exactly\n+         p4/branch -> prepend refs/remotes/ or refs/heads/\n+         branch -> prepend refs/remotes/p4/ or refs/heads/p4/\"\"\"\n+    if incomingRef.startswith(\"refs/\"):\n+        return incomingRef\n+    if importIntoRemotes:\n+        prepend = \"refs/remotes/\"\n+    else:\n+        prepend = \"refs/heads/\"\n+    if not incomingRef.startswith(\"p4/\"):\n+        prepend += \"p4/\"\n+    return prepend + incomingRef\n+\n+def shortP4Ref(incomingRef, importIntoRemotes=True):\n+    \"\"\"Standardize to a \"short ref\" if possible:\n+         refs/foo/bar/branch -> ignore\n+         refs/remotes/p4/branch or refs/heads/p4/branch -> shorten\n+         p4/branch -> shorten\"\"\"\n+    if importIntoRemotes:\n+        longprefix = \"refs/remotes/p4/\"\n+    else:\n+        longprefix = \"refs/heads/p4/\"\n+    if incomingRef.startswith(longprefix):\n+        return incomingRef[len(longprefix):]\n+    if incomingRef.startswith(\"p4/\"):\n+        return incomingRef[3:]\n+    return incomingRef\n+\n def p4BranchesInGit(branchesAreInRemotes=True):\n     \"\"\"Find all the branches whose names start with \"p4/\", looking\n        in remotes or heads as specified by the argument.  Return\n@@ -3750,9 +3780,13 @@ class P4Sync(Command, P4UserMap):\n \n             # restrict to just this one, disabling detect-branches\n             if branch_arg_given:\n-                short = self.branch.split(\"/\")[-1]\n+                short = shortP4Ref(self.branch, self.importIntoRemotes)\n                 if short in branches:\n                     self.p4BranchesInGit = [ short ]\n+                elif self.branch.startswith('refs/') and \\\n+                        branchExists(self.branch) and \\\n+                        '[git-p4:' in extractLogMessageFromGitCommit(self.branch):\n+                    self.p4BranchesInGit = [ self.branch ]\n             else:\n                 self.p4BranchesInGit = branches.keys()\n \n@@ -3769,7 +3803,8 @@ class P4Sync(Command, P4UserMap):\n \n             p4Change = 0\n             for branch in self.p4BranchesInGit:\n-                logMsg =  extractLogMessageFromGitCommit(self.refPrefix + branch)\n+                logMsg = extractLogMessageFromGitCommit(fullP4Ref(branch,\n+                                                        self.importIntoRemotes))\n \n                 settings = extractSettingsGitLog(logMsg)\n \n@@ -3802,18 +3837,7 @@ class P4Sync(Command, P4UserMap):\n                 if not self.silent and not self.detectBranches:\n                     print(\"Performing incremental import into %s git branch\" % self.branch)\n \n-        # accept multiple ref name abbreviations:\n-        #    refs/foo/bar/branch -> use it exactly\n-        #    p4/branch -> prepend refs/remotes/ or refs/heads/\n-        #    branch -> prepend refs/remotes/p4/ or refs/heads/p4/\n-        if not self.branch.startswith(\"refs/\"):\n-            if self.importIntoRemotes:\n-                prepend = \"refs/remotes/\"\n-            else:\n-                prepend = \"refs/heads/\"\n-            if not self.branch.startswith(\"p4/\"):\n-                prepend += \"p4/\"\n-            self.branch = prepend + self.branch\n+        self.branch = fullP4Ref(self.branch, self.importIntoRemotes)\n \n         if len(args) == 0 and self.depotPaths:\n             if not self.silent:\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex 806005a793a..cc457765c2e 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -74,6 +74,91 @@ test_expect_success 'git p4 sync new branch' '\n \t)\n '\n \n+#\n+# Setup as before, and then explicitly sync imported branch, using a\n+# different ref format.\n+#\n+test_expect_success 'git p4 sync existing branch without changes' '\n+\ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit head &&\n+\t\tgit p4 sync --branch=depot //depot@all &&\n+\t\tgit p4 sync --branch=refs/remotes/p4/depot >out &&\n+\t\ttest_i18ngrep \"No changes to import!\" out\n+\t)\n+'\n+\n+#\n+# Same as before, relative branch name.\n+#\n+test_expect_success 'git p4 sync existing branch with relative name' '\n+\ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit head &&\n+\t\tgit p4 sync --branch=branch1 //depot@all &&\n+\t\tgit p4 sync --branch=p4/branch1 >out &&\n+\t\ttest_i18ngrep \"No changes to import!\" out\n+\t)\n+'\n+\n+#\n+# Same as before, with a nested branch path, referenced different ways.\n+#\n+test_expect_success 'git p4 sync existing branch with nested path' '\n+\ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit head &&\n+\t\tgit p4 sync --branch=p4/some/path //depot@all &&\n+\t\tgit p4 sync --branch=some/path >out &&\n+\t\ttest_i18ngrep \"No changes to import!\" out\n+\t)\n+'\n+\n+#\n+# Same as before, with a full ref path outside the p4/* namespace.\n+#\n+test_expect_success 'git p4 sync branch explicit ref without p4 in path' '\n+\ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit head &&\n+\t\tgit p4 sync --branch=refs/remotes/someremote/depot //depot@all &&\n+\t\tgit p4 sync --branch=refs/remotes/someremote/depot >out &&\n+\t\ttest_i18ngrep \"No changes to import!\" out\n+\t)\n+'\n+\n+test_expect_success 'git p4 sync nonexistent ref' '\n+\ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit head &&\n+\t\tgit p4 sync --branch=depot //depot@all &&\n+\t\ttest_must_fail git p4 sync --branch=depot2 2>errs &&\n+\t\ttest_i18ngrep \"Perhaps you never did\" errs\n+\t)\n+'\n+\n+test_expect_success 'git p4 sync existing non-p4-imported ref' '\n+\ttest_create_repo \"$git\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit head &&\n+\t\tgit p4 sync --branch=depot //depot@all &&\n+\t\ttest_must_fail git p4 sync --branch=refs/heads/master 2>errs &&\n+\t\ttest_i18ngrep \"Perhaps you never did\" errs\n+\t)\n+'\n+\n test_expect_success 'clone two dirs' '\n \t(\n \t\tcd \"$cli\" &&\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 50a6f8bad5c..759a14fa87c 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -129,6 +129,16 @@ test_expect_success 'import depot, branch detection' '\n \t)\n '\n \n+test_expect_success 'sync specific detected branch' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --dest=\"$git\" --detect-branches //depot@all &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit p4 sync --branch=depot/branch2 >out &&\n+\t\ttest_i18ngrep \"No changes to import!\" out\n+\t)\n+'\n+\n test_expect_success 'import depot, branch detection, branchList branch definition' '\n \ttest_when_finished cleanup_git &&\n \ttest_create_repo \"$git\" &&\n\nbase-commit: 805e0a68082a217f0112db9ee86a022227a9c81b\n-- \ngitgitgadget\n"}]}