git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>,
	 Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v2 3/5] t4301: verify that merge-tree fails on missing blob objects
Date: Wed, 7 Feb 2024 12:24:59 -0500	[thread overview]
Message-ID: <CAPig+cSJz3U+vT==NhX5hcrTjsCggnAzhzQOvZcSXbcEGuYaKQ@mail.gmail.com> (raw)
In-Reply-To: <be1dadf28502fe3e9662fa61523e8c57ce3352f1.1707324462.git.gitgitgadget@gmail.com>

On Wed, Feb 7, 2024 at 11:48 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> We just fixed a problem where `merge-tree` would not fail on missing
> tree objects. Let's ensure that that problem does not occur with blob
> objects (and won't, in the future, either).
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
> @@ -961,4 +961,18 @@ test_expect_success 'error out on missing tree objects' '
> +test_expect_success 'error out on missing blob objects' '
> +       seq1=$(test_seq 1 10 | git hash-object -w --stdin) &&
> +       seq2=$(test_seq 1 11 | git hash-object -w --stdin) &&
> +       seq3=$(test_seq 0 10 | git hash-object -w --stdin) &&

Is there significance to the ranges passed to test_seq()? Or, can the
same be achieved by using arbitrary content for each blob?

    blob1=$(echo "one" | git hash-object -w --stdin) &&
    blob2=$(echo "two" | git hash-object -w --stdin) &&
    blob3=$(echo "three" | git hash-object -w --stdin) &&

> +       tree1=$(printf "100644 blob %s\tsequence" $seq1 | git mktree) &&
> +       tree2=$(printf "100644 blob %s\tsequence" $seq2 | git mktree) &&
> +       tree3=$(printf "100644 blob %s\tsequence" $seq3 | git mktree) &&

I found the lack of terminating "\n" in the `printf` confusing,
especially since the variable names (seq1, seq2, etc.) and the use of
`printf` seem to imply, at first glance, that each git-mktree
invocation is receiving multiple lines as input which, it turns out,
is not the case. Adding the missing "\n" would help:

   tree1=$(printf "100644 blob %s\tsequence\n" $seq1 | git mktree) &&
   tree2=$(printf "100644 blob %s\tsequence\n" $seq2 | git mktree) &&
   tree3=$(printf "100644 blob %s\tsequence\n" $seq3 | git mktree) &&

Interpolating the $seqN variable directly into the string rather than
using %s would make it even clearer that only a single line is being
generated as input to git-mktree:

   tree1=$(printf "100644 blob $seq1\tsequence\n" | git mktree) &&
   tree2=$(printf "100644 blob $seq2\tsequence\n" | git mktree) &&
   tree3=$(printf "100644 blob $seq3\tsequence\n" | git mktree) &&

Alternatively `echo` could be used, though it's not necessarily any nicer:

    tree1=$(echo "100644 blob $seq1Qsequence" | q_to_tab | git mktree) &&
    tree2=$(echo "100644 blob $seq2Qsequence" | q_to_tab | git mktree) &&
    tree3=$(echo "100644 blob $seq3Qsequence" | q_to_tab | git mktree) &&

> +       git init --bare missing-blob.git &&
> +       test_write_lines $seq1 $seq3 $tree1 $tree2 $tree3 |
> +       git pack-objects missing-blob.git/objects/pack/side1-whatever-is-missing &&
> +       test_must_fail git --git-dir=missing-blob.git merge-tree --merge-base=$tree1 $tree2 $tree3 >actual &&
> +       test_must_be_empty actual
> +'


  reply	other threads:[~2024-02-07 17:25 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 ` [PATCH v2 0/5] " Johannes Schindelin via GitGitGadget
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 [this message]
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

  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='CAPig+cSJz3U+vT==NhX5hcrTjsCggnAzhzQOvZcSXbcEGuYaKQ@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --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.
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).