git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 1/4] rebase -i: demonstrate obscure loose object cache bug
Date: Wed, 13 Mar 2019 03:16:31 -0700 (PDT)	[thread overview]
Message-ID: <b3fcd377652103584b6f307c6ee209980b44529f.1552472189.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.161.git.gitgitgadget@gmail.com>

From: Johannes Schindelin <johannes.schindelin@gmx.de>

We specifically support `exec` commands in `git rebase -i`'s todo lists
to rewrite the very same todo list. Of course, we need to validate that
todo list when re-reading it.

It is also totally legitimate to extend the todo list by `pick` lines
using short names of commits that were created only after the rebase
started.

And this is where the loose object cache interferes with this feature:
if *some* loose object was read whose hash shares the same first two
digits with a commit that was not yet created when that loose object was
created, then we fail to find that new commit by its short name in
`get_oid()`, and the interactive rebase fails with an obscure error
message like:

	error: invalid line 1: pick 6568fef
	error: please fix this using 'git rebase --edit-todo'.

Let's first demonstrate that this is actually a bug in a new regression
test, in a separate commit so that other developers who do not believe
me can cherry-pick it to confirm the problem.

This new regression test generates two commits whose hashes share the
first two hex digits (so that their corresponding loose objects live in
the same subdirectory of .git/objects/, and are therefore supposed to be
in the same loose object cache bin).

It then picks the first, to make sure that the loose object cache is
initialized and cached that object directory, then generates the second
commit and picks it, too. Since the commit was generated in a different
process than the sequencer that wants to pick it, the loose object cache
had no chance of being updated in the meantime.

Technically, we would need only one `exec` command in this regression
test case, but for ease of implementation, it uses a pseudo-recursive
call to the same script.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3429-rebase-edit-todo.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/t/t3429-rebase-edit-todo.sh b/t/t3429-rebase-edit-todo.sh
index b9292dfc2a..862f229c87 100755
--- a/t/t3429-rebase-edit-todo.sh
+++ b/t/t3429-rebase-edit-todo.sh
@@ -11,4 +11,26 @@ test_expect_success 'rebase exec modifies rebase-todo' '
 	test -e F
 '
 
+test_expect_failure SHA1 'loose object cache vs re-reading todo list' '
+	GIT_REBASE_TODO=.git/rebase-merge/git-rebase-todo &&
+	export GIT_REBASE_TODO &&
+	write_script append-todo.sh <<-\EOS &&
+	# For values 5 and 6, this yields SHA-1s with the same first two digits
+	echo "pick $(git rev-parse --short \
+		$(printf "%s\\n" \
+			"tree $EMPTY_TREE" \
+			"author A U Thor <author@example.org> $1 +0000" \
+			"committer A U Thor <author@example.org> $1 +0000" \
+			"" \
+			"$1" |
+		  git hash-object -t commit -w --stdin))" >>$GIT_REBASE_TODO
+
+	shift
+	test -z "$*" ||
+	echo "exec $0 $*" >>$GIT_REBASE_TODO
+	EOS
+
+	git rebase HEAD -x "./append-todo.sh 5 6"
+'
+
 test_done
-- 
gitgitgadget


  reply	other threads:[~2019-03-13 10:16 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13 10:16 [PATCH 0/4] get_oid: cope with a possibly stale loose object cache Johannes Schindelin via GitGitGadget
2019-03-13 10:16 ` Johannes Schindelin via GitGitGadget [this message]
2019-03-13 16:11   ` [PATCH 1/4] rebase -i: demonstrate obscure loose object cache bug Ævar Arnfjörð Bjarmason
2019-03-13 16:35     ` Jeff King
2019-03-13 16:53       ` Jeff King
2019-03-13 22:40         ` Johannes Schindelin
2019-03-14  0:07           ` Jeff King
2019-03-13 22:27       ` Johannes Schindelin
2019-03-14  0:05         ` Jeff King
2019-03-14  6:40       ` Johannes Sixt
2019-03-14 13:06         ` Johannes Schindelin
2019-03-14  1:12   ` Junio C Hamano
2019-03-14 12:44     ` Johannes Schindelin
2019-03-13 10:16 ` [PATCH 2/4] sequencer: improve error message when an OID could not be parsed Johannes Schindelin via GitGitGadget
2019-03-14  1:14   ` Junio C Hamano
2019-03-13 10:16 ` [PATCH 3/4] sequencer: move stale comment into correct location Johannes Schindelin via GitGitGadget
2019-03-13 10:16 ` [PATCH 4/4] get_oid(): when an object was not found, try harder Johannes Schindelin via GitGitGadget
2019-03-14  1:29   ` Junio C Hamano
2019-03-14  2:22     ` Jeff King
2019-03-14  2:40       ` Jeff King
2019-03-14  4:05         ` Junio C Hamano
2019-03-14 18:55           ` Jeff King
2019-03-14  3:49       ` Junio C Hamano
2019-03-14 13:17     ` Johannes Schindelin
2019-03-14 18:57       ` Jeff King
2019-03-14 19:55         ` Johannes Schindelin
2019-03-13 15:33 ` [PATCH 0/4] get_oid: cope with a possibly stale loose object cache Jeff King
2019-03-14 15:33 ` [PATCH v2 0/4] get_short_oid: " Johannes Schindelin via GitGitGadget
2019-03-14 15:33   ` [PATCH v2 1/4] rebase -i: demonstrate obscure loose object cache bug Johannes Schindelin via GitGitGadget
2019-03-14 15:33   ` [PATCH v2 2/4] sequencer: improve error message when an OID could not be parsed Johannes Schindelin via GitGitGadget
2019-03-14 15:33   ` [PATCH v2 3/4] sequencer: move stale comment into correct location Johannes Schindelin via GitGitGadget
2019-03-14 15:33   ` [PATCH v2 4/4] get_oid(): when an object was not found, try harder Johannes Schindelin via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b3fcd377652103584b6f307c6ee209980b44529f.1552472189.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).