Re: [BUG] mv: can trigger assertion failure with three parameters (builtin/mv.c:481)

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 5, 2024, 18:52 UTC
Message-ID: <xmqqil47obnw.fsf@gitster.g>
In-Reply-To: <d1f739fe-b28e-451f-9e01-3d2e24a0fe0d@app.fastmail.com>

"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

Show 8 quoted lines

> You can trigger an assertion by giving these arguments to `git mv`:
>
>     <dir>/file <dir> <other dir>
> ...
>> What did you expect to happen? (Expected behavior)
>
> A normal error message if the command is nonsensical (I don’t know; that’s
> not the point). Also `.git/index.lock` to be cleaned up.

Good find.

Not just that, but when the command fails in the middle like this, it leaves the working tree in a half-updated state, i.e.

> ./bin-wrappers/git -C $dir mv $dir/a/a.txt $dir/a $dir/b

will first move a/a.txt to b/a.txt, then try to move a (actually, all contents of it, including a/a.txt) to b/a and finds that "the command is nonsensical" and aborts, and by that time, there is no a/a.txt (i.e. the working tree has been modified). The failure should be made atomic, just like "git switch" to another branch may stop _without_ touching anything in the working tree when it may have to fail (e.g., due to a file being dirty).

Thanks for reporting, Kristoffer.

Any takers?

$ git shortlog --since=3.years -s -n -e --no-merges v2.43.0 builtin/mv.c
    15	Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
    10	Elijah Newren <newren@gmail.com>
     5	Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     2	Junio C Hamano <gitster@pobox.com>
     1	Andrzej Hunt <ajrhunt@google.com>
     1	Calvin Wan <calvinwan@google.com>
     1	Derrick Stolee <stolee@gmail.com>
     1	Sebastian Thiel <sebastian.thiel@icloud.com>
     1	Torsten Bögershausen <tboegi@web.de>