Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Junio C Hamano]

Jeff King  writes:

> On Wed, Sep 21, 2016 at 07:45:50PM +0200, Kevin Daudt wrote:
>
>> On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote:
>> > Kevin Daudt  writes:
>> > 
>> > > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
>> > >> 
>> > >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
>> > >>  - mailinfo: unescape quoted-pair in header fields
>> > >>  - t5100-mailinfo: replace common path prefix with variable
>> > >
>> > > Is this good enough, or do you want me to look into the feedback from
>> > > jeff?
>> > 
>> > If you are talking about the simplified loop that deliberately sets
>> > a rule that is looser than RFC, yes, I'd like to see you at least
>> > consider the pros and cons of his approach, which looked nicer to my
>> > brief reading of it.
>> > 
>> > It is perfectly OK by me (it may not be so if you ask Peff) if you
>> > decide that your version is better after doing so, though.
>> 
>> Alright, I'll look into it.
>
> Thanks. I am OK if we do not use my simplified version, but I think
> there were some issues I noted with your last version.

Yup, even some automated tool noticed a new leak ;-)

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Jeff King]

On Wed, Sep 21, 2016 at 07:45:50PM +0200, Kevin Daudt wrote:

> On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote:
> > Kevin Daudt  writes:
> > 
> > > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
> > >> 
> > >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
> > >>  - mailinfo: unescape quoted-pair in header fields
> > >>  - t5100-mailinfo: replace common path prefix with variable
> > >
> > > Is this good enough, or do you want me to look into the feedback from
> > > jeff?
> > 
> > If you are talking about the simplified loop that deliberately sets
> > a rule that is looser than RFC, yes, I'd like to see you at least
> > consider the pros and cons of his approach, which looked nicer to my
> > brief reading of it.
> > 
> > It is perfectly OK by me (it may not be so if you ask Peff) if you
> > decide that your version is better after doing so, though.
> 
> Alright, I'll look into it.

Thanks. I am OK if we do not use my simplified version, but I think
there were some issues I noted with your last version.

-Peff

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Johannes Schindelin]

Hi Junio,

On Wed, 21 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >> * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
> >>   (merged to 'next' on 2016-08-14 at 6891bcd)
> >>  + rebase-interactive: drop early check for valid ident
> >> 
> >>  Even when "git pull --rebase=preserve" (and the underlying "git
> >>  rebase --preserve") can complete without creating any new commit
> >>  (i.e. fast-forwards), it still insisted on having a usable ident
> >>  information (read: user.email is set correctly), which was less
> >>  than nice.  As the underlying commands used inside "git rebase"
> >>  would fail with a more meaningful error message and advice text
> >>  when the bogus ident matters, this extra check was removed.
> >> 
> >>  Will hold to see if people scream.
> >>  cf. <20160729224944.ga23...@sigill.intra.peff.net>
> >
> > Let's do this.
> 
> We have already been doing it (i.e. "hold to see if people scream")
> for some time.

I meant: let's merge this to `master`.

> Does it conflict with your effort to reimplement "rebase -i" in C

I do not think so.

> to keep this in 'next'?  Do you want it to move to 'master'?  I was
> under the impression that it would not make a difference to have or not
> have this patch once your reimplementation gets merged (meaning: the
> removal of the three lines will be done by wholesale removal of
> git-rebase--interactive.sh done the endgame of your series), so...

Oh, I failed to make clear that my patch series do *not* remove
git-rebase--interactive.sh. I just barely started to work to that end.
While the speed improvements are quite noticable, the rebase--helper
command still only implements the performance-critical code paths in C.

There is quite a bit of work left to do before git-rebase--interactive.sh
can be retired:

- --root is not handled via the sequencer yet,

- --preserve-merges is not handled either [*1*],

- the shell script still sets up the state directory,

- option parsing is still all-shell,

- probably more tasks I forgot.

The good news is that these parts can be converted independently from each
other, and even by independent developers (hint, hint ;-)).

Ciao,
Dscho

Footnote *1*: I am not sure that I want to port -p to C: in my view, this
is a failed experiment, to be replaced with a design based on my Git
garden shears. I tend to think that that part should be moved to a new
shell script ("git-rebase--preserve-merges.sh"?) unless some developer
other than me feels strongly enough to put their money where their mouth
is and teach the sequencer about it.

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Kevin Daudt]

On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote:
> Kevin Daudt  writes:
> 
> > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
> >> 
> >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
> >>  - mailinfo: unescape quoted-pair in header fields
> >>  - t5100-mailinfo: replace common path prefix with variable
> >
> > Is this good enough, or do you want me to look into the feedback from
> > jeff?
> 
> If you are talking about the simplified loop that deliberately sets
> a rule that is looser than RFC, yes, I'd like to see you at least
> consider the pros and cons of his approach, which looked nicer to my
> brief reading of it.
> 
> It is perfectly OK by me (it may not be so if you ask Peff) if you
> decide that your version is better after doing so, though.
> 
> Thanks.

