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

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

[Luke Diamand]

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

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

[Luke Diamand]

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(-)

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

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

[Luke Diamand]

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(-)

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

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

[Eric Sunshine]

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>

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

[Eric Sunshine]

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.

> 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.