Re: [PATCH v2 1/5] merge-tree: fail with a non-zero exit code on missing tree objects

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]

Hi Eric,

On Wed, 7 Feb 2024, Eric Sunshine wrote:

> On Wed, Feb 7, 2024 at 11:48 AM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> > When `git merge-tree` encounters a missing tree object, it should error
> > out and not continue quietly as if nothing had happened.
> >
> > However, as of time of writing, `git merge-tree` _does_ continue, and
> > then offers the empty tree as result.
> >
> > Let's fix this.
> >
> > 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
> > @@ -951,4 +951,14 @@ test_expect_success '--merge-base with tree OIDs' '
> > +test_expect_success 'error out on missing tree objects' '
> > +       git init --bare missing-tree.git &&
> > +       git rev-list side3 >list &&
> > +       git rev-parse side3^: >list &&
>
> Isn't the git-rev-parse invocation simply overwriting "list" rather
> than appending to it? Did you mean:
>
>     git rev-list side3 >list &&
>     git rev-parse side3^: >>list &&

Yes. And the fact that this test case still passed was for the _wrong_
reason! I adjusted the test case by writing the correct contents to `list`
and by verifying that the expected error message is shown.

>
> An alternative would be:
>
>     {
>         git rev-list side3 &&
>         git rev-parse side3^:
>     } >list &&

I find that uglier and longer, so I'll stay with the first option ;-)

Thank you for your review!
Johannes

>
> > +       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
> > +'
>