git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 3/5] t4301: verify that merge-tree fails on missing blob objects

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Feb 22, 2024, 14:12 UTC
Message-ID
<80255493-701b-b4fc-5d0f-54e059d97f71@gmx.de>
In-Reply-To
<CAPig+cSJz3U+vT==NhX5hcrTjsCggnAzhzQOvZcSXbcEGuYaKQ@mail.gmail.com>
Hi Eric,
On Wed, 7 Feb 2024, Eric Sunshine wrote:
Show 16 quoted lines
> 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()?

Originally, there was, since I wanted the merge to succeed without conflicts.

However, I simplified the test case similar to what you suggested and now specifically verify that the expected error message is shown (as opposed to, say, an error message indicating a merge conflict).

Show 19 quoted lines
> 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) &&
I added the `\n` and now avoid the `%s`.

Thank you for your review! Johannes

Show 22 quoted lines
>
> 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
> > +'
>
Previous: Eric SunshineNext: Johannes Schindelin via GitGitGadget
Message 20 of 48 in “merge-tree: handle missing objects correctly”
  1. 0/4 merge-tree: handle missing objects correctlyJohannes Schindelin via GitGitGadget, Feb 6, 2024
  2. 1/4 merge-tree: fail with a non-zero exit code on missing tree objectsJohannes Schindelin via GitGitGadget, Feb 6, 2024
  3. Patrick SteinhardtFeb 7, 2024
  4. 2/4 merge-ort: do check `parse_tree()`'s return valueJohannes Schindelin via GitGitGadget, Feb 6, 2024
  5. 3/4 t4301: verify that merge-tree fails on missing blob objectsJohannes Schindelin via GitGitGadget, Feb 6, 2024
  6. 4/4 Always check `parse_tree*()`'s return valueJohannes Schindelin via GitGitGadget, Feb 6, 2024
  7. Junio C HamanoFeb 6, 2024
  8. Patrick SteinhardtFeb 7, 2024
  9. Junio C HamanoFeb 6, 2024
  10. Patrick SteinhardtFeb 7, 2024
  11. 0/5 merge-tree: handle missing objects correctlyJohannes Schindelin via GitGitGadget, Feb 7, 2024
  12. 1/5 merge-tree: fail with a non-zero exit code on missing tree objectsJohannes Schindelin via GitGitGadget, Feb 7, 2024
  13. Eric SunshineFeb 7, 2024
  14. Johannes SchindelinFeb 22, 2024
  15. 2/5 merge-ort: do check `parse_tree()`'s return valueJohannes Schindelin via GitGitGadget, Feb 7, 2024
  16. 3/5 t4301: verify that merge-tree fails on missing blob objectsJohannes Schindelin via GitGitGadget, Feb 7, 2024
  17. Eric SunshineFeb 7, 2024
  18. Junio C HamanoFeb 7, 2024
  19. Eric SunshineFeb 8, 2024
  20. Johannes SchindelinFeb 22, 2024
  21. 4/5 Always check `parse_tree*()`'s return valueJohannes Schindelin via GitGitGadget, Feb 7, 2024
  22. Eric SunshineFeb 7, 2024
  23. Johannes SchindelinFeb 22, 2024
  24. Junio C HamanoFeb 22, 2024
  25. 5/5 cache-tree: avoid an unnecessary checkJohannes Schindelin via GitGitGadget, Feb 7, 2024
  26. Junio C HamanoFeb 7, 2024
  27. 0/5 merge-tree: handle missing objects correctlyJohannes Schindelin via GitGitGadget, Feb 22, 2024
  28. 1/5 merge-tree: fail with a non-zero exit code on missing tree objectsJohannes Schindelin via GitGitGadget, Feb 22, 2024
  29. Junio C HamanoFeb 22, 2024
  30. 2/5 merge-ort: do check `parse_tree()`'s return valueJohannes Schindelin via GitGitGadget, Feb 22, 2024
  31. Junio C HamanoFeb 22, 2024
  32. 3/5 t4301: verify that merge-tree fails on missing blob objectsJohannes Schindelin via GitGitGadget, Feb 22, 2024
  33. Junio C HamanoFeb 22, 2024
  34. Junio C HamanoFeb 22, 2024
  35. 4/5 Always check `parse_tree*()`'s return valueJohannes Schindelin via GitGitGadget, Feb 22, 2024
  36. Junio C HamanoFeb 22, 2024
  37. Johannes SchindelinFeb 23, 2024
  38. Junio C HamanoFeb 23, 2024
  39. 5/5 cache-tree: avoid an unnecessary checkJohannes Schindelin via GitGitGadget, Feb 22, 2024
  40. Junio C HamanoFeb 22, 2024
  41. 0/6 merge-tree: handle missing objects correctlyJohannes Schindelin via GitGitGadget, Feb 23, 2024
  42. 1/6 merge-tree: fail with a non-zero exit code on missing tree objectsJohannes Schindelin via GitGitGadget, Feb 23, 2024
  43. 2/6 merge-ort: do check `parse_tree()`'s return valueJohannes Schindelin via GitGitGadget, Feb 23, 2024
  44. 3/6 t4301: verify that merge-tree fails on missing blob objectsJohannes Schindelin via GitGitGadget, Feb 23, 2024
  45. 4/6 Always check `parse_tree*()`'s return valueJohannes Schindelin via GitGitGadget, Feb 23, 2024
  46. 5/6 cache-tree: avoid an unnecessary checkJohannes Schindelin via GitGitGadget, Feb 23, 2024
  47. 6/6 fill_tree_descriptor(): mark error message for translationJohannes Schindelin via GitGitGadget, Feb 23, 2024
  48. Junio C HamanoFeb 23, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.