All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v2 0/5] merge-tree: handle missing objects correctly
Date: Wed, 07 Feb 2024 16:47:36 +0000	[thread overview]
Message-ID: <pull.1651.v2.git.1707324461.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1651.git.1707212981.gitgitgadget@gmail.com>

I recently looked into issues where git merge-tree calls returned bogus data
(in one instance returning an empty tree for non-empty merge parents). By
the time I had a look at the corresponding repository, the issue was no
longer reproducible, but a closer look at the code combined with some manual
experimenting turned up the fact that missing tree objects aren't handled as
errors by git merge-tree.

While at it, I added a commit on top that tries to catch all remaining
unchecked parse_tree() calls.

This patch series is based on js/merge-tree-3-trees because I introduced
three unchecked parse_tree() calls in that topic branch.

Changes since v1:

 * Simplified the test case, avoiding a subshell and a pipe in the process.
 * Added a patch to remove a superfluous subtree->object.parsed guard around
   a parse_tree(subtree) call.

Johannes Schindelin (5):
  merge-tree: fail with a non-zero exit code on missing tree objects
  merge-ort: do check `parse_tree()`'s return value
  t4301: verify that merge-tree fails on missing blob objects
  Always check `parse_tree*()`'s return value
  cache-tree: avoid an unnecessary check

 builtin/checkout.c               | 19 ++++++++++++++++---
 builtin/clone.c                  |  3 ++-
 builtin/commit.c                 |  3 ++-
 builtin/merge-tree.c             |  6 ++++++
 builtin/read-tree.c              |  3 ++-
 builtin/reset.c                  |  4 ++++
 cache-tree.c                     |  4 ++--
 merge-ort.c                      | 16 +++++++++++-----
 merge-recursive.c                |  3 ++-
 merge.c                          |  5 ++++-
 reset.c                          |  5 +++++
 sequencer.c                      |  4 ++++
 t/t4301-merge-tree-write-tree.sh | 24 ++++++++++++++++++++++++
 13 files changed, 84 insertions(+), 15 deletions(-)


base-commit: 5f43cf5b2e4b68386d3774bce880b0f74d801635
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1651%2Fdscho%2Fmerge-tree-and-missing-objects-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1651/dscho/merge-tree-and-missing-objects-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1651

