# [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp 7 messages from 2016-06-27 to 2016-07-01. Participants: larsxschneider@gmail.com, Michael Haggerty, Johannes Sixt, Junio C Hamano, Vitor Antunes. Thread: https://gitlist.dev/t/42722 ## larsxschneider@gmail.com, 2016-06-27 07:26 Subject: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp Message-ID: <1467012398-7357-1-git-send-email-larsxschneider@gmail.com> URL: https://gitlist.dev/e/1467012398-7357-1-git-send-email-larsxschneider%40gmail.com ``` From: Lars Schneider Git-P4 used to place temporary refs under "git-p4-tmp". Since 3da1f37 Git checks that all refs are placed under "ref". Instruct Git-P4 to place temporary refs under "ref/git-p4-tmp". There are no backwards compatibility considerations as these refs are transient. All refs under "ref" are shared across all worktrees. This is not desired for temporary Git-P4 refs and will be adressed in a later patch. Signed-off-by: Lars Schneider --- Please note: As mentioned in $gmane/297703 I am no expert for the Git-P4 branch import. I post this patch to make the Git-P4 unit tests working, again. Critical review highly appreciated :-) Thanks, Lars git-p4.py | 2 +- t/t9801-git-p4-branch.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index b6593cf..6b252df 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap): self.useClientSpec_from_options = False self.clientSpecDirs = None self.tempBranches = [] - self.tempBranchLocation = "git-p4-tmp" + self.tempBranchLocation = "refs/git-p4-tmp" self.largeFileSystem = None if gitConfig('git-p4.largeFileSystem'): diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index 0aafd03..8f28ed2 100755 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' ' test_path_is_file file2 && test_path_is_file file3 && ! grep update file2 && - test_path_is_missing .git/git-p4-tmp + test_path_is_missing .git/ref/git-p4-tmp ) ' @@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist' test_path_is_file file2 && test_path_is_file file3 && ! grep update file2 && - test_path_is_missing .git/git-p4-tmp + test_path_is_missing .git/ref/git-p4-tmp ) ' -- 2.5.1 ``` ## Michael Haggerty, 2016-06-28 10:26 Subject: Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp Message-ID: <577250D4.3010106@alum.mit.edu> URL: https://gitlist.dev/e/577250D4.3010106%40alum.mit.edu In-Reply-To: <1467012398-7357-1-git-send-email-larsxschneider@gmail.com> ``` On 06/27/2016 09:26 AM, larsxschneider@gmail.com wrote: > Git-P4 used to place temporary refs under "git-p4-tmp". Since 3da1f37 > Git checks that all refs are placed under "ref". Instruct Git-P4 to > place temporary refs under "ref/git-p4-tmp". There are no backwards > compatibility considerations as these refs are transient. > > All refs under "ref" are shared across all worktrees. This is not > desired for temporary Git-P4 refs and will be adressed in a later patch. Thanks for working on this, Lars! Your change looks about what I would expect, and hits the three places in the source where the string `git-p4-tmp` appears, so it seems like a very plausible fix. Michael ``` ## Johannes Sixt, 2016-06-28 18:21 Subject: Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp Message-ID: <5772C00C.6000403@kdbg.org> URL: https://gitlist.dev/e/5772C00C.6000403%40kdbg.org In-Reply-To: <1467012398-7357-1-git-send-email-larsxschneider@gmail.com> ``` Am 27.06.2016 um 09:26 schrieb larsxschneider@gmail.com: > --- a/git-p4.py > +++ b/git-p4.py > @@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap): > self.useClientSpec_from_options = False > self.clientSpecDirs = None > self.tempBranches = [] > - self.tempBranchLocation = "git-p4-tmp" > + self.tempBranchLocation = "refs/git-p4-tmp" > self.largeFileSystem = None > > if gitConfig('git-p4.largeFileSystem'): > diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh > index 0aafd03..8f28ed2 100755 > --- a/t/t9801-git-p4-branch.sh > +++ b/t/t9801-git-p4-branch.sh > @@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' ' > test_path_is_file file2 && > test_path_is_file file3 && > ! grep update file2 && > - test_path_is_missing .git/git-p4-tmp > + test_path_is_missing .git/ref/git-p4-tmp This should be .git/refs/git-p4-tmp, no? Otherwise, this does not test what it should test. > ) > ' > > @@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist' > test_path_is_file file2 && > test_path_is_file file3 && > ! grep update file2 && > - test_path_is_missing .git/git-p4-tmp > + test_path_is_missing .git/ref/git-p4-tmp Same here. > ) > ' -- Hannes ``` ## Junio C Hamano, 2016-06-28 18:49 Subject: Re: [PATCH v1] git-p4: place temporary refs used for branch import under ref/git-p4-tmp Message-ID: URL: https://gitlist.dev/e/xmqqeg7h87yg.fsf%40gitster.mtv.corp.google.com In-Reply-To: <5772C00C.6000403@kdbg.org> ``` Johannes Sixt writes: > Am 27.06.2016 um 09:26 schrieb larsxschneider@gmail.com: >> --- a/git-p4.py >> +++ b/git-p4.py >> @@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap): >> self.useClientSpec_from_options = False >> self.clientSpecDirs = None >> self.tempBranches = [] >> - self.tempBranchLocation = "git-p4-tmp" >> + self.tempBranchLocation = "refs/git-p4-tmp" >> self.largeFileSystem = None >> >> if gitConfig('git-p4.largeFileSystem'): >> diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh >> index 0aafd03..8f28ed2 100755 >> --- a/t/t9801-git-p4-branch.sh >> +++ b/t/t9801-git-p4-branch.sh >> @@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' ' >> test_path_is_file file2 && >> test_path_is_file file3 && >> ! grep update file2 && >> - test_path_is_missing .git/git-p4-tmp >> + test_path_is_missing .git/ref/git-p4-tmp > > This should be .git/refs/git-p4-tmp, no? Otherwise, this does not test > what it should test. Yes, and it probably should use "git show-ref --verify" to future-proof, instead of assuming the file-based ref backend. > >> ) >> ' >> >> @@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist' >> test_path_is_file file2 && >> test_path_is_file file3 && >> ! grep update file2 && >> - test_path_is_missing .git/git-p4-tmp >> + test_path_is_missing .git/ref/git-p4-tmp > > Same here. > >> ) >> ' > > -- Hannes ``` ## larsxschneider@gmail.com, 2016-06-29 07:35 Subject: [PATCH v2] git-p4: place temporary refs used for branch import under refs/git-p4-tmp Message-ID: <1467185727-8235-1-git-send-email-larsxschneider@gmail.com> URL: https://gitlist.dev/e/1467185727-8235-1-git-send-email-larsxschneider%40gmail.com In-Reply-To: ``` From: Lars Schneider Git-P4 used to place temporary refs under "git-p4-tmp". Since 3da1f37 Git checks that all refs are placed under "refs". Instruct Git-P4 to place temporary refs under "refs/git-p4-tmp". There are no backwards compatibility considerations as these refs are transient. Use "git show-ref --verify" to check the (non-)existience of the refs instead of file checks assuming the file-based ref backend. All refs under "refs" are shared across all worktrees. This is not desired for temporary Git-P4 refs and will be adressed in a later patch. Signed-off-by: Lars Schneider --- Thank you Hannes for the sharp eye! diff to v1: * check non-existence of refs/git-p4-tmp instead of ref/git-p4-tmp * use refs/git-p4-tmp instead of ref/git-p4-tmp in commit message * check reference with "git show-ref --verify" to be future-proof (thanks Junio!) Cheers, Lars git-p4.py | 2 +- t/t9801-git-p4-branch.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index b6593cf..6b252df 100755 --- a/git-p4.py +++ b/git-p4.py @@ -2274,7 +2274,7 @@ class P4Sync(Command, P4UserMap): self.useClientSpec_from_options = False self.clientSpecDirs = None self.tempBranches = [] - self.tempBranchLocation = "git-p4-tmp" + self.tempBranchLocation = "refs/git-p4-tmp" self.largeFileSystem = None if gitConfig('git-p4.largeFileSystem'): diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh index 0aafd03..6a86d69 100755 --- a/t/t9801-git-p4-branch.sh +++ b/t/t9801-git-p4-branch.sh @@ -300,7 +300,7 @@ test_expect_success 'git p4 clone complex branches' ' test_path_is_file file2 && test_path_is_file file3 && ! grep update file2 && - test_path_is_missing .git/git-p4-tmp + test_must_fail git show-ref --verify refs/git-p4-tmp ) ' @@ -352,7 +352,7 @@ test_expect_success 'git p4 sync changes to two branches in the same changelist' test_path_is_file file2 && test_path_is_file file3 && ! grep update file2 && - test_path_is_missing .git/git-p4-tmp + test_must_fail git show-ref --verify refs/git-p4-tmp ) ' -- 2.5.1 ``` ## Vitor Antunes, 2016-07-01 13:45 Subject: Re: [PATCH v2] git-p4: place temporary refs used for branch import under refs/git-p4-tmp Message-ID: URL: https://gitlist.dev/e/loom.20160701T154359-664%40post.gmane.org In-Reply-To: <1467185727-8235-1-git-send-email-larsxschneider@gmail.com> ``` Hi, From a git-p4 point of view, I see no problems with this change. Thanks and regards, Vitor ``` ## Junio C Hamano, 2016-07-01 20:10 Subject: Re: [PATCH v2] git-p4: place temporary refs used for branch import under refs/git-p4-tmp Message-ID: URL: https://gitlist.dev/e/xmqq37ntxgod.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Thanks. ```