{"thread":{"id":"32634","subject":"[PATCH 00/14] git p4 branch handling fixes","startedAt":"2013-01-15T00:46:54Z","lastAt":"2013-01-15T00:47:08Z","messageCount":15,"participants":["Pete Wyckoff"],"isPatch":true,"patchVersion":1,"patchTotal":14},"messages":[{"id":"206860","messageId":"1358210828-2369-1-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":null,"subject":"[PATCH 00/14] git p4 branch handling fixes","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:46:54Z","receivedAt":"2013-01-15T00:46:54Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"There are multiple oddities in how git-p4 treats multiple\np4 branches, as created with \"clone\" or \"sync\" and the\n'--branch' argument. Olivier reported some of these recently\nin http://thread.gmane.org/gmane.comp.version-control.git/212613\n\nThere are two observable behavior changes, but they\nare in the category of \"bug fixes\" in my opinion:\n\n - p4/HEAD symbolic ref is always created now; it used to\n be created only after the first sync operation after a clone\n\n - using clone --branch now checks out files; it used to\n complain that there was no p4/master ref\n\nPete Wyckoff (14):\n git p4: test sync/clone --branch behavior\n git p4: rearrange and simplify hasOrigin handling\n git p4: add comments to p4BranchesInGit\n git p4: inline listExistingP4GitBranches\n git p4: create p4/HEAD on initial clone\n git p4: verify expected refs in clone --bare test\n git p4: clone --branch should checkout master\n git p4 doc: fix branch detection example\n git p4: allow short ref names to --branch\n git p4: rearrange self.initialParent use\n git p4: fail gracefully on sync with no master branch\n git p4: fix sync --branch when no master branch\n git p4 test: keep P4CLIENT changes inside subshells\n git p4: fix submit when no master branch\n\n Documentation/git-p4.txt | 22 +++++--\n git-p4.py | 152 ++++++++++++++++++++++++++++++++--------------\n t/t9800-git-p4-basic.sh | 9 ++-\n t/t9806-git-p4-options.sh | 128 ++++++++++++++++++++++++++++++++++++--\n 4 files changed, 253 insertions(+), 58 deletions(-)\n\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206861","messageId":"1358210828-2369-2-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 01/14] git p4: test sync/clone --branch behavior","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:46:55Z","receivedAt":"2013-01-15T00:46:55Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Add failing tests to document behavior when there are multiple p4\nbranches, as created using the --branch option. In particular:\n\nUsing clone --branch populates the specified branch correctly, but\ndies with an error when trying to checkout master.\n\nCalling sync without a master branch dies with an error looking for\nmaster. When there are two or more branches, a sync does\nnothing due to branch detection code, but that is expected.\n\nUsing sync --branch to try to update just a particular branch\nupdates no branch, but appears to succeed.\n\nSigned-off-by: Pete Wyckoff \n---\n t/t9806-git-p4-options.sh | 53 +++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 49 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex fa40cc8..844aae0 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -27,14 +27,59 @@ test_expect_success 'clone no --git-dir' '\n \ttest_must_fail git p4 clone --git-dir=xx //depot\n '\n \n-test_expect_success 'clone --branch' '\n+test_expect_failure 'clone --branch should checkout master' '\n \tgit p4 clone --branch=refs/remotes/p4/sb --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \t(\n \t\tcd \"$git\" &&\n-\t\tgit ls-files >files &&\n-\t\ttest_line_count = 0 files &&\n-\t\ttest_path_is_file .git/refs/remotes/p4/sb\n+\t\tgit rev-parse refs/remotes/p4/sb >sb &&\n+\t\tgit rev-parse refs/heads/master >master &&\n+\t\ttest_cmp sb master &&\n+\t\tgit rev-parse HEAD >head &&\n+\t\ttest_cmp sb head\n+\t)\n+'\n+\n+test_expect_failure 'sync when branch is not called master should work' '\n+\tgit p4 clone --branch=refs/remotes/p4/sb --dest=\"$git\" //depot@2 &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit p4 sync &&\n+\t\tgit show -s --format=%s refs/remotes/p4/sb >show &&\n+\t\tgrep \"change 3\" show\n+\t)\n+'\n+\n+# engages --detect-branches code, which will do filename filtering so\n+# no sync to either b1 or b2\n+test_expect_success 'sync when two branches but no master should noop' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init &&\n+\t\tgit p4 sync --branch=refs/remotes/p4/b1 //depot@2 &&\n+\t\tgit p4 sync --branch=refs/remotes/p4/b2 //depot@2 &&\n+\t\tgit p4 sync &&\n+\t\tgit show -s --format=%s refs/remotes/p4/b1 >show &&\n+\t\tgrep \"Initial import\" show &&\n+\t\tgit show -s --format=%s refs/remotes/p4/b2 >show &&\n+\t\tgrep \"Initial import\" show\n+\t)\n+'\n+\n+test_expect_failure 'sync --branch updates specified branch' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init &&\n+\t\tgit p4 sync --branch=refs/remotes/p4/b1 //depot@2 &&\n+\t\tgit p4 sync --branch=refs/remotes/p4/b2 //depot@2 &&\n+\t\tgit p4 sync --branch=refs/remotes/p4/b2 &&\n+\t\tgit show -s --format=%s refs/remotes/p4/b1 >show &&\n+\t\tgrep \"Initial import\" show &&\n+\t\tgit show -s --format=%s refs/remotes/p4/b2 >show &&\n+\t\tgrep \"change 3\" show\n \t)\n '\n \n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206862","messageId":"1358210828-2369-3-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 02/14] git p4: rearrange and simplify hasOrigin handling","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:46:56Z","receivedAt":"2013-01-15T00:46:56Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"\nSigned-off-by: Pete Wyckoff \n---\n git-p4.py | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 69f1452..68f7458 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2754,23 +2754,23 @@ class P4Sync(Command, P4UserMap):\n self.changeRange = \"\"\n self.initialParent = \"\"\n self.previousDepotPaths = []\n+ self.hasOrigin = False\n \n # map from branch depot path to parent branch\n self.knownBranches = {}\n self.initialParents = {}\n- self.hasOrigin = originP4BranchesExist()\n- if not self.syncWithOrigin:\n- self.hasOrigin = False\n \n if self.importIntoRemotes:\n self.refPrefix = \"refs/remotes/p4/\"\n else:\n self.refPrefix = \"refs/heads/p4/\"\n \n- if self.syncWithOrigin and self.hasOrigin:\n- if not self.silent:\n- print \"Syncing with origin first by calling git fetch origin\"\n- system(\"git fetch origin\")\n+ if self.syncWithOrigin:\n+ self.hasOrigin = originP4BranchesExist()\n+ if self.hasOrigin:\n+ if not self.silent:\n+ print 'Syncing with origin first, using \"git fetch origin\"'\n+ system(\"git fetch origin\")\n \n if len(self.branch) == 0:\n self.branch = self.refPrefix + \"master\"\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206863","messageId":"1358210828-2369-4-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 03/14] git p4: add comments to p4BranchesInGit","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:46:57Z","receivedAt":"2013-01-15T00:46:57Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"\nSigned-off-by: Pete Wyckoff \n---\n git-p4.py | 25 +++++++++++++++++--------\n 1 file changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 68f7458..03680b0 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -553,27 +553,36 @@ def gitConfigList(key):\n _gitConfig[key] = read_pipe(\"git config --get-all %s\" % key, ignore_error=True).strip().split(os.linesep)\n return _gitConfig[key]\n \n-def p4BranchesInGit(branchesAreInRemotes = True):\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+ a dictionary of { branch: revision } for each one found.\n+ The branch names are the short names, without any\n+ \"p4/\" prefix.\"\"\"\n+\n branches = {}\n \n cmdline = \"git rev-parse --symbolic \"\n if branchesAreInRemotes:\n- cmdline += \" --remotes\"\n+ cmdline += \"--remotes\"\n else:\n- cmdline += \" --branches\"\n+ cmdline += \"--branches\"\n \n for line in read_pipe_lines(cmdline):\n line = line.strip()\n \n- ## only import to p4/\n- if not line.startswith('p4/') or line == \"p4/HEAD\":\n+ # only import to p4/\n+ if not line.startswith('p4/'):\n+ continue\n+ # special symbolic ref to p4/master\n+ if line == \"p4/HEAD\":\n continue\n- branch = line\n \n- # strip off p4\n- branch = re.sub (\"^p4/\", \"\", line)\n+ # strip off p4/ prefix\n+ branch = line[len(\"p4/\"):]\n \n branches[branch] = parseRevision(line)\n+\n return branches\n \n def findUpstreamBranchPoint(head = \"HEAD\"):\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206864","messageId":"1358210828-2369-5-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 04/14] git p4: inline listExistingP4GitBranches","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:46:58Z","receivedAt":"2013-01-15T00:46:58Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"It is four lines of code used in only one place. Simplify by\nincluding it where it is used.\n\nSigned-off-by: Pete Wyckoff \n---\n git-p4.py | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 03680b0..8814049 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2518,13 +2518,6 @@ class P4Sync(Command, P4UserMap):\n branch = branch[len(self.projectName):]\n self.knownBranches[branch] = branch\n \n- def listExistingP4GitBranches(self):\n- # branches holds mapping from name to commit\n- branches = p4BranchesInGit(self.importIntoRemotes)\n- self.p4BranchesInGit = branches.keys()\n- for branch in branches.keys():\n- self.initialParents[self.refPrefix + branch] = branches[branch]\n-\n def updateOptionDict(self, d):\n option_keys = {}\n if self.keepRepoPath:\n@@ -2805,7 +2798,12 @@ class P4Sync(Command, P4UserMap):\n if args == []:\n if self.hasOrigin:\n createOrUpdateBranchesFromOrigin(self.refPrefix, self.silent)\n- self.listExistingP4GitBranches()\n+\n+ # branches holds mapping from branch name to sha1\n+ branches = p4BranchesInGit(self.importIntoRemotes)\n+ self.p4BranchesInGit = branches.keys()\n+ for branch in branches.keys():\n+ self.initialParents[self.refPrefix + branch] = branches[branch]\n \n if len(self.p4BranchesInGit) > 1:\n if not self.silent:\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206865","messageId":"1358210828-2369-6-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 05/14] git p4: create p4/HEAD on initial clone","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:46:59Z","receivedAt":"2013-01-15T00:46:59Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"There is code to create a symbolic reference from p4/HEAD to\np4/master. This allows saying \"git show p4\" as a shortcut\nto \"git show p4/master\", for example.\n\nBut this reference was only created on the second \"git p4 sync\"\n(or first sync after a clone). Make it work on the initial\nclone or sync.\n\nSigned-off-by: Pete Wyckoff \n---\n git-p4.py | 12 ++++++++----\n t/t9806-git-p4-options.sh | 23 +++++++++++++++++++++++\n 2 files changed, 31 insertions(+), 4 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 8814049..537eac6 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2778,10 +2778,7 @@ class P4Sync(Command, P4UserMap):\n self.branch = self.refPrefix + \"master\"\n if gitBranchExists(\"refs/heads/p4\") and self.importIntoRemotes:\n system(\"git update-ref %s refs/heads/p4\" % self.branch)\n- system(\"git branch -D p4\");\n- # create it /after/ importing, when master exists\n- if not gitBranchExists(self.refPrefix + \"HEAD\") and self.importIntoRemotes and gitBranchExists(self.branch):\n- system(\"git symbolic-ref %sHEAD %s\" % (self.refPrefix, self.branch))\n+ system(\"git branch -D p4\")\n \n # accept either the command-line option, or the configuration variable\n if self.useClientSpec:\n@@ -3013,6 +3010,13 @@ class P4Sync(Command, P4UserMap):\n read_pipe(\"git update-ref -d %s\" % branch)\n os.rmdir(os.path.join(os.environ.get(\"GIT_DIR\", \".git\"), self.tempBranchLocation))\n \n+ # Create a symbolic ref p4/HEAD pointing to p4/ to allow\n+ # a convenient shortcut refname \"p4\".\n+ if self.importIntoRemotes:\n+ head_ref = self.refPrefix + \"HEAD\"\n+ if not gitBranchExists(head_ref) and gitBranchExists(self.branch):\n+ system([\"git\", \"symbolic-ref\", head_ref, self.branch])\n+\n return True\n \n class P4Rebase(Command):\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex 844aae0..4900aef 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -83,6 +83,29 @@ test_expect_failure 'sync --branch updates specified branch' '\n \t)\n '\n \n+# allows using the refname \"p4\" as a short name for p4/master\n+test_expect_success 'clone creates HEAD symbolic reference' '\n+\tgit p4 clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit rev-parse --verify refs/remotes/p4/master >master &&\n+\t\tgit rev-parse --verify p4 >p4 &&\n+\t\ttest_cmp master p4\n+\t)\n+'\n+\n+test_expect_success 'clone --branch creates HEAD symbolic reference' '\n+\tgit p4 clone --branch=refs/remotes/p4/sb --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit rev-parse --verify refs/remotes/p4/sb >sb &&\n+\t\tgit rev-parse --verify p4 >p4 &&\n+\t\ttest_cmp sb p4\n+\t)\n+'\n+\n test_expect_success 'clone --changesfile' '\n \ttest_when_finished \"rm cf\" &&\n \tprintf \"1\\n3\\n\" >cf &&\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206866","messageId":"1358210828-2369-7-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 06/14] git p4: verify expected refs in clone --bare test","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:00Z","receivedAt":"2013-01-15T00:47:00Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Make sure that the standard branches are created as expected.\n\nSigned-off-by: Pete Wyckoff \n---\n t/t9800-git-p4-basic.sh | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh\nindex 8c59796..166e752 100755\n--- a/t/t9800-git-p4-basic.sh\n+++ b/t/t9800-git-p4-basic.sh\n@@ -160,9 +160,12 @@ test_expect_success 'clone --bare should make a bare repository' '\n \ttest_when_finished cleanup_git &&\n \t(\n \t\tcd \"$git\" &&\n-\t\ttest ! -d .git &&\n-\t\tbare=`git config --get core.bare` &&\n-\t\ttest \"$bare\" = true\n+\t\ttest_path_is_missing .git &&\n+\t\tgit config --get --bool core.bare true &&\n+\t\tgit rev-parse --verify refs/remotes/p4/master &&\n+\t\tgit rev-parse --verify refs/remotes/p4/HEAD &&\n+\t\tgit rev-parse --verify refs/heads/master &&\n+\t\tgit rev-parse --verify HEAD\n \t)\n '\n \n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206867","messageId":"1358210828-2369-8-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 07/14] git p4: clone --branch should checkout master","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:01Z","receivedAt":"2013-01-15T00:47:01Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"When using the --branch argument to \"git p4 clone\", one\nmight specify a destination for p4 changes different from\nthe default refs/remotes/p4/master. Both cases should\ncreate a master branch and checkout files.\n\nSigned-off-by: Pete Wyckoff \n---\n Documentation/git-p4.txt | 3 +--\n git-p4.py | 20 +++++++++-----------\n t/t9806-git-p4-options.sh | 2 +-\n 3 files changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex beff622..2623bee 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -174,8 +174,7 @@ subsequent 'sync' operations.\n --branch ::\n \tImport changes into given branch. If the branch starts with\n \t'refs/', it will be used as is, otherwise the path 'refs/heads/'\n-\twill be prepended. The default branch is 'master'. If used\n-\twith an initial clone, no HEAD will be checked out.\n+\twill be prepended. The default branch is 'p4/master'.\n +\n This example imports a new remote \"p4/proj2\" into an existing\n git repository:\ndiff --git a/git-p4.py b/git-p4.py\nindex 537eac6..d92f00c 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3124,17 +3124,15 @@ class P4Clone(P4Sync):\n \n if not P4Sync.run(self, depotPaths):\n return False\n- if self.branch != \"master\":\n- if self.importIntoRemotes:\n- masterbranch = \"refs/remotes/p4/master\"\n- else:\n- masterbranch = \"refs/heads/p4/master\"\n- if gitBranchExists(masterbranch):\n- system(\"git branch master %s\" % masterbranch)\n- if not self.cloneBare:\n- system(\"git checkout -f\")\n- else:\n- print \"Could not detect main branch. No checkout/master branch created.\"\n+\n+ # create a master branch and check out a work tree\n+ if gitBranchExists(self.branch):\n+ system([ \"git\", \"branch\", \"master\", self.branch ])\n+ if not self.cloneBare:\n+ system([ \"git\", \"checkout\", \"-f\" ])\n+ else:\n+ print 'Not checking out any branch, use ' \\\n+ '\"git checkout -q -b master \"'\n \n # auto-set this variable if invoked with --use-client-spec\n if self.useClientSpec_from_options:\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex 4900aef..2ad3a3e 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -27,7 +27,7 @@ test_expect_success 'clone no --git-dir' '\n \ttest_must_fail git p4 clone --git-dir=xx //depot\n '\n \n-test_expect_failure 'clone --branch should checkout master' '\n+test_expect_success 'clone --branch should checkout master' '\n \tgit p4 clone --branch=refs/remotes/p4/sb --dest=\"$git\" //depot &&\n \ttest_when_finished cleanup_git &&\n \t(\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206868","messageId":"1358210828-2369-9-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 08/14] git p4 doc: fix branch detection example","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:02Z","receivedAt":"2013-01-15T00:47:02Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Make sure that the example on how to use git-p4.branchList\nworks if typed directly. In particular, it does not make sense\nto set a config variable until the git repository has been\ninitialized.\n\nReported-by: Olivier Delalleau \nSigned-off-by: Pete Wyckoff \n---\n Documentation/git-p4.txt | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 2623bee..7c5230e 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -393,8 +393,10 @@ the path elements in the p4 repository. The example above relied on the\n presence of the p4 branch. Without p4 branches, the same result will\n occur with:\n ----\n+git init depot\n+cd depot\n git config git-p4.branchList main:branch1\n-git p4 clone --detect-branches //depot@all\n+git p4 clone --detect-branches //depot@all .\n ----\n \n \n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206869","messageId":"1358210828-2369-10-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 09/14] git p4: allow short ref names to --branch","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:03Z","receivedAt":"2013-01-15T00:47:03Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"For a clone or sync, --branch says where the newly imported\nbranch should go, or which existing branch to sync up. It\ntakes an argument, which is currently either something that\nstarts with \"refs/\", or if not, \"refs/heads/p4\" is prepended.\n\nPutting it in heads seems like a bad default; these should\ngo in remotes/p4/ in most situations. Make that the new default,\nand be more liberal in the form of the branch name.\n\nSigned-off-by: Pete Wyckoff \n---\n Documentation/git-p4.txt | 7 +++++--\n git-p4.py | 12 +++++++++++-\n t/t9806-git-p4-options.sh | 21 +++++++++++++++++++++\n 3 files changed, 37 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 7c5230e..7bd5c29 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -173,8 +173,11 @@ subsequent 'sync' operations.\n \n --branch ::\n \tImport changes into given branch. If the branch starts with\n-\t'refs/', it will be used as is, otherwise the path 'refs/heads/'\n-\twill be prepended. The default branch is 'p4/master'.\n+\t'refs/', it will be used as is. Otherwise if it does not start\n+\twith 'p4/', that prefix is added. The branch is assumed to\n+\tname a remote tracking, but this can be modified using\n+\t'--import-local', or by giving a full ref name. The default\n+\tbranch is 'master'.\n +\n This example imports a new remote \"p4/proj2\" into an existing\n git repository:\ndiff --git a/git-p4.py b/git-p4.py\nindex d92f00c..5dcb527 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2847,8 +2847,18 @@ 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- self.branch = \"refs/heads/\" + self.branch\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 \n if len(args) == 0 and self.depotPaths:\n if not self.silent:\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex 2ad3a3e..c0d4433 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -51,6 +51,27 @@ test_expect_failure 'sync when branch is not called master should work' '\n \t)\n '\n \n+test_expect_success 'sync --branch builds the full ref name correctly' '\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init &&\n+\n+\t\tgit p4 sync --branch=b1 //depot &&\n+\t\tgit rev-parse --verify refs/remotes/p4/b1 &&\n+\t\tgit p4 sync --branch=p4/b2 //depot &&\n+\t\tgit rev-parse --verify refs/remotes/p4/b2 &&\n+\n+\t\tgit p4 sync --import-local --branch=h1 //depot &&\n+\t\tgit rev-parse --verify refs/heads/p4/h1 &&\n+\t\tgit p4 sync --import-local --branch=p4/h2 //depot &&\n+\t\tgit rev-parse --verify refs/heads/p4/h2 &&\n+\n+\t\tgit p4 sync --branch=refs/stuff //depot &&\n+\t\tgit rev-parse --verify refs/stuff\n+\t)\n+'\n+\n # engages --detect-branches code, which will do filename filtering so\n # no sync to either b1 or b2\n test_expect_success 'sync when two branches but no master should noop' '\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206870","messageId":"1358210828-2369-11-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 10/14] git p4: rearrange self.initialParent use","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:04Z","receivedAt":"2013-01-15T00:47:04Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This was set in a couple of places, both of which were very\nfar away from its use. Move it a bit closer to importChanges(),\nand add some comments.\n\nSigned-off-by: Pete Wyckoff \n---\n git-p4.py | 12 +++++++++---\n 1 file changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 5dcb527..9b07ddd 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2689,6 +2689,7 @@ class P4Sync(Command, P4UserMap):\n files = self.extractFilesFromCommit(description)\n self.commit(description, files, self.branch,\n self.initialParent)\n+ # only needed once, to connect to the previous commit\n self.initialParent = \"\"\n except IOError:\n print self.gitError.read()\n@@ -2754,7 +2755,6 @@ class P4Sync(Command, P4UserMap):\n def run(self, args):\n self.depotPaths = []\n self.changeRange = \"\"\n- self.initialParent = \"\"\n self.previousDepotPaths = []\n self.hasOrigin = False\n \n@@ -2842,8 +2842,6 @@ class P4Sync(Command, P4UserMap):\n if p4Change > 0:\n self.depotPaths = sorted(self.previousDepotPaths)\n self.changeRange = \"@%s,#head\" % p4Change\n- if not self.detectBranches:\n- self.initialParent = parseRevision(self.branch)\n if not self.silent and not self.detectBranches:\n print \"Performing incremental import into %s git branch\" % self.branch\n \n@@ -2988,6 +2986,14 @@ class P4Sync(Command, P4UserMap):\n \n self.updatedBranches = set()\n \n+ if not self.detectBranches:\n+ if args:\n+ # start a new branch\n+ self.initialParent = \"\"\n+ else:\n+ # build on a previous revision\n+ self.initialParent = parseRevision(self.branch)\n+\n self.importChanges(changes)\n \n if not self.silent:\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206871","messageId":"1358210828-2369-12-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 11/14] git p4: fail gracefully on sync with no master branch","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:05Z","receivedAt":"2013-01-15T00:47:05Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"If --branch was used to build a repository with no\nrefs/remotes/p4/master, future syncs will not know\nwhich branch to sync. Notice this situation and\nprint a helpful error message.\n\nSigned-off-by: Pete Wyckoff \n---\n git-p4.py | 29 +++++++++++++++++++++++++++--\n t/t9806-git-p4-options.sh | 9 ++++-----\n 2 files changed, 31 insertions(+), 7 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 9b07ddd..390d3f1 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -585,6 +585,17 @@ def p4BranchesInGit(branchesAreInRemotes=True):\n \n return branches\n \n+def branch_exists(branch):\n+ \"\"\"Make sure that the given ref name really exists.\"\"\"\n+\n+ cmd = [ \"git\", \"rev-parse\", \"--symbolic\", \"--verify\", branch ]\n+ p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)\n+ out, _ = p.communicate()\n+ if p.returncode:\n+ return False\n+ # expect exactly one line of output: the branch name\n+ return out.rstrip() == branch\n+\n def findUpstreamBranchPoint(head = \"HEAD\"):\n branches = p4BranchesInGit()\n # map from depot-path to branch name\n@@ -2774,6 +2785,7 @@ class P4Sync(Command, P4UserMap):\n print 'Syncing with origin first, using \"git fetch origin\"'\n system(\"git fetch origin\")\n \n+ branch_arg_given = bool(self.branch)\n if len(self.branch) == 0:\n self.branch = self.refPrefix + \"master\"\n if gitBranchExists(\"refs/heads/p4\") and self.importIntoRemotes:\n@@ -2967,8 +2979,21 @@ class P4Sync(Command, P4UserMap):\n else:\n # catch \"git p4 sync\" with no new branches, in a repo that\n # does not have any existing p4 branches\n- if len(args) == 0 and not self.p4BranchesInGit:\n- die(\"No remote p4 branches. Perhaps you never did \\\"git p4 clone\\\" in here.\");\n+ if len(args) == 0:\n+ if not self.p4BranchesInGit:\n+ die(\"No remote p4 branches. Perhaps you never did \\\"git p4 clone\\\" in here.\")\n+\n+ # The default branch is master, unless --branch is used to\n+ # specify something else. Make sure it exists, or complain\n+ # nicely about how to use --branch.\n+ if not self.detectBranches:\n+ if not branch_exists(self.branch):\n+ if branch_arg_given:\n+ die(\"Error: branch %s does not exist.\" % self.branch)\n+ else:\n+ die(\"Error: no branch %s; perhaps specify one with --branch.\" %\n+ self.branch)\n+\n if self.verbose:\n print \"Getting p4 changes for %s...%s\" % (', '.join(self.depotPaths),\n self.changeRange)\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex c0d4433..a51f122 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -40,14 +40,13 @@ test_expect_success 'clone --branch should checkout master' '\n \t)\n '\n \n-test_expect_failure 'sync when branch is not called master should work' '\n-\tgit p4 clone --branch=refs/remotes/p4/sb --dest=\"$git\" //depot@2 &&\n+test_expect_success 'sync when no master branch prints a nice error' '\n \ttest_when_finished cleanup_git &&\n+\tgit p4 clone --branch=refs/remotes/p4/sb --dest=\"$git\" //depot@2 &&\n \t(\n \t\tcd \"$git\" &&\n-\t\tgit p4 sync &&\n-\t\tgit show -s --format=%s refs/remotes/p4/sb >show &&\n-\t\tgrep \"change 3\" show\n+\t\ttest_must_fail git p4 sync 2>err &&\n+\t\tgrep \"Error: no branch refs/remotes/p4/master\" err\n \t)\n '\n \n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206872","messageId":"1358210828-2369-13-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 12/14] git p4: fix sync --branch when no master branch","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:06Z","receivedAt":"2013-01-15T00:47:06Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"It is legal to sync a branch with a different name than\nrefs/remotes/p4/master, and to do so even when master does\nnot exist.\n\nSigned-off-by: Pete Wyckoff \n---\n Documentation/git-p4.txt | 5 +++++\n git-p4.py | 14 +++++++++++---\n t/t9806-git-p4-options.sh | 8 ++++----\n 3 files changed, 20 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 7bd5c29..e79d046 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -112,6 +112,11 @@ will be fetched and consulted first during a 'git p4 sync'. Since\n importing directly from p4 is considerably slower than pulling changes\n from a git remote, this can be useful in a multi-developer environment.\n \n+If there are multiple branches, doing 'git p4 sync' will automatically\n+use the \"BRANCH DETECTION\" algorithm to try to partition new changes\n+into the right branch. This can be overridden with the '--branch'\n+option to specify just a single branch to update.\n+\n \n Rebase\n ~~~~~~\ndiff --git a/git-p4.py b/git-p4.py\nindex 390d3f1..77bde59 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2810,14 +2810,22 @@ class P4Sync(Command, P4UserMap):\n \n # branches holds mapping from branch name to sha1\n branches = p4BranchesInGit(self.importIntoRemotes)\n- self.p4BranchesInGit = branches.keys()\n- for branch in branches.keys():\n- self.initialParents[self.refPrefix + branch] = branches[branch]\n+\n+ # restrict to just this one, disabling detect-branches\n+ if branch_arg_given:\n+ short = self.branch.split(\"/\")[-1]\n+ if short in branches:\n+ self.p4BranchesInGit = [ short ]\n+ else:\n+ self.p4BranchesInGit = branches.keys()\n \n if len(self.p4BranchesInGit) > 1:\n if not self.silent:\n print \"Importing from/into multiple branches\"\n self.detectBranches = True\n+ for branch in branches.keys():\n+ self.initialParents[self.refPrefix + branch] = \\\n+ branches[branch]\n \n if self.verbose:\n print \"branches: %s\" % self.p4BranchesInGit\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex a51f122..3b0000f 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -88,14 +88,14 @@ test_expect_success 'sync when two branches but no master should noop' '\n \t)\n '\n \n-test_expect_failure 'sync --branch updates specified branch' '\n+test_expect_success 'sync --branch updates specific branch, no detection' '\n \ttest_when_finished cleanup_git &&\n \t(\n \t\tcd \"$git\" &&\n \t\tgit init &&\n-\t\tgit p4 sync --branch=refs/remotes/p4/b1 //depot@2 &&\n-\t\tgit p4 sync --branch=refs/remotes/p4/b2 //depot@2 &&\n-\t\tgit p4 sync --branch=refs/remotes/p4/b2 &&\n+\t\tgit p4 sync --branch=b1 //depot@2 &&\n+\t\tgit p4 sync --branch=b2 //depot@2 &&\n+\t\tgit p4 sync --branch=b2 &&\n \t\tgit show -s --format=%s refs/remotes/p4/b1 >show &&\n \t\tgrep \"Initial import\" show &&\n \t\tgit show -s --format=%s refs/remotes/p4/b2 >show &&\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206873","messageId":"1358210828-2369-14-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 13/14] git p4 test: keep P4CLIENT changes inside subshells","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:07Z","receivedAt":"2013-01-15T00:47:07Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Tests assume that this is set to something valid. Make sure\nthat the 'clone --use-client-spec' does not leak its changes\nout into the rest of the tests.\n\nSigned-off-by: Pete Wyckoff \n---\n t/t9806-git-p4-options.sh | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex 3b0000f..8d914a5 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -226,9 +226,11 @@ test_expect_success 'clone --use-client-spec' '\n \t\tView: //depot/sub/... //client2/bus/...\n \t\tEOF\n \t) &&\n-\tP4CLIENT=client2 &&\n \ttest_when_finished cleanup_git &&\n-\tgit p4 clone --dest=\"$git\" --use-client-spec //depot/... &&\n+\t(\n+\t\tP4CLIENT=client2 &&\n+\t\tgit p4 clone --dest=\"$git\" --use-client-spec //depot/...\n+\t) &&\n \t(\n \t\tcd \"$git\" &&\n \t\ttest_path_is_file bus/dir/f4 &&\n@@ -241,6 +243,7 @@ test_expect_success 'clone --use-client-spec' '\n \t\tcd \"$git\" &&\n \t\tgit init &&\n \t\tgit config git-p4.useClientSpec true &&\n+\t\tP4CLIENT=client2 &&\n \t\tgit p4 sync //depot/... &&\n \t\tgit checkout -b master p4/master &&\n \t\ttest_path_is_file bus/dir/f4 &&\n-- \n1.8.1.350.gdbf6fd0\n"},{"id":"206874","messageId":"1358210828-2369-15-git-send-email-pw@padd.com","threadId":"32634","inReplyTo":"1358210828-2369-1-git-send-email-pw@padd.com","subject":"[PATCH 14/14] git p4: fix submit when no master branch","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2013-01-15T00:47:08Z","receivedAt":"2013-01-15T00:47:08Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"It finds its upstream and applies the commit properly, but\nthe sync step will fail unless it is told which branch to\nwork on.\n\nSigned-off-by: Pete Wyckoff \n---\n Documentation/git-p4.txt | 5 +++++\n git-p4.py | 6 +++++-\n t/t9806-git-p4-options.sh | 25 +++++++++++++++++++++++++\n 3 files changed, 35 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex e79d046..f70ef9d 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -294,6 +294,11 @@ These options can be used to modify 'git p4 submit' behavior.\n \tto bypass the prompt, causing conflicting commits to be automatically\n \tskipped, or to quit trying to apply commits, without prompting.\n \n+--branch ::\n+\tAfter submitting, sync this named branch instead of the default\n+\tp4/master. See the \"Sync options\" section above for more\n+\tinformation.\n+\n Rebase options\n ~~~~~~~~~~~~~~\n These options can be used to modify 'git p4 rebase' behavior.\ndiff --git a/git-p4.py b/git-p4.py\nindex 77bde59..2da5649 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -927,7 +927,8 @@ class P4Submit(Command, P4UserMap):\n optparse.make_option(\"--dry-run\", \"-n\", dest=\"dry_run\", action=\"store_true\"),\n optparse.make_option(\"--prepare-p4-only\", dest=\"prepare_p4_only\", action=\"store_true\"),\n optparse.make_option(\"--conflict\", dest=\"conflict_behavior\",\n- choices=self.conflict_behavior_choices)\n+ choices=self.conflict_behavior_choices),\n+ optparse.make_option(\"--branch\", dest=\"branch\"),\n ]\n self.description = \"Submit changes from git to the perforce depot.\"\n self.usage += \" [name of git branch to submit into perforce depot]\"\n@@ -940,6 +941,7 @@ class P4Submit(Command, P4UserMap):\n self.isWindows = (platform.system() == \"Windows\")\n self.exportLabels = False\n self.p4HasMoveCommand = p4_has_move_command()\n+ self.branch = None\n \n def check(self):\n if len(p4CmdList(\"opened ...\")) > 0:\n@@ -1676,6 +1678,8 @@ class P4Submit(Command, P4UserMap):\n print \"All commits applied!\"\n \n sync = P4Sync()\n+ if self.branch:\n+ sync.branch = self.branch\n sync.run([])\n \n rebase = P4Rebase()\ndiff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh\nindex 8d914a5..4f077ee 100755\n--- a/t/t9806-git-p4-options.sh\n+++ b/t/t9806-git-p4-options.sh\n@@ -251,6 +251,31 @@ test_expect_success 'clone --use-client-spec' '\n \t)\n '\n \n+test_expect_success 'submit works with no p4/master' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --branch=b1 //depot@1,2 --destination=\"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\ttest_commit submit-1-branch &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit --branch=b1\n+\t)\n+'\n+\n+# The sync/rebase part post-submit will engage detect-branches\n+# machinery which will not do anything in this particular test.\n+test_expect_success 'submit works with two branches' '\n+\ttest_when_finished cleanup_git &&\n+\tgit p4 clone --branch=b1 //depot@1,2 --destination=\"$git\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit p4 sync --branch=b2 //depot@1,3 &&\n+\t\ttest_commit submit-2-branches &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit p4 submit\n+\t)\n+'\n+\n test_expect_success 'kill p4d' '\n \tkill_p4d\n '\n-- \n1.8.1.350.gdbf6fd0\n"}]}