Hi Jeremy,
On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@xxxxxxxxx>
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.
> +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
> +
> +test_done
> --
> 2.20.0 (Apple Git-115)
>
>
