Re: [PATCH (Apple Git) 03/13] t0500: New regression test for git add of a path that contains a .git directory
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 30, 2019, 13:12 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1901301355140.41@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20190129193818.8645-4-jeremyhu@apple.com>
Hi Jeremy,
On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
As Eric pointed out, commits with such a vanishing commit message are very, very sad commits. And somewhere, a kitten dies every time you submit such a commit.
Show 16 quoted lines
> +test_expect_success '<rdar://problem/10238070> -- git add of a path that contains a .git directory' '
> + rm -rf .git &&
> + mkdir -p orig/sub/dir/otherdir &&
> + cd orig/sub &&
> + echo "1" > dir/file &&
> + echo "2" > dir/otherdir/file &&
> + git init --quiet &&
> + git add -A &&
> + git commit -m "Initial Commit" --quiet &&
> + cd - > /dev/null &&
> + git init --bare --quiet "${TESTROOT}/git_dir.git" &&
> + git --git-dir="${TESTROOT}/git_dir.git" --work-tree=/ add -f -- "${TESTROOT}/orig/sub/" &&
> + git --git-dir="${TESTROOT}/git_dir.git" --work-tree=/ add -f -- "${TESTROOT}/orig/" &&
> + git --git-dir="${TESTROOT}/git_dir.git" --work-tree=/ commit -m "Commit." |
> + grep -q "2 files changed, 2 insertions"
> +'Let's try to waste some time and reverse engineer what this test is about, shall we?
So first the .git directory is removed. I really have to wonder why because we seem to do pretty much everything after that outside of that directory, so I bet that the test would do the exact same thing without mucking with that .git directory.
The some submodule with nested directories is set up (we could do this much easier by using `mkdir orig && git init orig/sub && test_commit -C orig/sub 1 && mkdir orig/sub/dir & test_commit -C orig/sub dir/2`, but let's look further before suggesting a better way to implement this).
Then a bare directory is created *somewhere*, and then the submodule as well as its parent directory is added.
Finally, a commit is created with that new index.
So is the problem that this test tries to catch that a directory containing a submodule is added together with its .git directory?
I could understand that, I would understand that you would add a regression test to catch this, but since it is added with `test_expect_success`, I would expect this regression to be fixed for a long time (and probably be committed together with a regression test that verifies the very same as your new test).
Okay, so I give up on analyzing this further and simply go back to the indicated commit introducing the regression, and applying your patch on top, to see whether it fails. Because there is nothing Apple-specific about it, I'll do this in an Ubuntu VM (because I have no Apple hardware handy, so the only way for me to debug this on macOS would be via Azure Pipelines, which is tedious and slow).
But no, this test fails with or without 18e051a3981f (setup: translate symlinks in filename when using absolute paths, 2010-12-27) reverted.
So the ball is squarely back in your court: care to explain what the haggling heck your patch is trying to achieve?
Thanks, Johannes
Show 6 quoted lines
> + > +test_done > -- > 2.20.0 (Apple Git-115) > >