Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar

2 messages, 2 authors, 2016-11-21 · open the first message on its own page

Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar

From: Junio C Hamano <hidden>
Date: 2016-11-21 19:12:46

Jeff King [off-list ref] writes:
On Mon, Nov 21, 2016 at 10:15:43AM -0800, Junio C Hamano wrote:
quoted
quoted
+	test_cmp expect actual
+'
+
Is this a recent regression?  When applied on top of 'maint' or
older, it seems to pass just fine.

    ... Goes and looks ...

Interesting.  Peff's b9605bc4f2 ("config: only read .git/config from
configured repos", 2016-09-12) is where this starts failing, which
is understandable given the code change to builtin/stripspace.c in
[2/3]. 

The analysis of the log message in [2/3] is wrong and needs
updating, though.  In the old world order it worked by accident to
call git_config() without calling setup_git_directory(); after
b9605bc4f2, that no longer is valid and is exposed as a bug.  
Yeah, I noticed that while reading the patch. My b9605bc4f2 did regress
this case, but called out the fact that "cd subdir && git stripspace"
would never have worked. So one step back, 2 steps forward, and Dscho's
patch is the right step forward.
Yes, absolutely.

I sent out a set of proposed amends, and the one for this step 1/3
runs the command inside a subdirectory to force it not to find the
.git/config file relative to its pwd, which can reveal the existing
breakage without help by b9605bc4f2 ;-) hence can be forked for
older maintenance tracks.

Re: [PATCH 1/3] rebase -i: identify problems with core.commentchar

From: Jeff King <hidden>
Date: 2016-11-21 23:38:45

On Mon, Nov 21, 2016 at 11:12:39AM -0800, Junio C Hamano wrote:
quoted
Yeah, I noticed that while reading the patch. My b9605bc4f2 did regress
this case, but called out the fact that "cd subdir && git stripspace"
would never have worked. So one step back, 2 steps forward, and Dscho's
patch is the right step forward.
Yes, absolutely.

I sent out a set of proposed amends, and the one for this step 1/3
runs the command inside a subdirectory to force it not to find the
.git/config file relative to its pwd, which can reveal the existing
breakage without help by b9605bc4f2 ;-) hence can be forked for
older maintenance tracks.
Makes sense, and your amended patch looks good.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help