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

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

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* ah/misc-message-fixes (2016-09-08) 5 commits
  (merged to 'next' on 2016-09-12 at a113aea)
 + unpack-trees: do not capitalize "working"
 + git-merge-octopus: do not capitalize "octopus"
 + git-rebase--interactive: fix English grammar
 + cat-file: put spaces around pipes in usage string
 + am: put spaces around pipe in usage string

 Message cleanup.


* bc/object-id (2016-09-07) 20 commits
  (merged to 'next' on 2016-09-15 at 34c6459)
 + builtin/reset: convert to use struct object_id
 + builtin/commit-tree: convert to struct object_id
 + builtin/am: convert to struct object_id
 + refs: add an update_ref_oid function.
 + sha1_name: convert get_sha1_mb to struct object_id
 + builtin/update-index: convert file to struct object_id
 + notes: convert init_notes to use struct object_id
 + builtin/rm: convert to use struct object_id
 + builtin/blame: convert file to use struct object_id
 + Convert read_mmblob to take struct object_id.
 + notes-merge: convert struct notes_merge_pair to struct object_id
 + builtin/checkout: convert some static functions to struct object_id
 + streaming: make stream_blob_to_fd take struct object_id
 + builtin: convert textconv_object to use struct object_id
 + builtin/cat-file: convert some static functions to struct object_id
 + builtin/cat-file: convert struct expand_data to use struct object_id
 + builtin/log: convert some static functions to use struct object_id
 + builtin/blame: convert struct origin to use struct object_id
 + builtin/apply: convert static functions to struct object_id
 + cache: convert struct cache_entry to use struct object_id

 The "unsigned char sha1[20]" to "struct object_id" conversion
 continues.  Notable changes in this round includes that ce->sha1,
 i.e. the object name recorded in the cache_entry, turns into an
 object_id.

 It had merge conflicts with a few topics in flight (Christian's
 "apply.c split", Dscho's "cat-file --filters" and Jeff Hostetler's
 "status --porcelain-v2").  Extra sets of eyes double-checking for
 mismerges are highly appreciated.


* cc/apply-am (2016-09-07) 41 commits
  (merged to 'next' on 2016-09-12 at 854edde)
 + builtin/am: use apply API in run_apply()
 + apply: learn to use a different index file
 + apply: pass apply state to build_fake_ancestor()
 + apply: refactor `git apply` option parsing
 + apply: change error_routine when silent
 + usage: add get_error_routine() and get_warn_routine()
 + usage: add set_warn_routine()
 + apply: don't print on stdout in verbosity_silent mode
 + apply: make it possible to silently apply
 + apply: use error_errno() where possible
 + apply: make some parsing functions static again
 + apply: move libified code from builtin/apply.c to apply.{c,h}
 + apply: rename and move opt constants to apply.h
 + builtin/apply: rename option parsing functions
 + builtin/apply: make create_one_file() return -1 on error
 + builtin/apply: make try_create_file() return -1 on error
 + builtin/apply: make write_out_results() return -1 on error
 + builtin/apply: make write_out_one_result() return -1 on error
 + builtin/apply: make create_file() return -1 on error
 + builtin/apply: make add_index_file() return -1 on error
 + builtin/apply: make add_conflicted_stages_file() return -1 on error
 + builtin/apply: make remove_file() return -1 on error
 + builtin/apply: make build_fake_ancestor() return -1 on error
 + builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
 + builtin/apply: make gitdiff_*() return -1 on error
 + builtin/apply: make gitdiff_*() return 1 at end of header
 + builtin/apply: make parse_traditional_patch() return -1 on error
 + builtin/apply: make apply_all_patches() return 128 or 1 on error
 + builtin/apply: move check_apply_state() to apply.c
 + builtin/apply: make check_apply_state() return -1 instead of die()ing
 + apply: make init_apply_state() return -1 instead of exit()ing
 + builtin/apply: move init_apply_state() to apply.c
 + builtin/apply: make parse_ignorewhitespace_option() return -1 instead of 
die()ing
 + builtin/apply: make parse_whitespace_option() return -1 instead of die()ing
 + builtin/apply: make parse_single_patch() return -1 on error
 + builtin/apply: make parse_chunk() return a negative integer on error
 + builtin/apply: make find_header() return -128 instead of die()ing
 + builtin/apply: read_patch_file() return -1 instead of die()ing
 + builtin/apply: make apply_patch() return -1 or -128 instead of die()ing
 + apply: move 'struct apply_state' to apply.h
 +