{"thread":{"id":"42722","subject":"[PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp","startedAt":"2016-06-27T07:26:45Z","lastAt":"2016-07-01T20:11:46Z","messageCount":7,"participants":["larsxschneider@gmail.com","Michael Haggerty","Johannes Sixt","Junio C Hamano","Vitor Antunes"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"290225","messageId":"1467012398-7357-1-git-send-email-larsxschneider@gmail.com","threadId":"42722","inReplyTo":null,"subject":"[PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-06-27T07:26:38Z","receivedAt":"2016-06-27T07:26:45Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nGit-P4 used to place temporary refs under \"git-p4-tmp\". Since 3da1f37\nGit checks that all refs are placed under \"ref\". Instruct Git-P4 to\nplace temporary refs under \"ref/git-p4-tmp\". There are no backwards\ncompatibility considerations as these refs are transient.\n\nAll refs under \"ref\" are shared across all worktrees. This is not\ndesired for temporary Git-P4 refs and will be adressed in a later patch.\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n\nPlease note: As mentioned in $gmane/297703 I am no expert for the Git-P4\nbranch import. I post this patch to make the Git-P4 unit tests working,\nagain. Critical review highly appreciated :-)\n\nThanks,\nLars\n\n\n git-p4.py                | 2 +-\n t/t9801-git-p4-branch.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex b6593cf..6b252df 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap):\n         self.useClientSpec_from_options = False\n         self.clientSpecDirs = None\n         self.tempBranches = []\n-        self.tempBranchLocation = \"git-p4-tmp\"\n+        self.tempBranchLocation = \"refs/git-p4-tmp\"\n         self.largeFileSystem = None\n\n         if gitConfig('git-p4.largeFileSystem'):\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 0aafd03..8f28ed2 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' '\n \t\ttest_path_is_file file2 &&\n \t\ttest_path_is_file file3 &&\n \t\t! grep update file2 &&\n-\t\ttest_path_is_missing .git/git-p4-tmp\n+\t\ttest_path_is_missing .git/ref/git-p4-tmp\n \t)\n '\n\n@@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist'\n \t\ttest_path_is_file file2 &&\n \t\ttest_path_is_file file3 &&\n \t\t! grep update file2 &&\n-\t\ttest_path_is_missing .git/git-p4-tmp\n+\t\ttest_path_is_missing .git/ref/git-p4-tmp\n \t)\n '\n\n--\n2.5.1\n\n"},{"id":"290361","messageId":"577250D4.3010106@alum.mit.edu","threadId":"42722","inReplyTo":"1467012398-7357-1-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-06-28T10:26:28Z","receivedAt":"2016-06-28T10:27:44Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/27/2016 09:26 AM, larsxschneider@gmail.com wrote:\n> Git-P4 used to place temporary refs under \"git-p4-tmp\". Since 3da1f37\n> Git checks that all refs are placed under \"ref\". Instruct Git-P4 to\n> place temporary refs under \"ref/git-p4-tmp\". There are no backwards\n> compatibility considerations as these refs are transient.\n> \n> All refs under \"ref\" are shared across all worktrees. This is not\n> desired for temporary Git-P4 refs and will be adressed in a later patch.\n\nThanks for working on this, Lars! Your change looks about what I would\nexpect, and hits the three places in the source where the string\n`git-p4-tmp` appears, so it seems like a very plausible fix.\n\nMichael\n\n"},{"id":"290420","messageId":"5772C00C.6000403@kdbg.org","threadId":"42722","inReplyTo":"1467012398-7357-1-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-06-28T18:21:00Z","receivedAt":"2016-06-28T18:21:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.06.2016 um 09:26 schrieb larsxschneider@gmail.com:\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap):\n>           self.useClientSpec_from_options = False\n>           self.clientSpecDirs = None\n>           self.tempBranches = []\n> -        self.tempBranchLocation = \"git-p4-tmp\"\n> +        self.tempBranchLocation = \"refs/git-p4-tmp\"\n>           self.largeFileSystem = None\n>\n>           if gitConfig('git-p4.largeFileSystem'):\n> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\n> index 0aafd03..8f28ed2 100755\n> --- a/t/t9801-git-p4-branch.sh\n> +++ b/t/t9801-git-p4-branch.sh\n> @@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' '\n>   \t\ttest_path_is_file file2 &&\n>   \t\ttest_path_is_file file3 &&\n>   \t\t! grep update file2 &&\n> -\t\ttest_path_is_missing .git/git-p4-tmp\n> +\t\ttest_path_is_missing .git/ref/git-p4-tmp\n\nThis should be .git/refs/git-p4-tmp, no? Otherwise, this does not test \nwhat it should test.\n\n>   \t)\n>   '\n>\n> @@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist'\n>   \t\ttest_path_is_file file2 &&\n>   \t\ttest_path_is_file file3 &&\n>   \t\t! grep update file2 &&\n> -\t\ttest_path_is_missing .git/git-p4-tmp\n> +\t\ttest_path_is_missing .git/ref/git-p4-tmp\n\nSame here.\n\n>   \t)\n>   '\n\n-- Hannes\n\n"},{"id":"290423","messageId":"xmqqeg7h87yg.fsf@gitster.mtv.corp.google.com","threadId":"42722","inReplyTo":"5772C00C.6000403@kdbg.org","subject":"Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-28T18:49:27Z","receivedAt":"2016-06-28T18:49:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 27.06.2016 um 09:26 schrieb larsxschneider@gmail.com:\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap):\n>>           self.useClientSpec_from_options = False\n>>           self.clientSpecDirs = None\n>>           self.tempBranches = []\n>> -        self.tempBranchLocation = \"git-p4-tmp\"\n>> +        self.tempBranchLocation = \"refs/git-p4-tmp\"\n>>           self.largeFileSystem = None\n>>\n>>           if gitConfig('git-p4.largeFileSystem'):\n>> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\n>> index 0aafd03..8f28ed2 100755\n>> --- a/t/t9801-git-p4-branch.sh\n>> +++ b/t/t9801-git-p4-branch.sh\n>> @@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' '\n>>   \t\ttest_path_is_file file2 &&\n>>   \t\ttest_path_is_file file3 &&\n>>   \t\t! grep update file2 &&\n>> -\t\ttest_path_is_missing .git/git-p4-tmp\n>> +\t\ttest_path_is_missing .git/ref/git-p4-tmp\n>\n> This should be .git/refs/git-p4-tmp, no? Otherwise, this does not test\n> what it should test.\n\nYes, and it probably should use \"git show-ref --verify\" to\nfuture-proof, instead of assuming the file-based ref backend.\n\n>\n>>   \t)\n>>   '\n>>\n>> @@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist'\n>>   \t\ttest_path_is_file file2 &&\n>>   \t\ttest_path_is_file file3 &&\n>>   \t\t! grep update file2 &&\n>> -\t\ttest_path_is_missing .git/git-p4-tmp\n>> +\t\ttest_path_is_missing .git/ref/git-p4-tmp\n>\n> Same here.\n>\n>>   \t)\n>>   '\n>\n> -- Hannes\n"},{"id":"290461","messageId":"1467185727-8235-1-git-send-email-larsxschneider@gmail.com","threadId":"42722","inReplyTo":"xmqqeg7h87yg.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2] git-p4: place temporary refs used for branch import under refs/git-p4-tmp","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-06-29T07:35:27Z","receivedAt":"2016-06-29T07:35:30Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nGit-P4 used to place temporary refs under \"git-p4-tmp\". Since 3da1f37\nGit checks that all refs are placed under \"refs\". Instruct Git-P4 to\nplace temporary refs under \"refs/git-p4-tmp\". There are no backwards\ncompatibility considerations as these refs are transient.\n\nUse \"git show-ref --verify\" to check the (non-)existience of the refs\ninstead of file checks assuming the file-based ref backend.\n\nAll refs under \"refs\" are shared across all worktrees. This is not\ndesired for temporary Git-P4 refs and will be adressed in a later patch.\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n\nThank you Hannes for the sharp eye!\n\ndiff to v1:\n* check non-existence of refs/git-p4-tmp instead of ref/git-p4-tmp\n* use refs/git-p4-tmp instead of ref/git-p4-tmp in commit message\n* check reference with \"git show-ref --verify\" to be future-proof (thanks Junio!)\n\nCheers,\nLars\n\n\n git-p4.py                | 2 +-\n t/t9801-git-p4-branch.sh | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex b6593cf..6b252df 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap):\n         self.useClientSpec_from_options = False\n         self.clientSpecDirs = None\n         self.tempBranches = []\n-        self.tempBranchLocation = \"git-p4-tmp\"\n+        self.tempBranchLocation = \"refs/git-p4-tmp\"\n         self.largeFileSystem = None\n\n         if gitConfig('git-p4.largeFileSystem'):\ndiff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh\nindex 0aafd03..6a86d69 100755\n--- a/t/t9801-git-p4-branch.sh\n+++ b/t/t9801-git-p4-branch.sh\n@@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' '\n \t\ttest_path_is_file file2 &&\n \t\ttest_path_is_file file3 &&\n \t\t! grep update file2 &&\n-\t\ttest_path_is_missing .git/git-p4-tmp\n+\t\ttest_must_fail git show-ref --verify refs/git-p4-tmp\n \t)\n '\n\n@@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist'\n \t\ttest_path_is_file file2 &&\n \t\ttest_path_is_file file3 &&\n \t\t! grep update file2 &&\n-\t\ttest_path_is_missing .git/git-p4-tmp\n+\t\ttest_must_fail git show-ref --verify refs/git-p4-tmp\n \t)\n '\n\n--\n2.5.1\n\n"},{"id":"290661","messageId":"loom.20160701T154359-664@post.gmane.org","threadId":"42722","inReplyTo":"1467185727-8235-1-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v2] git-p4: place temporary refs used for branch import under refs/git-p4-tmp","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2016-07-01T13:45:44Z","receivedAt":"2016-07-01T13:45:59Z","isPatch":true,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"Hi,\n\nFrom a git-p4 point of view, I see no problems with this change.\n\nThanks and regards,\nVitor\n\n\n\n"},{"id":"290709","messageId":"xmqq37ntxgod.fsf@gitster.mtv.corp.google.com","threadId":"42722","inReplyTo":"loom.20160701T154359-664@post.gmane.org","subject":"Re: [PATCH v2] git-p4: place temporary refs used for branch import under refs/git-p4-tmp","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-01T20:10:58Z","receivedAt":"2016-07-01T20:11:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"}]}