Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup
On Fri, Sep 9, 2016 at 4:22 AM, Jeff King wrote: > If you're curious what the fix looks like, it's in: > > https://github.com/peff/git jk/config-repo-setup > > The actual fix is in the final patch, but it needed a lot of preparatory > work to avoid breaking various programs that made bad assumptions (and > in the process, I uncovered a ton of other minor bugs). > > This is just a preview in case you're interested, for two reasons: > > 1. I literally _just_ put the finishing touches on it, and it's > extensive and tricky enough that I really should give it one more > proofread. > > 2. There may be other related fallouts from the bug related to running > "git init /path/to/foo" when "/path/to/foo" already exists (and in > that case we _do_ want to read its config, but not the config from > an existing repository). This may all just work fine, but I need to > think about some tests. > > -Peff I looked over the series and it all seems like solid improvements to me, and I agree with the reasoning and logic. It's nice to also read very descriptive commit messages that clearly explain each improvement. Regards, Jake
Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup
On Fri, Sep 09, 2016 at 05:32:21PM +0700, Duy Nguyen wrote: > On Fri, Sep 9, 2016 at 3:02 AM, Jeff King wrote: > > On Thu, Sep 08, 2016 at 08:47:18PM +0700, Nguyễn Thái Ngọc Duy wrote: > > > >> git-init somehow reads '.git/config' at current directory and sets > >> log_all_ref_updates based on this file. Because log_all_ref_updates is > >> not unspecified (-1) any more. It will not be written to the new repo's > >> config file (see create_default_files() function). > >> > >> This will affect our tests in the next patch as we will compare the > >> config file and expect that core.logallrefupdates is already set to true > >> by "git init main-worktree". > > > > This is a bug for more than worktrees, and is something I'm working on > > fixing > > Great! test_expect_failure it is. But I'll make a separate patch, > independent from this series though. If you're curious what the fix looks like, it's in: https://github.com/peff/git jk/config-repo-setup The actual fix is in the final patch, but it needed a lot of preparatory work to avoid breaking various programs that made bad assumptions (and in the process, I uncovered a ton of other minor bugs). This is just a preview in case you're interested, for two reasons: 1. I literally _just_ put the finishing touches on it, and it's extensive and tricky enough that I really should give it one more proofread. 2. There may be other related fallouts from the bug related to running "git init /path/to/foo" when "/path/to/foo" already exists (and in that case we _do_ want to read its config, but not the config from an existing repository). This may all just work fine, but I need to think about some tests. -Peff
Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup
On Fri, Sep 9, 2016 at 3:02 AM, Jeff King wrote: > On Thu, Sep 08, 2016 at 08:47:18PM +0700, Nguyễn Thái Ngọc Duy wrote: > >> git-init somehow reads '.git/config' at current directory and sets >> log_all_ref_updates based on this file. Because log_all_ref_updates is >> not unspecified (-1) any more. It will not be written to the new repo's >> config file (see create_default_files() function). >> >> This will affect our tests in the next patch as we will compare the >> config file and expect that core.logallrefupdates is already set to true >> by "git init main-worktree". > > This is a bug for more than worktrees, and is something I'm working on > fixing Great! test_expect_failure it is. But I'll make a separate patch, independent from this series though. -- Duy
Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup
On Thu, Sep 08, 2016 at 08:47:18PM +0700, Nguyễn Thái Ngọc Duy wrote: > git-init somehow reads '.git/config' at current directory and sets > log_all_ref_updates based on this file. Because log_all_ref_updates is > not unspecified (-1) any more. It will not be written to the new repo's > config file (see create_default_files() function). > > This will affect our tests in the next patch as we will compare the > config file and expect that core.logallrefupdates is already set to true > by "git init main-worktree". This is a bug for more than worktrees, and is something I'm working on fixing (what I'd like to do is kill off the lazy fallback to ".git/" as the repo name, which is almost always the wrong thing to do). I'm not opposed to your workaround, but just FYI. -Peff
Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup
Nguyễn Thái Ngọc Duy writes: > git-init somehow reads '.git/config' at current directory and sets > log_all_ref_updates based on this file. Because log_all_ref_updates is > not unspecified (-1) any more. It will not be written to the new repo's > config file (see create_default_files() function). I vaguely recall this was discussed recently somewhere. Is this related to <20160902091130.jxckdeomw35ee...@sigill.intra.peff.net>? > This will affect our tests in the next patch as we will compare the > config file and expect that core.logallrefupdates is already set to true > by "git init main-worktree". Ugh. The "mv" workaround is fine to demonstrate that you have fixed one of two problems, but as you said, it would be nice to have another that expects failure until both of them gets fixed. > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > t/t0001-init.sh | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > index d64e5e3..393c940 100755 > --- a/t/t0001-init.sh > +++ b/t/t0001-init.sh > @@ -385,7 +385,9 @@ test_expect_success MINGW 'bare git dir not hidden' ' > ' > > test_expect_success 're-init from a linked worktree' ' > + mv .git/config work-around-init-reading-wrong-file && > git init main-worktree && > + mv work-around-init-reading-wrong-file .git/config && > ( > cd main-worktree && > test_commit first &&
[PATCH 2/3] t0001: work around the bug that reads config file before repo setup
git-init somehow reads '.git/config' at current directory and sets log_all_ref_updates based on this file. Because log_all_ref_updates is not unspecified (-1) any more. It will not be written to the new repo's config file (see create_default_files() function). This will affect our tests in the next patch as we will compare the config file and expect that core.logallrefupdates is already set to true by "git init main-worktree". Signed-off-by: Nguyễn Thái Ngọc Duy --- t/t0001-init.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/t0001-init.sh b/t/t0001-init.sh index d64e5e3..393c940 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -385,7 +385,9 @@ test_expect_success MINGW 'bare git dir not hidden' ' ' test_expect_success 're-init from a linked worktree' ' + mv .git/config work-around-init-reading-wrong-file && git init main-worktree && + mv work-around-init-reading-wrong-file .git/config && ( cd main-worktree && test_commit first && -- 2.8.2.524.g6ff3d78
