Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Jeff King
On Tue, Nov 22, 2016 at 01:55:07PM -0800, Junio C Hamano wrote: > > And this test makes sense. Even without "sub", it would show the > > regression, but it's a good idea to test the sub-directory case to cover > > the path-munging. > > Yup. Obviously during my initial attempt I was scratching

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Junio C Hamano
Jeff King writes: >> +test_expect_success 'mailinfo with mailinfo.scissors config' ' >> +test_config mailinfo.scissors true && >> +( >> +mkdir sub && >> +cd sub && >> +git mailinfo ../msg0014.sc ../patch0014.sc <../0014 >>

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Jeff King
On Tue, Nov 22, 2016 at 01:22:24PM -0800, Junio C Hamano wrote: > OK. The "mailinfo" part turns out a bit more than RUN_SETUP_GENTLY > as it takes paths from the command line that needs to be adjusted > for the prefix. I wondered at first if our lack of parse-options here was holding us back

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Jeff King
On Tue, Nov 22, 2016 at 04:19:20PM -0500, Jeff King wrote: > > > Do you want to do another round of -rc3? Ship with the > > > minor regressions and fix them up in v2.11.1? > > > > I am leaning towards the former (though we may also end up doing the > > latter). > > I think I'd lead towards

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Junio C Hamano
Jeff King writes: > On Tue, Nov 22, 2016 at 12:24:15PM -0800, Junio C Hamano wrote: > >> > Do you want to do another round of -rc3? Ship with the >> > minor regressions and fix them up in v2.11.1? >> >> I am leaning towards the former (though we may also end up doing the >>

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Jeff King
On Tue, Nov 22, 2016 at 12:24:15PM -0800, Junio C Hamano wrote: > > Do you want to do another round of -rc3? Ship with the > > minor regressions and fix them up in v2.11.1? > > I am leaning towards the former (though we may also end up doing the > latter). I think I'd lead towards -rc3, as

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Junio C Hamano
Jeff King writes: > So what do you want to do for v2.11? I think there are minor regressions > in stripspace, archive, and mailinfo with respect to reading > .git/config. The right solution in each case is to do a gentle repo > setup. We have patches for stripspace already, but

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Jeff King
On Tue, Nov 22, 2016 at 11:10:25AM -0800, Junio C Hamano wrote: > Archive & Upload-archive: > > "cd Documentation && git archive --remote=origin" immediately hits > "BUG: setup_git_env called without repository" if your Git is built > with b1ef400eec ("setup_git_env: avoid blind fall-back to

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Junio C Hamano
Junio C Hamano writes: > I think we want to audit the ones without RUN_SETUP* in the command > table in order to hunt for regression aka "a fix revealed a bug that > was covered by .git/config accidentally getting read when run from > the top-level of the working tree",

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Junio C Hamano
Johannes Schindelin writes: >> This conditional config file reading is a trap for similar bugs to >> happen again. Is there any reason we should not just mark the command >> RUN_SETUP_GENTLY in git.c and call git_config() here unconditionally? > > As I plan to slip

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Johannes Schindelin
Hi Duy, On Tue, 22 Nov 2016, Duy Nguyen wrote: > On Mon, Nov 21, 2016 at 9:18 PM, Johannes Schindelin > wrote: > > When eff80a9 (Allow custom "comment char", 2013-01-16) taught the > > `stripspace` command to respect the config setting `core.commentChar`, > > it

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Johannes Schindelin
Hi Junio, On Mon, 21 Nov 2016, Junio C Hamano wrote: > From: Johannes Schindelin > > The way "git stripspace" reads the configuration was not quite > correct, in that it forgot to probe for a possibly existing > repository (note: stripspace is designed to be usable

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-22 Thread Duy Nguyen
On Mon, Nov 21, 2016 at 9:18 PM, Johannes Schindelin wrote: > When eff80a9 (Allow custom "comment char", 2013-01-16) taught the > `stripspace` command to respect the config setting `core.commentChar`, > it forgot that this variable may be defined in .git/config. > > So

Re: [PATCH 2/3] stripspace: respect repository config

2016-11-21 Thread Junio C Hamano
Junio C Hamano writes: > From: Johannes Schindelin > > The way "git stripspace" reads the configuration was not quite > correct, in that it forgot to probe for a possibly existing > repository (note: stripspace is designed to be usable outside the

[PATCH 2/3] stripspace: respect repository config

2016-11-21 Thread Junio C Hamano
From: Johannes Schindelin The way "git stripspace" reads the configuration was not quite correct, in that it forgot to probe for a possibly existing repository (note: stripspace is designed to be usable outside the repository as well) before doing so. Due to this,

[PATCH 2/3] stripspace: respect repository config

2016-11-21 Thread Johannes Schindelin
When eff80a9 (Allow custom "comment char", 2013-01-16) taught the `stripspace` command to respect the config setting `core.commentChar`, it forgot that this variable may be defined in .git/config. So when rebasing interactively with a commentChar defined in the current repository's config, the