{"thread":{"id":"40958","subject":"[PATCH v2] git-p4: Add option to ignore empty commits","startedAt":"2015-12-08T09:36:21Z","lastAt":"2015-12-10T01:05:55Z","messageCount":3,"participants":["larsxschneider@gmail.com","Luke Diamand"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"274168","messageId":"1449567382-63713-1-git-send-email-larsxschneider@gmail.com","threadId":"40958","inReplyTo":null,"subject":"[PATCH v2] git-p4: Add option to ignore empty commits","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-08T09:36:21Z","receivedAt":"2015-12-08T09:36:21Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\ndiff to v1:\n* change the default behavior and replace \"ignore empty commits\" option\n  with \"keep empty commits\" (thanks Junio/Luke)\n  --> I kept the original topic name in the subject letter to ease finding\n      v1, ok?\n* add 'an' to fix grammar in commit message (thanks Luke)\n* remove quotes around 'true' in docs to avoid confusion (thanks Luke)\n* add deletion test (thanks Junio/Luke)\n* use print statement to avoid ugly \\n (thanks Luke)\n* use positional argument specifiers in Python format string syntax to\n  support Python 2.6 (thanks Pete Harlan)\n\nThanks,\nLars\n\nLars Schneider (1):\n  git-p4: Add option to keep empty commits\n\n Documentation/git-p4.txt             |   4 ++\n git-p4.py                            |  44 +++++++-----\n t/t9826-git-p4-keep-empty-commits.sh | 134 +++++++++++++++++++++++++++++++++++\n 3 files changed, 165 insertions(+), 17 deletions(-)\n create mode 100755 t/t9826-git-p4-keep-empty-commits.sh\n\n--\n2.5.1\n"},{"id":"274169","messageId":"1449567382-63713-2-git-send-email-larsxschneider@gmail.com","threadId":"40958","inReplyTo":"1449567382-63713-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v2] git-p4: Add option to keep empty commits","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-12-08T09:36:22Z","receivedAt":"2015-12-08T09:36:22Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nA changelist that contains only excluded files due to a client spec was\nimported as an empty commit. Fix that issue by ignoring these commits.\nAdd option \"git-p4.keepEmptyCommits\" to make the previous behavior\navailable.\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\nHelped-by: Pete Harlan\n---\n Documentation/git-p4.txt             |   4 ++\n git-p4.py                            |  44 +++++++-----\n t/t9826-git-p4-keep-empty-commits.sh | 134 +++++++++++++++++++++++++++++++++++\n 3 files changed, 165 insertions(+), 17 deletions(-)\n create mode 100755 t/t9826-git-p4-keep-empty-commits.sh\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 82aa5d6..b3e768e 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -510,6 +510,10 @@ git-p4.useClientSpec::\n \toption '--use-client-spec'.  See the \"CLIENT SPEC\" section above.\n \tThis variable is a boolean, not the name of a p4 client.\n \n+git-p4.keepEmptyCommits::\n+\tA changelist that contains only excluded files will be imported\n+\tas an empty commit if this boolean option is set to true.\n+\n Submit variables\n ~~~~~~~~~~~~~~~~\n git-p4.detectRenames::\ndiff --git a/git-p4.py b/git-p4.py\nindex 0093fa3..62c26bc 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):\n         filesToDelete = []\n \n         for f in files:\n-            # if using a client spec, only add the files that have\n-            # a path in the client\n-            if self.clientSpecDirs:\n-                if self.clientSpecDirs.map_in_client(f['path']) == \"\":\n-                    continue\n-\n             filesForCommit.append(f)\n             if f['action'] in self.delete_actions:\n                 filesToDelete.append(f)\n@@ -2361,25 +2355,41 @@ class P4Sync(Command, P4UserMap):\n         gitStream.write(description)\n         gitStream.write(\"\\n\")\n \n+    def inClientSpec(self, path):\n+        if not self.clientSpecDirs:\n+            return True\n+        inClientSpec = self.clientSpecDirs.map_in_client(path)\n+        if not inClientSpec and self.verbose:\n+            print('Ignoring file outside of client spec: {0}'.format(path))\n+        return inClientSpec\n+\n+    def hasBranchPrefix(self, path):\n+        if not self.branchPrefixes:\n+            return True\n+        hasPrefix = [p for p in self.branchPrefixes\n+                        if p4PathStartsWith(path, p)]\n+        if hasPrefix and self.verbose:\n+            print('Ignoring file outside of prefix: {0}'.format(path))\n+        return hasPrefix\n+\n     def commit(self, details, files, branch, parent = \"\"):\n         epoch = details[\"time\"]\n         author = details[\"user\"]\n \n         if self.verbose:\n-            print \"commit into %s\" % branch\n-\n-        # start with reading files; if that fails, we should not\n-        # create a commit.\n-        new_files = []\n-        for f in files:\n-            if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], p)]:\n-                new_files.append (f)\n-            else:\n-                sys.stderr.write(\"Ignoring file outside of prefix: %s\\n\" % f['path'])\n+            print('commit into {0}'.format(branch))\n \n         if self.clientSpecDirs:\n             self.clientSpecDirs.update_client_spec_path_cache(files)\n \n+        files = [f for f in files\n+            if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]\n+\n+        if not files and not gitConfigBool('git-p4.keepEmptyCommits'):\n+            print('Ignoring revision {0} as it would produce an empty commit.'\n+                .format(details['change']))\n+            return\n+\n         self.gitStream.write(\"commit %s\\n\" % branch)\n #        gitStream.write(\"mark :%s\\n\" % details[\"change\"])\n         self.committedChanges.add(int(details[\"change\"]))\n@@ -2403,7 +2413,7 @@ class P4Sync(Command, P4UserMap):\n                 print \"parent %s\" % parent\n             self.gitStream.write(\"from %s\\n\" % parent)\n \n-        self.streamP4Files(new_files)\n+        self.streamP4Files(files)\n         self.gitStream.write(\"\\n\")\n \n         change = int(details[\"change\"])\ndiff --git a/t/t9826-git-p4-keep-empty-commits.sh b/t/t9826-git-p4-keep-empty-commits.sh\nnew file mode 100755\nindex 0000000..be12960\n--- /dev/null\n+++ b/t/t9826-git-p4-keep-empty-commits.sh\n@@ -0,0 +1,134 @@\n+#!/bin/sh\n+\n+test_description='Clone repositories and keep empty commits'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+test_expect_success 'Create a repo' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\n+\t\tmkdir -p subdir &&\n+\n+\t\t>subdir/file1.txt &&\n+\t\tp4 add subdir/file1.txt &&\n+\t\tp4 submit -d \"Add file 1\" &&\n+\n+\t\t>file2.txt &&\n+\t\tp4 add file2.txt &&\n+\t\tp4 submit -d \"Add file 2\" &&\n+\n+\t\t>subdir/file3.txt &&\n+\t\tp4 add subdir/file3.txt &&\n+\t\tp4 submit -d \"Add file 3\" &&\n+\n+\t\t>file4.txt &&\n+\t\tp4 add file4.txt &&\n+\t\tp4 submit -d \"Add file 4\" &&\n+\n+\t\tp4 delete subdir/file3.txt &&\n+\t\tp4 submit -d \"Remove file 3\" &&\n+\n+\t\tp4 delete file4.txt &&\n+\t\tp4 submit -d \"Remove file 4\"\n+\t)\n+'\n+\n+test_expect_success 'Clone repo root path with all history' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init . &&\n+\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n+\t\tcat >expect <<-\\EOF &&\n+Remove file 4\n+[git-p4: depot-paths = \"//depot/\": change = 6]\n+\n+Remove file 3\n+[git-p4: depot-paths = \"//depot/\": change = 5]\n+\n+Add file 4\n+[git-p4: depot-paths = \"//depot/\": change = 4]\n+\n+Add file 3\n+[git-p4: depot-paths = \"//depot/\": change = 3]\n+\n+Add file 2\n+[git-p4: depot-paths = \"//depot/\": change = 2]\n+\n+Add file 1\n+[git-p4: depot-paths = \"//depot/\": change = 1]\n+\n+\t\tEOF\n+\t\tgit log --format=%B >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'Clone repo subdir with all history but keep empty commits' '\n+\tclient_view \"//depot/subdir/... //client/subdir/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init . &&\n+\t\tgit config git-p4.keepEmptyCommits true &&\n+\t\tgit p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n+\t\tcat >expect <<-\\EOF &&\n+Remove file 4\n+[git-p4: depot-paths = \"//depot/\": change = 6]\n+\n+Remove file 3\n+[git-p4: depot-paths = \"//depot/\": change = 5]\n+\n+Add file 4\n+[git-p4: depot-paths = \"//depot/\": change = 4]\n+\n+Add file 3\n+[git-p4: depot-paths = \"//depot/\": change = 3]\n+\n+Add file 2\n+[git-p4: depot-paths = \"//depot/\": change = 2]\n+\n+Add file 1\n+[git-p4: depot-paths = \"//depot/\": change = 1]\n+\n+\t\tEOF\n+\t\tgit log --format=%B >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'Clone repo subdir with all history' '\n+\tclient_view \"//depot/subdir/... //client/subdir/...\" &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit init . &&\n+\t\tgit p4 clone --use-client-spec --destination=\"$git\" --verbose //depot@all &&\n+\t\tcat >expect <<-\\EOF &&\n+Remove file 3\n+[git-p4: depot-paths = \"//depot/\": change = 5]\n+\n+Add file 3\n+[git-p4: depot-paths = \"//depot/\": change = 3]\n+\n+Add file 1\n+[git-p4: depot-paths = \"//depot/\": change = 1]\n+\n+\t\tEOF\n+\t\tgit log --format=%B >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n2.5.1\n"},{"id":"274222","messageId":"CAE5ih79WXkJYxrr3PAkXc6Upa6nOiqTpQEBOuwxHEk6xWsO5pQ@mail.gmail.com","threadId":"40958","inReplyTo":"1449567382-63713-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v2] git-p4: Add option to keep empty commits","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-12-10T01:05:55Z","receivedAt":"2015-12-10T01:05:55Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 8 December 2015 at 09:36,  <larsxschneider@gmail.com> wrote:\n> From: Lars Schneider <larsxschneider@gmail.com>\n>\n> A changelist that contains only excluded files due to a client spec was\n> imported as an empty commit. Fix that issue by ignoring these commits.\n> Add option \"git-p4.keepEmptyCommits\" to make the previous behavior\n> available.\n\nLooks good to me. I think we discussed this a while back and it all\nseemed sensible then, apart from not being quite bold enough.\n\nAck.\n\n\n\n>\n> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>\n> Helped-by: Pete Harlan\n> ---\n>  Documentation/git-p4.txt             |   4 ++\n>  git-p4.py                            |  44 +++++++-----\n>  t/t9826-git-p4-keep-empty-commits.sh | 134 +++++++++++++++++++++++++++++++++++\n>  3 files changed, 165 insertions(+), 17 deletions(-)\n>  create mode 100755 t/t9826-git-p4-keep-empty-commits.sh\n>\n> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\n> index 82aa5d6..b3e768e 100644\n> --- a/Documentation/git-p4.txt\n> +++ b/Documentation/git-p4.txt\n> @@ -510,6 +510,10 @@ git-p4.useClientSpec::\n>         option '--use-client-spec'.  See the \"CLIENT SPEC\" section above.\n>         This variable is a boolean, not the name of a p4 client.\n>\n> +git-p4.keepEmptyCommits::\n> +       A changelist that contains only excluded files will be imported\n> +       as an empty commit if this boolean option is set to true.\n> +\n>  Submit variables\n>  ~~~~~~~~~~~~~~~~\n>  git-p4.detectRenames::\n> diff --git a/git-p4.py b/git-p4.py\n> index 0093fa3..62c26bc 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):\n>          filesToDelete = []\n>\n>          for f in files:\n> -            # if using a client spec, only add the files that have\n> -            # a path in the client\n> -            if self.clientSpecDirs:\n> -                if self.clientSpecDirs.map_in_client(f['path']) == \"\":\n> -                    continue\n> -\n>              filesForCommit.append(f)\n>              if f['action'] in self.delete_actions:\n>                  filesToDelete.append(f)\n> @@ -2361,25 +2355,41 @@ class P4Sync(Command, P4UserMap):\n>          gitStream.write(description)\n>          gitStream.write(\"\\n\")\n>\n> +    def inClientSpec(self, path):\n> +        if not self.clientSpecDirs:\n> +            return True\n> +        inClientSpec = self.clientSpecDirs.map_in_client(path)\n> +        if not inClientSpec and self.verbose:\n> +            print('Ignoring file outside of client spec: {0}'.format(path))\n> +        return inClientSpec\n> +\n> +    def hasBranchPrefix(self, path):\n> +        if not self.branchPrefixes:\n> +            return True\n> +        hasPrefix = [p for p in self.branchPrefixes\n> +                        if p4PathStartsWith(path, p)]\n> +        if hasPrefix and self.verbose:\n> +            print('Ignoring file outside of prefix: {0}'.format(path))\n> +        return hasPrefix\n> +\n>      def commit(self, details, files, branch, parent = \"\"):\n>          epoch = details[\"time\"]\n>          author = details[\"user\"]\n>\n>          if self.verbose:\n> -            print \"commit into %s\" % branch\n> -\n> -        # start with reading files; if that fails, we should not\n> -        # create a commit.\n> -        new_files = []\n> -        for f in files:\n> -            if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], p)]:\n> -                new_files.append (f)\n> -            else:\n> -                sys.stderr.write(\"Ignoring file outside of prefix: %s\\n\" % f['path'])\n> +            print('commit into {0}'.format(branch))\n>\n>          if self.clientSpecDirs:\n>              self.clientSpecDirs.update_client_spec_path_cache(files)\n>\n> +        files = [f for f in files\n> +            if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]\n> +\n> +        if not files and not gitConfigBool('git-p4.keepEmptyCommits'):\n> +            print('Ignoring revision {0} as it would produce an empty commit.'\n> +                .format(details['change']))\n> +            return\n> +\n>          self.gitStream.write(\"commit %s\\n\" % branch)\n>  #        gitStream.write(\"mark :%s\\n\" % details[\"change\"])\n>          self.committedChanges.add(int(details[\"change\"]))\n> @@ -2403,7 +2413,7 @@ class P4Sync(Command, P4UserMap):\n>                  print \"parent %s\" % parent\n>              self.gitStream.write(\"from %s\\n\" % parent)\n>\n> -        self.streamP4Files(new_files)\n> +        self.streamP4Files(files)\n>          self.gitStream.write(\"\\n\")\n>\n>          change = int(details[\"change\"])\n> diff --git a/t/t9826-git-p4-keep-empty-commits.sh b/t/t9826-git-p4-keep-empty-commits.sh\n> new file mode 100755\n> index 0000000..be12960\n> --- /dev/null\n> +++ b/t/t9826-git-p4-keep-empty-commits.sh\n> @@ -0,0 +1,134 @@\n> +#!/bin/sh\n> +\n> +test_description='Clone repositories and keep empty commits'\n> +\n> +. ./lib-git-p4.sh\n> +\n> +test_expect_success 'start p4d' '\n> +       start_p4d\n> +'\n> +\n> +test_expect_success 'Create a repo' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       (\n> +               cd \"$cli\" &&\n> +\n> +               mkdir -p subdir &&\n> +\n> +               >subdir/file1.txt &&\n> +               p4 add subdir/file1.txt &&\n> +               p4 submit -d \"Add file 1\" &&\n> +\n> +               >file2.txt &&\n> +               p4 add file2.txt &&\n> +               p4 submit -d \"Add file 2\" &&\n> +\n> +               >subdir/file3.txt &&\n> +               p4 add subdir/file3.txt &&\n> +               p4 submit -d \"Add file 3\" &&\n> +\n> +               >file4.txt &&\n> +               p4 add file4.txt &&\n> +               p4 submit -d \"Add file 4\" &&\n> +\n> +               p4 delete subdir/file3.txt &&\n> +               p4 submit -d \"Remove file 3\" &&\n> +\n> +               p4 delete file4.txt &&\n> +               p4 submit -d \"Remove file 4\"\n> +       )\n> +'\n> +\n> +test_expect_success 'Clone repo root path with all history' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +               git init . &&\n> +               git p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n> +               cat >expect <<-\\EOF &&\n> +Remove file 4\n> +[git-p4: depot-paths = \"//depot/\": change = 6]\n> +\n> +Remove file 3\n> +[git-p4: depot-paths = \"//depot/\": change = 5]\n> +\n> +Add file 4\n> +[git-p4: depot-paths = \"//depot/\": change = 4]\n> +\n> +Add file 3\n> +[git-p4: depot-paths = \"//depot/\": change = 3]\n> +\n> +Add file 2\n> +[git-p4: depot-paths = \"//depot/\": change = 2]\n> +\n> +Add file 1\n> +[git-p4: depot-paths = \"//depot/\": change = 1]\n> +\n> +               EOF\n> +               git log --format=%B >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'Clone repo subdir with all history but keep empty commits' '\n> +       client_view \"//depot/subdir/... //client/subdir/...\" &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +               git init . &&\n> +               git config git-p4.keepEmptyCommits true &&\n> +               git p4 clone --use-client-spec --destination=\"$git\" //depot@all &&\n> +               cat >expect <<-\\EOF &&\n> +Remove file 4\n> +[git-p4: depot-paths = \"//depot/\": change = 6]\n> +\n> +Remove file 3\n> +[git-p4: depot-paths = \"//depot/\": change = 5]\n> +\n> +Add file 4\n> +[git-p4: depot-paths = \"//depot/\": change = 4]\n> +\n> +Add file 3\n> +[git-p4: depot-paths = \"//depot/\": change = 3]\n> +\n> +Add file 2\n> +[git-p4: depot-paths = \"//depot/\": change = 2]\n> +\n> +Add file 1\n> +[git-p4: depot-paths = \"//depot/\": change = 1]\n> +\n> +               EOF\n> +               git log --format=%B >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'Clone repo subdir with all history' '\n> +       client_view \"//depot/subdir/... //client/subdir/...\" &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +               git init . &&\n> +               git p4 clone --use-client-spec --destination=\"$git\" --verbose //depot@all &&\n> +               cat >expect <<-\\EOF &&\n> +Remove file 3\n> +[git-p4: depot-paths = \"//depot/\": change = 5]\n> +\n> +Add file 3\n> +[git-p4: depot-paths = \"//depot/\": change = 3]\n> +\n> +Add file 1\n> +[git-p4: depot-paths = \"//depot/\": change = 1]\n> +\n> +               EOF\n> +               git log --format=%B >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'kill p4d' '\n> +       kill_p4d\n> +'\n> +\n> +test_done\n> --\n> 2.5.1\n>\n"}]}