Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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 +