# A bug in git-add with GIT_DIR? 3 messages from 2018-12-20 to 2018-12-20. Participants: Orgad Shaneh, Jeff King. Thread: https://gitlist.dev/t/50073 ## Orgad Shaneh, 2018-12-20 08:37 Subject: A bug in git-add with GIT_DIR? Message-ID: URL: https://gitlist.dev/e/CAGHpTBKyBgPURYfuZgVwnskGSy9L1%2B3WMrYuPmziQ7VcGDkMcA%40mail.gmail.com ``` Hi, I played around with t5403-post-checkout-hook, and noticed that its state is not exactly what I'd expect it to be. The test setup is: echo Data for commit0. >a && echo Data for commit0. >b && git update-index --add a && git update-index --add b && tree0=$(git write-tree) && commit0=$(echo setup | git commit-tree $tree0) && git update-ref refs/heads/master $commit0 && git clone ./. clone1 && git clone ./. clone2 && GIT_DIR=clone2/.git git branch new2 && echo Data for commit1. >clone2/b && GIT_DIR=clone2/.git git add clone2/b && GIT_DIR=clone2/.git git commit -m new2 Now, the line before the last one executes git add clone2/b with GIT_DIR set. I'd expect that to add b inside clone2, but instead it adds an inexistent clone2/clone2/b, and if I stop at this line, then the status shows: On branch master Your branch is up to date with 'origin/master'. Changes to be committed: (use "git reset HEAD ..." to unstage) new file: clone2/b Changes not staged for commit: (use "git add/rm ..." to update what will be committed) (use "git checkout -- ..." to discard changes in working directory) modified: b deleted: clone2/b Is this the intended behavior? It looks like that's not what the test meant to do anyway... And if I change it to (cd clone2 && git add b), then the commits look reasonable, but step 6 fails. - Orgad ``` ## Jeff King, 2018-12-20 15:18 Subject: Re: A bug in git-add with GIT_DIR? Message-ID: <20181220151830.GD27361@sigill.intra.peff.net> URL: https://gitlist.dev/e/20181220151830.GD27361%40sigill.intra.peff.net In-Reply-To: ``` On Thu, Dec 20, 2018 at 10:37:21AM +0200, Orgad Shaneh wrote: > I played around with t5403-post-checkout-hook, and noticed that its > state is not exactly what I'd expect it to be. > > The test setup is: > echo Data for commit0. >a && > echo Data for commit0. >b && > git update-index --add a && > git update-index --add b && > tree0=$(git write-tree) && > commit0=$(echo setup | git commit-tree $tree0) && > git update-ref refs/heads/master $commit0 && > git clone ./. clone1 && > git clone ./. clone2 && > GIT_DIR=clone2/.git git branch new2 && > echo Data for commit1. >clone2/b && > GIT_DIR=clone2/.git git add clone2/b && > GIT_DIR=clone2/.git git commit -m new2 > > Now, the line before the last one executes git add clone2/b with GIT_DIR set. When GIT_DIR is set but not GIT_WORK_TREE, the current directory is taken as the working tree. So that will find clone2/b (from the current directory, which is a real file), and add an index entry with that path "clone2/b" and the sha1 of that content. But when commands are run from inside "clone2", they will naturally treat "clone2" as the working tree. And since "clone2/b" does not exist inside there, they will say "oops, it looks like this file has been deleted". > I'd expect that to add b inside clone2, but instead it adds an > inexistent clone2/clone2/b, and if I stop at this line, then the > status shows: Sort of. It never sees the path "clone2/clone2/b", but the path in the index coupled with the working tree being inside clone2 means that it would look for such a file. > On branch master > Your branch is up to date with 'origin/master'. > > Changes to be committed: > (use "git reset HEAD ..." to unstage) > > new file: clone2/b > > Changes not staged for commit: > (use "git add/rm ..." to update what will be committed) > (use "git checkout -- ..." to discard changes in working directory) > > modified: b > deleted: clone2/b > > Is this the intended behavior? It looks like that's not what the test > meant to do anyway... This is the expected behavior if you did "cd clone2 && git status". Looking at the test, I don't think it quite meant to do this. It looks like it predates "git -C", but for some reason did not want to "cd" in a subshell. I think it would be better written as: git -C clone2 add b && git -C clone2 commit -m new2 or: ( cd clone2 && git add b && git commit -m new2 ) And ditto for all of the other uses of $GIT_DIR in that script. E.g., the ones that do: GIT_DIR=clone1/.git git checkout master are likely writing the contents of clone1's master branch to the _current_ directory (not the working tree in clone1). > And if I change it to (cd clone2 && git add b), then the commits look > reasonable, but step 6 fails. You probably just need to update the other calls, too, so they all match. -Peff ``` ## Orgad Shaneh, 2018-12-20 15:21 Subject: Re: A bug in git-add with GIT_DIR? Message-ID: URL: https://gitlist.dev/e/CAGHpTBKoH9aqKKNg9k%3DmY2_AZymiga5x8%2B5jNtetVZzzqsCEPg%40mail.gmail.com In-Reply-To: <20181220151830.GD27361@sigill.intra.peff.net> ``` On Thu, Dec 20, 2018 at 5:18 PM Jeff King wrote: > > On Thu, Dec 20, 2018 at 10:37:21AM +0200, Orgad Shaneh wrote: > > > I played around with t5403-post-checkout-hook, and noticed that its > > state is not exactly what I'd expect it to be. > > > > The test setup is: > > echo Data for commit0. >a && > > echo Data for commit0. >b && > > git update-index --add a && > > git update-index --add b && > > tree0=$(git write-tree) && > > commit0=$(echo setup | git commit-tree $tree0) && > > git update-ref refs/heads/master $commit0 && > > git clone ./. clone1 && > > git clone ./. clone2 && > > GIT_DIR=clone2/.git git branch new2 && > > echo Data for commit1. >clone2/b && > > GIT_DIR=clone2/.git git add clone2/b && > > GIT_DIR=clone2/.git git commit -m new2 > > > > Now, the line before the last one executes git add clone2/b with GIT_DIR set. > > When GIT_DIR is set but not GIT_WORK_TREE, the current directory is > taken as the working tree. > > So that will find clone2/b (from the current directory, which is a real > file), and add an index entry with that path "clone2/b" and the sha1 of > that content. > > But when commands are run from inside "clone2", they will naturally > treat "clone2" as the working tree. And since "clone2/b" does not exist > inside there, they will say "oops, it looks like this file has been > deleted". > > > I'd expect that to add b inside clone2, but instead it adds an > > inexistent clone2/clone2/b, and if I stop at this line, then the > > status shows: > > Sort of. It never sees the path "clone2/clone2/b", but the path in the > index coupled with the working tree being inside clone2 means that it > would look for such a file. > > > On branch master > > Your branch is up to date with 'origin/master'. > > > > Changes to be committed: > > (use "git reset HEAD ..." to unstage) > > > > new file: clone2/b > > > > Changes not staged for commit: > > (use "git add/rm ..." to update what will be committed) > > (use "git checkout -- ..." to discard changes in working directory) > > > > modified: b > > deleted: clone2/b > > > > Is this the intended behavior? It looks like that's not what the test > > meant to do anyway... > > This is the expected behavior if you did "cd clone2 && git status". > Looking at the test, I don't think it quite meant to do this. It looks > like it predates "git -C", but for some reason did not want to "cd" in a > subshell. > > I think it would be better written as: > > git -C clone2 add b && > git -C clone2 commit -m new2 > > or: > > ( > cd clone2 && > git add b && > git commit -m new2 > ) > > And ditto for all of the other uses of $GIT_DIR in that script. E.g., > the ones that do: > > GIT_DIR=clone1/.git git checkout master > > are likely writing the contents of clone1's master branch to the > _current_ directory (not the working tree in clone1). > > > And if I change it to (cd clone2 && git add b), then the commits look > > reasonable, but step 6 fails. > > You probably just need to update the other calls, too, so they all > match. > > -Peff Thanks. I'll refactor the tests and post a patch later. - Orgad ```