Alright, I'll look into it.

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Junio C Hamano]

Kevin Daudt  writes:

> On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
>> 
>> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
>>  - mailinfo: unescape quoted-pair in header fields
>>  - t5100-mailinfo: replace common path prefix with variable
>
> Is this good enough, or do you want me to look into the feedback from
> jeff?

If you are talking about the simplified loop that deliberately sets
a rule that is looser than RFC, yes, I'd like to see you at least
consider the pros and cons of his approach, which looked nicer to my
brief reading of it.

It is perfectly OK by me (it may not be so if you ask Peff) if you
decide that your version is better after doing so, though.

Thanks.

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Kevin Daudt]

On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
> 
> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
>  - mailinfo: unescape quoted-pair in header fields
>  - t5100-mailinfo: replace common path prefix with variable

Is this good enough, or do you want me to look into the feedback from
jeff?

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Junio C Hamano]

Duy Nguyen  writes:

> On Tue, Sep 20, 2016 at 6:30 AM, Junio C Hamano  wrote:
>> * nd/checkout-disambiguation (2016-09-09) 4 commits
>>  - fixup! checkout.txt: document a common case that ignores ambiguation rules
>>  - checkout: fix ambiguity check in subdir
>>  - checkout.txt: document a common case that ignores ambiguation rules
>>  - checkout: add some spaces between code and comment
>>
>>  "git checkout " does not follow the usual disambiguation
>>  rules when the  can be both a rev and a path, to allow
>>  checking out a branch 'foo' in a project that happens to have a
>>  file 'foo' in the working tree without having to disambiguate.
>>  This was poorly documented and the check was incorrect when the
>>  command was run from a subdirectory.
>>
>>  Waiting for an Ack for fixup!
>
> Oops, I didn't know (I have about 300 unread git mails in my inbox), Ack.

Thanks.

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Junio C Hamano]

Johannes Schindelin  writes:

>> * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
>>   (merged to 'next' on 2016-08-14 at 6891bcd)
>>  + rebase-interactive: drop early check for valid ident
>> 
>>  Even when "git pull --rebase=preserve" (and the underlying "git
>>  rebase --preserve") can complete without creating any new commit
>>  (i.e. fast-forwards), it still insisted on having a usable ident
>>  information (read: user.email is set correctly), which was less
>>  than nice.  As the underlying commands used inside "git rebase"
>>  would fail with a more meaningful error message and advice text
>>  when the bogus ident matters, this extra check was removed.
>> 
>>  Will hold to see if people scream.
>>  cf. <20160729224944.ga23...@sigill.intra.peff.net>
>
> Let's do this.

We have already been doing it (i.e. "hold to see if people scream")
for some time.

Does it conflict with your effort to reimplement "rebase -i" in C to
keep this in 'next'?  Do you want it to move to 'master'?  I was
under the impression that it would not make a difference to have or
not have this patch once your reimplementation gets merged (meaning:
the removal of the three lines will be done by wholesale removal of
git-rebase--interactive.sh done the endgame of your series), so...

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Duy Nguyen]

On Tue, Sep 20, 2016 at 6:30 AM, Junio C Hamano  wrote:
> * nd/checkout-disambiguation (2016-09-09) 4 commits
>  - fixup! checkout.txt: document a common case that ignores ambiguation rules
>  - checkout: fix ambiguity check in subdir
>  - checkout.txt: document a common case that ignores ambiguation rules
>  - checkout: add some spaces between code and comment
>
>  "git checkout " does not follow the usual disambiguation
>  rules when the  can be both a rev and a path, to allow
>  checking out a branch 'foo' in a project that happens to have a
>  file 'foo' in the working tree without having to disambiguate.
>  This was poorly documented and the check was incorrect when the
>  command was run from a subdirectory.
>
>  Waiting for an Ack for fixup!

Oops, I didn't know (I have about 300 unread git mails in my inbox), Ack.
-- 
Duy

Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)

[Johannes Schindelin]

Hi,

On Mon, 19 Sep 2016, Junio C Hamano wrote:

> * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
>   (merged to 'next' on 2016-08-14 at 6891bcd)
>  + rebase-interactive: drop early check for valid ident
> 
>  Even when "git pull --rebase=preserve" (and the underlying "git
>  rebase --preserve") can complete without creating any new commit
>  (i.e. fast-forwards), it still insisted on having a usable ident
>  information (read: user.email is set correctly), which was less
>  than nice.  As the underlying commands used inside "git rebase"
>  would fail with a more meaningful error message and advice text
>  when the bogus ident matters, this extra check was removed.
> 
>  Will hold to see if people scream.
>  cf. <20160729224944.ga23...@sigill.intra.peff.net>

Let's do this.

Ciao,
Dscho