Range-diff vs v1:

 1:  a3e8ae86114 ! 1:  01dfd66568c merge-tree: fail with a non-zero exit code on missing tree objects
     @@ merge-ort.c: static int collect_merge_info(struct merge_options *opt,
      
       ## t/t4301-merge-tree-write-tree.sh ##
      @@ t/t4301-merge-tree-write-tree.sh: test_expect_success '--merge-base with tree OIDs' '
     - 	git merge-tree --merge-base=side1^^{tree} side1^{tree} side3^{tree} >with-trees &&
       	test_cmp with-commits with-trees
       '
     + 
      +test_expect_success 'error out on missing tree objects' '
      +	git init --bare missing-tree.git &&
     -+	(
     -+		git rev-list side3 &&
     -+		git rev-parse side3^:
     -+	) | git pack-objects missing-tree.git/objects/pack/side3-tree-is-missing &&
     ++	git rev-list side3 >list &&
     ++	git rev-parse side3^: >list &&
     ++	git pack-objects missing-tree.git/objects/pack/side3-tree-is-missing <list &&
      +	side3=$(git rev-parse side3) &&
      +	test_must_fail git --git-dir=missing-tree.git merge-tree $side3^ $side3 >actual &&
      +	test_must_be_empty actual
      +'
     - 
     ++
       test_done
 2:  3e5b787fc03 = 2:  a1bbb7e06e5 merge-ort: do check `parse_tree()`'s return value
 3:  85d3e672871 = 3:  be1dadf2850 t4301: verify that merge-tree fails on missing blob objects
 4:  93abd7000b8 = 4:  ffd38ad602a Always check `parse_tree*()`'s return value
 -:  ----------- > 5:  43c04749513 cache-tree: avoid an unnecessary check

-- 
gitgitgadget

  parent reply	other threads:[~2024-02-07 16:47 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06  9:49 [PATCH 0/4] merge-tree: handle missing objects correctly Johannes Schindelin via GitGitGadget
2024-02-06  9:49 ` [PATCH 1/4] merge-tree: fail with a non-zero exit code on missing tree objects Johannes Schindelin via GitGitGadget
2024-02-07  7:42   ` Patrick Steinhardt
2024-02-06  9:49 ` [PATCH 2/4] merge-ort: do check `parse_tree()`'s return value Johannes Schindelin via GitGitGadget
2024-02-06  9:49 ` [PATCH 3/4] t4301: verify that merge-tree fails on missing blob objects Johannes Schindelin via GitGitGadget
2024-02-06  9:49 ` [PATCH 4/4] Always check `parse_tree*()`'s return value Johannes Schindelin via GitGitGadget
2024-02-06 22:07   ` Junio C Hamano
2024-02-07  7:42   ` Patrick Steinhardt
2024-02-06 21:12 ` [PATCH 0/4] merge-tree: handle missing objects correctly Junio C Hamano
2024-02-07  7:42 ` Patrick Steinhardt
2024-02-07 16:47 ` Johannes Schindelin via GitGitGadget [this message]
2024-02-07 16:47   ` [PATCH v2 1/5] merge-tree: fail with a non-zero exit code on missing tree objects Johannes Schindelin via GitGitGadget
2024-02-07 17:02     ` Eric Sunshine
2024-02-22 14:09       ` Johannes Schindelin
2024-02-07 16:47   ` [PATCH v2 2/5] merge-ort: do check `parse_tree()`'s return value Johannes Schindelin via GitGitGadget
2024-02-07 16:47   ` [PATCH v2 3/5] t4301: verify that merge-tree fails on missing blob objects Johannes Schindelin via GitGitGadget
2024-02-07 17:24     ` Eric Sunshine
2024-02-07 21:24       ` Junio C Hamano
2024-02-08  0:52       ` Eric Sunshine
2024-02-22 14:12       ` Johannes Schindelin
2024-02-07 16:47   ` [PATCH v2 4/5] Always check `parse_tree*()`'s return value Johannes Schindelin via GitGitGadget
2024-02-07 17:26     ` Eric Sunshine
2024-02-22 14:08       ` Johannes Schindelin
2024-02-22 17:05         ` Junio C Hamano
2024-02-07 16:47   ` [PATCH v2 5/5] cache-tree: avoid an unnecessary check Johannes Schindelin via GitGitGadget
2024-02-07 16:58     ` Junio C Hamano
2024-02-22 14:36   ` [PATCH v3 0/5] merge-tree: handle missing objects correctly Johannes Schindelin via GitGitGadget
2024-02-22 14:36     ` [PATCH v3 1/5] merge-tree: fail with a non-zero exit code on missing tree objects Johannes Schindelin via GitGitGadget
2024-02-22 17:13       ` Junio C Hamano
2024-02-22 14:36     ` [PATCH v3 2/5] merge-ort: do check `parse_tree()`'s return value Johannes Schindelin via GitGitGadget
2024-02-22 17:18       ` Junio C Hamano
2024-02-22 14:36     ` [PATCH v3 3/5] t4301: verify that merge-tree fails on missing blob objects Johannes Schindelin via GitGitGadget
2024-02-22 17:27       ` Junio C Hamano
2024-02-22 18:23         ` Junio C Hamano
2024-02-22 14:36     ` [PATCH v3 4/5] Always check `parse_tree*()`'s return value Johannes Schindelin via GitGitGadget
2024-02-22 17:58       ` Junio C Hamano
2024-02-23  8:33         ` Johannes Schindelin
2024-02-23 17:17           ` Junio C Hamano
2024-02-22 14:36     ` [PATCH v3 5/5] cache-tree: avoid an unnecessary check Johannes Schindelin via GitGitGadget
2024-02-22 18:00       ` Junio C Hamano
2024-02-23  8:34     ` [PATCH v4 0/6] merge-tree: handle missing objects correctly Johannes Schindelin via GitGitGadget
2024-02-23  8:34       ` [PATCH v4 1/6] merge-tree: fail with a non-zero exit code on missing tree objects Johannes Schindelin via GitGitGadget
2024-02-23  8:34       ` [PATCH v4 2/6] merge-ort: do check `parse_tree()`'s return value Johannes Schindelin via GitGitGadget
2024-02-23  8:34       ` [PATCH v4 3/6] t4301: verify that merge-tree fails on missing blob objects Johannes Schindelin via GitGitGadget
2024-02-23  8:34       ` [PATCH v4 4/6] Always check `parse_tree*()`'s return value Johannes Schindelin via GitGitGadget
2024-02-23  8:34       ` [PATCH v4 5/6] cache-tree: avoid an unnecessary check Johannes Schindelin via GitGitGadget
2024-02-23  8:34       ` [PATCH v4 6/6] fill_tree_descriptor(): mark error message for translation Johannes Schindelin via GitGitGadget
2024-02-23 18:23       ` [PATCH v4 0/6] merge-tree: handle missing objects correctly Junio C Hamano

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

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

  git send-email \
    --in-reply-to=pull.1651.v2.git.1707324461.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=ps@pks.im \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.