threads / patch / 54260

patch, 2 partsgit-p4: unshelve uses HEAD^n, not HEAD~n

Subject: [PATCH 0/2] git-p4: unshelve uses HEAD^n, not HEAD~n

## tl;dr

5 messages between Sep 19, 2020 and Sep 20, 2020. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Luke Diamand· Sep 19, 2020, 08:54 UTC · lore

Jackson(Xuhui) Liu found that git-p4 unshelve fails, and suggested a fix.

I have updated the tests to spot the error, and added his suggested fix, which also works for me.

Luke Diamand (2):
  git-p4 unshelve: adding a commit breaks git-p4 unshelve
  git-p4: use HEAD~$n to find parent commit for unshelve
 git-p4.py           | 2 +-
 t/t9832-unshelve.sh | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)
-- 
2.28.0
Luke Diamand· Sep 19, 2020, 08:54 UTC · re: Luke Diamand · lore

[PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve

git-p4 unshelve uses HEAD^$n to find the parent commit, which fails if there is an additional commit.

Signed-off-by: Luke Diamand <luke@diamand.org>
---
 t/t9832-unshelve.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
Show changes to t/t9832-unshelve.sh +5 −2
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
index e9276c48f4..feda4499dd 100755
--- a/t/t9832-unshelve.sh
+++ b/t/t9832-unshelve.sh
@@ -29,8 +29,11 @@ test_expect_success 'init depot' '
 	)
 '
 
+# Create an initial clone, with a commit unrelated to the P4 change
+# on HEAD
 test_expect_success 'initial clone' '
-	git p4 clone --dest="$git" //depot/@all
+	git p4 clone --dest="$git" //depot/@all &&
+    test_commit -C "$git" "unrelated"
 '
 
 test_expect_success 'create shelved changelist' '
@@ -77,7 +80,7 @@ EOF
 	)
 '
 
-test_expect_success 'update shelved changelist and re-unshelve' '
+test_expect_failure 'update shelved changelist and re-unshelve' '
 	test_when_finished cleanup_git &&
 	(
 		cd "$cli" &&

-- 
2.28.0
Luke Diamand· Sep 19, 2020, 08:54 UTC · re: Luke Diamand · lore

[PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve

Found-by: Liu Xuhui (Jackson) <Xuhui.Liu@amd.com>
Signed-off-by: Luke Diamand <luke@diamand.org>
---
 git-p4.py           | 2 +-
 t/t9832-unshelve.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Show changes to 2 files +2 −2

git-p4.py, t/t9832-unshelve.sh

diff --git a/git-p4.py b/git-p4.py
index ca79dc0900..4433ca53de 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -4237,7 +4237,7 @@ def findLastP4Revision(self, starting_point):
         """
 
         for parent in (range(65535)):
-            log = extractLogMessageFromGitCommit("{0}^{1}".format(starting_point, parent))
+            log = extractLogMessageFromGitCommit("{0}~{1}".format(starting_point, parent))
             settings = extractSettingsGitLog(log)
             if 'change' in settings:
                 return settings
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
index feda4499dd..7194fb2855 100755
--- a/t/t9832-unshelve.sh
+++ b/t/t9832-unshelve.sh
@@ -80,7 +80,7 @@ EOF
 	)
 '
 
-test_expect_failure 'update shelved changelist and re-unshelve' '
+test_expect_success 'update shelved changelist and re-unshelve' '
 	test_when_finished cleanup_git &&
 	(
 		cd "$cli" &&

-- 
2.28.0
Eric Sunshine· Sep 20, 2020, 05:34 UTC · re: Luke Diamand · lore

Re: [PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve

On Sat, Sep 19, 2020 at 4:54 AM Luke Diamand <luke@diamand.org> wrote:
> git-p4: use HEAD~$n to find parent commit for unshelve

This commit message repeats what the patch itself says but doesn't explain why this change is being made or what problem is being solved. Some explanation to help readers understand the problem would be welcome.

> Found-by: Liu Xuhui (Jackson) <Xuhui.Liu@amd.com>
I believe this would generally be stated as Reported-by:.
> Signed-off-by: Luke Diamand <luke@diamand.org>
Eric Sunshine· Sep 20, 2020, 05:31 UTC · re: Luke Diamand · lore

Re: [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve

On Sat, Sep 19, 2020 at 4:54 AM Luke Diamand <luke@diamand.org> wrote:
> git-p4 unshelve: adding a commit breaks git-p4 unshelve
>
> git-p4 unshelve uses HEAD^$n to find the parent commit, which
> fails if there is an additional commit.

It was a bit difficult understanding the purpose of this patch based upon the commit message alone. It might be clearer if written like this:

    git-p4: demonstrate `unshelve` bug
    `git p4 unshelve` uses HEAD^$n to find the parent commit, which
    fails if there is an additional commit. Augment the tests to
    demonstrate this problem.
Show 11 quoted lines
> Signed-off-by: Luke Diamand <luke@diamand.org>
> ---
> diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
> @@ -29,8 +29,11 @@ test_expect_success 'init depot' '
> +# Create an initial clone, with a commit unrelated to the P4 change
> +# on HEAD
>  test_expect_success 'initial clone' '
> -       git p4 clone --dest="$git" //depot/@all
> +       git p4 clone --dest="$git" //depot/@all &&
> +    test_commit -C "$git" "unrelated"
>  '
Strange indentation of the new line. Use TAB rather than spaces.

← back to recent threads