git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/2] git-p4: unshelve uses HEAD^n, not HEAD~n
@ 2020-09-19  8:54 Luke Diamand
  2020-09-19  8:54 ` [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve Luke Diamand
  0 siblings, 1 reply; 5+ messages in thread
From: Luke Diamand @ 2020-09-19  8:54 UTC (permalink / raw)
  To: git; +Cc: Liu Xuhui (Jackson), 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve
  2020-09-19  8:54 [PATCH 0/2] git-p4: unshelve uses HEAD^n, not HEAD~n Luke Diamand
@ 2020-09-19  8:54 ` Luke Diamand
  2020-09-19  8:54   ` [PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve Luke Diamand
  2020-09-20  5:31   ` [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve Eric Sunshine
  0 siblings, 2 replies; 5+ messages in thread
From: Luke Diamand @ 2020-09-19  8:54 UTC (permalink / raw)
  To: git; +Cc: Liu Xuhui (Jackson), 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve
  2020-09-19  8:54 ` [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve Luke Diamand
@ 2020-09-19  8:54   ` Luke Diamand
  2020-09-20  5:34     ` Eric Sunshine
  2020-09-20  5:31   ` [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve Eric Sunshine
  1 sibling, 1 reply; 5+ messages in thread
From: Luke Diamand @ 2020-09-19  8:54 UTC (permalink / raw)
  To: git; +Cc: Liu Xuhui (Jackson), 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve
  2020-09-19  8:54 ` [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve Luke Diamand
  2020-09-19  8:54   ` [PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve Luke Diamand
@ 2020-09-20  5:31   ` Eric Sunshine
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Sunshine @ 2020-09-20  5:31 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Git List, Liu Xuhui (Jackson)

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve
  2020-09-19  8:54   ` [PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve Luke Diamand
@ 2020-09-20  5:34     ` Eric Sunshine
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Sunshine @ 2020-09-20  5:34 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Git List, Liu Xuhui (Jackson)

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>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-20  5:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-19  8:54 [PATCH 0/2] git-p4: unshelve uses HEAD^n, not HEAD~n Luke Diamand
2020-09-19  8:54 ` [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve Luke Diamand
2020-09-19  8:54   ` [PATCH 2/2] git-p4: use HEAD~$n to find parent commit for unshelve Luke Diamand
2020-09-20  5:34     ` Eric Sunshine
2020-09-20  5:31   ` [PATCH 1/2] git-p4 unshelve: adding a commit breaks git-p4 unshelve Eric Sunshine

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git