{"thread":{"id":"29740","subject":"[PATCH 1/2] git-p4: set useClientSpec variable on initial clone","startedAt":"2012-02-26T01:06:23Z","lastAt":"2012-02-27T00:25:14Z","messageCount":5,"participants":["Pete Wyckoff","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"185444","messageId":"1330218385-22309-1-git-send-email-pw@padd.com","threadId":"29740","inReplyTo":null,"subject":"[PATCH 0/2] git-p4: fix submit regression with --use-client-spec","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-02-26T01:06:23Z","receivedAt":"2012-02-26T01:06:23Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"This pair of patches fixes a regression that happened with ecb7cf9\n(git-p4: rewrite view handling, 2012-01-02).  There are two factors that\naffect where files go in the client when submitting:  the cilent spec,\nand the depot path.  When the depot path was not exactly the root of\nthe client, submit fails.\n\nFix this by always using the true client root.  And also notice that\nsomebody has to tell the submit path that it should be looking at the\ncilent spec.  Save useClientSpec in a configuration variable if it\nwas specified as an option on the command line.\n\nJunio: can you put this on maint to go out with the next 1.9.x?\n\nPete Wyckoff (2):\n  git-p4: set useClientSpec variable on initial clone\n  git-p4: fix submit regression with clientSpec and subdir clone\n\n Documentation/git-p4.txt      |   10 ++-\n contrib/fast-import/git-p4    |   97 ++++++++++++++++---------\n t/t9809-git-p4-client-view.sh |  159 ++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 219 insertions(+), 47 deletions(-)\n\n-- \n1.7.9.219.g1d56c.dirty\n"},{"id":"185443","messageId":"1330218385-22309-2-git-send-email-pw@padd.com","threadId":"29740","inReplyTo":"1330218385-22309-1-git-send-email-pw@padd.com","subject":"[PATCH 1/2] git-p4: set useClientSpec variable on initial clone","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-02-26T01:06:24Z","receivedAt":"2012-02-26T01:06:24Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"If --use-client-spec was given, set the matching configuration\nvariable.  This is necessary to ensure that future submits\nwork properly.\n\nThe alternatives of requiring the user to set it, or providing\na command-line option on every submit, are error prone.\n\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n Documentation/git-p4.txt      |   10 +++++++---\n contrib/fast-import/git-p4    |   11 ++++++++++-\n t/t9809-git-p4-client-view.sh |   17 +++++++++++++++++\n 3 files changed, 34 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 78938b2..ed82790 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -303,9 +303,13 @@ CLIENT SPEC\n -----------\n The p4 client specification is maintained with the 'p4 client' command\n and contains among other fields, a View that specifies how the depot\n-is mapped into the client repository.  Git-p4 can consult the client\n-spec when given the '--use-client-spec' option or useClientSpec\n-variable.\n+is mapped into the client repository.  The 'clone' and 'sync' commands\n+can consult the client spec when given the '--use-client-spec' option or\n+when the useClientSpec variable is true.  After 'git p4 clone', the\n+useClientSpec variable is automatically set in the repository\n+configuration file.  This allows future 'git p4 submit' commands to\n+work properly; the submit command looks only at the variable and does\n+not have a command-line option.\n \n The full syntax for a p4 view is documented in 'p4 help views'.  Git-p4\n knows only a subset of the view syntax.  It understands multi-line\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 3e1aa27..94f0a12 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -1428,6 +1428,7 @@ class P4Sync(Command, P4UserMap):\n         self.p4BranchesInGit = []\n         self.cloneExclude = []\n         self.useClientSpec = False\n+        self.useClientSpec_from_options = False\n         self.clientSpecDirs = None\n \n         if gitConfig(\"git-p4.syncFromOrigin\") == \"false\":\n@@ -2136,7 +2137,11 @@ class P4Sync(Command, P4UserMap):\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 \n-        if not self.useClientSpec:\n+        # accept either the command-line option, or the configuration variable\n+        if self.useClientSpec:\n+            # will use this after clone to set the variable\n+            self.useClientSpec_from_options = True\n+        else:\n             if gitConfig(\"git-p4.useclientspec\", \"--bool\") == \"true\":\n                 self.useClientSpec = True\n         if self.useClientSpec:\n@@ -2455,6 +2460,10 @@ class P4Clone(P4Sync):\n             else:\n                 print \"Could not detect main branch. No checkout/master branch created.\"\n \n+        # auto-set this variable if invoked with --use-client-spec\n+        if self.useClientSpec_from_options:\n+            system(\"git config --bool git-p4.useclientspec true\")\n+\n         return True\n \n class P4Branches(Command):\ndiff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh\nindex c9471d5..25e01a4 100755\n--- a/t/t9809-git-p4-client-view.sh\n+++ b/t/t9809-git-p4-client-view.sh\n@@ -241,6 +241,23 @@ test_expect_success 'quotes on rhs only' '\n '\n \n #\n+# Submit tests\n+#\n+\n+# clone sets variable\n+test_expect_success 'clone --use-client-spec sets useClientSpec' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --use-client-spec --dest=\"$git\" //depot &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config --bool git-p4.useClientSpec >actual &&\n+\t\techo true >true &&\n+\t\ttest_cmp actual true\n+\t)\n+'\n+\n+#\n # Rename directories to test quoting in depot-side mappings\n # //depot\n #    - \"dir 1\"\n-- \n1.7.9.219.g1d56c.dirty\n"},{"id":"185445","messageId":"1330218385-22309-3-git-send-email-pw@padd.com","threadId":"29740","inReplyTo":"1330218385-22309-1-git-send-email-pw@padd.com","subject":"[PATCH 2/2] git-p4: fix submit regression with clientSpec and subdir clone","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-02-26T01:06:25Z","receivedAt":"2012-02-26T01:06:25Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"When the --use-client-spec is given to clone, and the clone\npath is a subset of the full tree as specified in the client,\nfuture submits will go to the wrong place.\n\nFactor out getClientSpec() so both clone/sync and submit can\nuse it.  Introduce getClientRoot() that is needed for the client\nspec case, and use it instead of p4Where().\n\nTest the five possible submit behaviors (add, modify, rename,\ncopy, delete).\n\nReported-by: Laurent Charrière <lcharriere@promptu.com>\nSigned-off-by: Pete Wyckoff <pw@padd.com>\n---\n contrib/fast-import/git-p4    |   86 ++++++++++++++++---------\n t/t9809-git-p4-client-view.sh |  142 +++++++++++++++++++++++++++++++++++++---\n 2 files changed, 185 insertions(+), 43 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 94f0a12..9ccc87b 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -555,6 +555,46 @@ def p4PathStartsWith(path, prefix):\n         return path.lower().startswith(prefix.lower())\n     return path.startswith(prefix)\n \n+def getClientSpec():\n+    \"\"\"Look at the p4 client spec, create a View() object that contains\n+       all the mappings, and return it.\"\"\"\n+\n+    specList = p4CmdList(\"client -o\")\n+    if len(specList) != 1:\n+        die('Output from \"client -o\" is %d lines, expecting 1' %\n+            len(specList))\n+\n+    # dictionary of all client parameters\n+    entry = specList[0]\n+\n+    # just the keys that start with \"View\"\n+    view_keys = [ k for k in entry.keys() if k.startswith(\"View\") ]\n+\n+    # hold this new View\n+    view = View()\n+\n+    # append the lines, in order, to the view\n+    for view_num in range(len(view_keys)):\n+        k = \"View%d\" % view_num\n+        if k not in view_keys:\n+            die(\"Expected view key %s missing\" % k)\n+        view.append(entry[k])\n+\n+    return view\n+\n+def getClientRoot():\n+    \"\"\"Grab the client directory.\"\"\"\n+\n+    output = p4CmdList(\"client -o\")\n+    if len(output) != 1:\n+        die('Output from \"client -o\" is %d lines, expecting 1' % len(output))\n+\n+    entry = output[0]\n+    if \"Root\" not in entry:\n+        die('Client has no \"Root\"')\n+\n+    return entry[\"Root\"]\n+\n class Command:\n     def __init__(self):\n         self.usage = \"usage: %prog [options]\"\n@@ -1119,11 +1159,20 @@ class P4Submit(Command, P4UserMap):\n             print \"Internal error: cannot locate perforce depot path from existing branches\"\n             sys.exit(128)\n \n-        self.clientPath = p4Where(self.depotPath)\n+        self.useClientSpec = False\n+        if gitConfig(\"git-p4.useclientspec\", \"--bool\") == \"true\":\n+            self.useClientSpec = True\n+        if self.useClientSpec:\n+            self.clientSpecDirs = getClientSpec()\n+\n+        if self.useClientSpec:\n+            # all files are relative to the client spec\n+            self.clientPath = getClientRoot()\n+        else:\n+            self.clientPath = p4Where(self.depotPath)\n \n-        if len(self.clientPath) == 0:\n-            print \"Error: Cannot locate perforce checkout of %s in client view\" % self.depotPath\n-            sys.exit(128)\n+        if self.clientPath == \"\":\n+            die(\"Error: Cannot locate perforce checkout of %s in client view\" % self.depotPath)\n \n         print \"Perforce checkout for depot path %s located at %s\" % (self.depotPath, self.clientPath)\n         self.oldWorkingDirectory = os.getcwd()\n@@ -2078,33 +2127,6 @@ class P4Sync(Command, P4UserMap):\n             print self.gitError.read()\n \n \n-    def getClientSpec(self):\n-        specList = p4CmdList(\"client -o\")\n-        if len(specList) != 1:\n-            die('Output from \"client -o\" is %d lines, expecting 1' %\n-                len(specList))\n-\n-        # dictionary of all client parameters\n-        entry = specList[0]\n-\n-        # just the keys that start with \"View\"\n-        view_keys = [ k for k in entry.keys() if k.startswith(\"View\") ]\n-\n-        # hold this new View\n-        view = View()\n-\n-        # append the lines, in order, to the view\n-        for view_num in range(len(view_keys)):\n-            k = \"View%d\" % view_num\n-            if k not in view_keys:\n-                die(\"Expected view key %s missing\" % k)\n-            view.append(entry[k])\n-\n-        self.clientSpecDirs = view\n-        if self.verbose:\n-            for i, m in enumerate(self.clientSpecDirs.mappings):\n-                    print \"clientSpecDirs %d: %s\" % (i, str(m))\n-\n     def run(self, args):\n         self.depotPaths = []\n         self.changeRange = \"\"\n@@ -2145,7 +2167,7 @@ class P4Sync(Command, P4UserMap):\n             if gitConfig(\"git-p4.useclientspec\", \"--bool\") == \"true\":\n                 self.useClientSpec = True\n         if self.useClientSpec:\n-            self.getClientSpec()\n+            self.clientSpecDirs = getClientSpec()\n \n         # TODO: should always look at previous commits,\n         # merge with previous imports, if possible.\ndiff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh\nindex 25e01a4..9642641 100755\n--- a/t/t9809-git-p4-client-view.sh\n+++ b/t/t9809-git-p4-client-view.sh\n@@ -71,20 +71,24 @@ git_verify() {\n #   - dir2\n #     - file21\n #     - file22\n+init_depot() {\n+\tfor d in 1 2 ; do\n+\t\tmkdir -p dir$d &&\n+\t\tfor f in 1 2 ; do\n+\t\t\techo dir$d/file$d$f >dir$d/file$d$f &&\n+\t\t\tp4 add dir$d/file$d$f &&\n+\t\t\tp4 submit -d \"dir$d/file$d$f\"\n+\t\tdone\n+\tdone &&\n+\tfind . -type f ! -name files >files &&\n+\tcheck_files_exist dir1/file11 dir1/file12 \\\n+\t\t\t  dir2/file21 dir2/file22\n+}\n+\n test_expect_success 'init depot' '\n \t(\n \t\tcd \"$cli\" &&\n-\t\tfor d in 1 2 ; do\n-\t\t\tmkdir -p dir$d &&\n-\t\t\tfor f in 1 2 ; do\n-\t\t\t\techo dir$d/file$d$f >dir$d/file$d$f &&\n-\t\t\t\tp4 add dir$d/file$d$f &&\n-\t\t\t\tp4 submit -d \"dir$d/file$d$f\"\n-\t\t\tdone\n-\t\tdone &&\n-\t\tfind . -type f ! -name files >files &&\n-\t\tcheck_files_exist dir1/file11 dir1/file12 \\\n-\t\t\t\t  dir2/file21 dir2/file22\n+\t\tinit_depot\n \t)\n '\n \n@@ -257,6 +261,122 @@ test_expect_success 'clone --use-client-spec sets useClientSpec' '\n \t)\n '\n \n+# clone just a subdir of the client spec\n+test_expect_success 'subdir clone' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\tfiles=\"dir1/file11 dir1/file12 dir2/file21 dir2/file22\" &&\n+\tclient_verify $files &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --use-client-spec --dest=\"$git\" //depot/dir1 &&\n+\tgit_verify dir1/file11 dir1/file12\n+'\n+\n+#\n+# submit back, see what happens:  five cases\n+#\n+test_expect_success 'subdir clone, submit modify' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --use-client-spec --dest=\"$git\" //depot/dir1 &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\techo line >>dir1/file12 &&\n+\t\tgit add dir1/file12 &&\n+\t\tgit commit -m dir1/file12 &&\n+\t\t\"$GITP4\" submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_file dir1/file12 &&\n+\t\ttest_line_count = 2 dir1/file12\n+\t)\n+'\n+\n+test_expect_success 'subdir clone, submit add' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --use-client-spec --dest=\"$git\" //depot/dir1 &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\techo file13 >dir1/file13 &&\n+\t\tgit add dir1/file13 &&\n+\t\tgit commit -m dir1/file13 &&\n+\t\t\"$GITP4\" submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_file dir1/file13\n+\t)\n+'\n+\n+test_expect_success 'subdir clone, submit delete' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --use-client-spec --dest=\"$git\" //depot/dir1 &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit rm dir1/file12 &&\n+\t\tgit commit -m \"delete dir1/file12\" &&\n+\t\t\"$GITP4\" submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing dir1/file12\n+\t)\n+'\n+\n+test_expect_success 'subdir clone, submit copy' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --use-client-spec --dest=\"$git\" //depot/dir1 &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.detectCopies true &&\n+\t\tcp dir1/file11 dir1/file11a &&\n+\t\tgit add dir1/file11a &&\n+\t\tgit commit -m \"copy to dir1/file11a\" &&\n+\t\t\"$GITP4\" submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_file dir1/file11a\n+\t)\n+'\n+\n+test_expect_success 'subdir clone, submit rename' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t\"$GITP4\" clone --use-client-spec --dest=\"$git\" //depot/dir1 &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\tgit config git-p4.detectRenames true &&\n+\t\tgit mv dir1/file13 dir1/file13a &&\n+\t\tgit commit -m \"rename dir1/file13 to dir1/file13a\" &&\n+\t\t\"$GITP4\" submit\n+\t) &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\ttest_path_is_missing dir1/file13 &&\n+\t\ttest_path_is_file dir1/file13a\n+\t)\n+'\n+\n+test_expect_success 'reinit depot' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 sync -f &&\n+\t\trm files &&\n+\t\tp4 delete */* &&\n+\t\tp4 submit -d \"delete all files\" &&\n+\t\tinit_depot\n+\t)\n+'\n+\n #\n # Rename directories to test quoting in depot-side mappings\n # //depot\n-- \n1.7.9.219.g1d56c.dirty\n"},{"id":"185480","messageId":"7vhaydcenz.fsf@alter.siamese.dyndns.org","threadId":"29740","inReplyTo":"1330218385-22309-1-git-send-email-pw@padd.com","subject":"Re: [PATCH 0/2] git-p4: fix submit regression with --use-client-spec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-27T00:18:08Z","receivedAt":"2012-02-27T00:18:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Wyckoff <pw@padd.com> writes:\n\n> This pair of patches fixes a regression that happened with ecb7cf9\n> (git-p4: rewrite view handling, 2012-01-02).  There are two factors that\n> affect where files go in the client when submitting:  the cilent spec,\n> and the depot path.  When the depot path was not exactly the root of\n> the client, submit fails.\n>\n> Fix this by always using the true client root.  And also notice that\n> somebody has to tell the submit path that it should be looking at the\n> cilent spec.  Save useClientSpec in a configuration variable if it\n> was specified as an option on the command line.\n>\n> Junio: can you put this on maint to go out with the next 1.9.x?\n\nSurely and thanks.\n\nYour \"p4: submit with wildcards\" seems to conflict with this change and\nmay need to be rebased, though.\n\n>\n> Pete Wyckoff (2):\n>   git-p4: set useClientSpec variable on initial clone\n>   git-p4: fix submit regression with clientSpec and subdir clone\n>\n>  Documentation/git-p4.txt      |   10 ++-\n>  contrib/fast-import/git-p4    |   97 ++++++++++++++++---------\n>  t/t9809-git-p4-client-view.sh |  159 ++++++++++++++++++++++++++++++++++++++---\n>  3 files changed, 219 insertions(+), 47 deletions(-)\n"},{"id":"185481","messageId":"20120227002514.GA31028@padd.com","threadId":"29740","inReplyTo":"7vhaydcenz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/2] git-p4: fix submit regression with --use-client-spec","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2012-02-27T00:25:14Z","receivedAt":"2012-02-27T00:25:14Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"gitster@pobox.com wrote on Sun, 26 Feb 2012 16:18 -0800:\n> Pete Wyckoff <pw@padd.com> writes:\n> \n> > This pair of patches fixes a regression that happened with ecb7cf9\n> > (git-p4: rewrite view handling, 2012-01-02).  There are two factors that\n> > affect where files go in the client when submitting:  the cilent spec,\n> > and the depot path.  When the depot path was not exactly the root of\n> > the client, submit fails.\n> >\n> > Fix this by always using the true client root.  And also notice that\n> > somebody has to tell the submit path that it should be looking at the\n> > cilent spec.  Save useClientSpec in a configuration variable if it\n> > was specified as an option on the command line.\n> >\n> > Junio: can you put this on maint to go out with the next 1.9.x?\n> \n> Surely and thanks.\n> \n> Your \"p4: submit with wildcards\" seems to conflict with this change and\n> may need to be rebased, though.\n\nAh, yes.  The conflicting chunks can be added in any order above\nclass P4Command.  I'll rebase and resend in a few days, after\nwaiting for review comments.\n\n\t\t-- Pete\n"}]}