* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
@ 2018-06-28 22:42 ` Stefan Beller
2018-06-29 13:55 ` Derrick Stolee
2018-06-29 19:51 ` Junio C Hamano
2018-06-29 11:10 ` ag/rebase-i-append-todo-help, was " Alban Gruin
` (8 subsequent siblings)
9 siblings, 2 replies; 24+ messages in thread
From: Stefan Beller @ 2018-06-28 22:42 UTC (permalink / raw)
To: Junio C Hamano, Jonathan Tan; +Cc: git
On Thu, Jun 28, 2018 at 2:40 PM Junio C Hamano <gitster@pobox.com> wrote:
> The tip of 'next' has been rewound and it currently has only 4
> topics. Quite a many topics are cooking in 'pu' and need to be
> sifted into good bins (for 'next') and the remainder. Help is
> appreciated in that area ;-)
Which branches are needing review most? Or should I start
commenting on random stuff? ;-)
>
> * bw/ref-in-want (2018-06-28) 8 commits
> - fetch-pack: implement ref-in-want
> - fetch-pack: put shallow info in output parameter
> - fetch: refactor to make function args narrower
> - fetch: refactor fetch_refs into two functions
> - fetch: refactor the population of peer ref OIDs
> - upload-pack: test negotiation with changing repository
> - upload-pack: implement ref-in-want
> - test-pkt-line: add unpack-sideband subcommand
>
> Protocol v2 has been updated to allow slightly out-of-sync set of
> servers to work together to serve a single client, which would be
> useful with load-balanced servers that talk smart HTTP transport.
>
I think another selling point, which should be emphasized more is
the ease of ACL checking on the server side.
Even when the read permissions are per repository (e.g. githubs
model AFAICT), the serving side still has to do a lookup "Can
a wanted sha1 be reached from all refs?", which now can be
optimized away ("Can I access the master branch?") and it
is cheaper to run the server.
Specifically if read permissions are per ref (Gerrits model), I'd
expect some CPU savings on the serving side.
> * jt/remove-pack-bitmap-global (2018-06-21) 2 commits
> - pack-bitmap: add free function
> - pack-bitmap: remove bitmap_git global variable
>
> The effort to move globals to per-repository in-core structure
> continues.
This is mostly done, though Peff seems to expect a reroll with
clarification on how the series is structured?
https://public-inbox.org/git/20180611211033.GB26235@sigill.intra.peff.net/
> * sb/submodule-move-head-error-msg (2018-06-25) 1 commit
> - submodule.c: report the submodule that an error occurs in
>
> Needs a reroll.
> cf. <20180622081713.5360-1-szeder.dev@gmail.com>
https://public-inbox.org/git/xmqqmuviq2n7.fsf@gitster-ct.c.googlers.com/
suggests that you applied that change and a reroll would not be needed.
>
> * ds/multi-pack-index (2018-06-25) 24 commits
> - midx: clear midx on repack
> - packfile: skip loading index if in multi-pack-index
> - midx: prevent duplicate packfile loads
> - midx: use midx in approximate_object_count
> - midx: use existing midx when writing new one
> - midx: use midx in abbreviation calculations
> - midx: read objects from multi-pack-index
> - midx: prepare midxed_git struct
> - config: create core.multiPackIndex setting
> - midx: write object offsets
> - midx: write object id fanout chunk
> - midx: write object ids in a chunk
> - midx: sort and deduplicate objects from packfiles
> - midx: read pack names into array
> - multi-pack-index: write pack names in chunk
> - multi-pack-index: read packfile list
> - packfile: generalize pack directory list
> - multi-pack-index: expand test data
> - multi-pack-index: load into memory
> - midx: write header information to lockfile
> - multi-pack-index: add 'write' verb
> - multi-pack-index: add builtin
> - multi-pack-index: add format details
> - multi-pack-index: add design document
>
> When there are too many packfiles in a repository (which is not
> recommended), looking up an object in these would require
> consulting many pack .idx files; a new mechanism to have a single
> file that consolidates all of these .idx files is introduced.
>
>
> * nd/use-the-index-compat-less (2018-06-25) 13 commits
> - wt-status.c: stop using index compat macros
> - sha1-name.c: stop using index compat macros
> - sequencer.c: stop using index compat macros
> - revision.c: stop using index compat macros
> - rerere.c: stop using index compat macros
> - merge.c: stop using index compat macros
> - merge-recursive.c: stop using index compat macros
> - entry.c: stop using index compat macros
> - diff.c: stop using index compat macros
> - diff-lib.c: stop using index compat macros
> - check-racy.c: stop using index compat macros
> - blame.c: stop using index compat macros
> - apply.c: stop using index compat macros
>
> Use of many convenience functions that operate on the_index
> "primary in-core index instance" have been rewritten to explicitly
> call the coutnerpart functions that take arbitrary index_state and
> pass the_index.
>
> I'd say that alone is a useless code churn, but people seem to be
> excited about the change, so it is queued here.
As someone who also does lots of refactoring lately, I am excited
to see the code health moving in the right direction.
It is easy to quantify how often we are bitten by code churn
(that you call useless here); and very hard to quantify bugs
introduced or features not written/upstreamed due to clunky
API (as we don't see those or do not attribute a bug to a bad API).
So we are naturally biased towards seeing code churn
as bad, as the signals favor one over the other.
>
> * jt/fetch-pack-negotiator (2018-06-15) 7 commits
> - fetch-pack: introduce negotiator API
> - fetch-pack: move common check and marking together
> - fetch-pack: make negotiation-related vars local
> - fetch-pack: use ref adv. to prune "have" sent
> - fetch-pack: directly end negotiation if ACK ready
> - fetch-pack: clear marks before re-marking
> - fetch-pack: split up everything_local()
> (this branch is used by jt/fetch-nego-tip.)
>
> Code restructuring and a small fix to transport protocol v2 during
> fetching.
>
> Is this ready for 'next'?
That is a good topic to review for me, I'll jump on it.
> * ds/commit-graph-fsck (2018-06-27) 22 commits
[...]
>
> "git fsck" learns to make sure the optional commit-graph file is in
> a sane state.
>
> Is this ready for 'next'?
I hope so, as I plan to reroll the next object store series based
on it. I'll also review that.
>
> * jm/cache-entry-from-mem-pool (2018-06-28) 8 commits
> - block alloc: add validations around cache_entry lifecyle
> - block alloc: allocate cache entries from mem-pool
> - mem-pool: fill out functionality
> - mem-pool: add life cycle management functions
> - mem-pool: only search head block for available space
> - block alloc: add lifecycle APIs for cache_entry structs
> - read-cache: make_cache_entry should take object_id struct
> - read-cache: teach refresh_cache_entry() to take istate
>
> For a large tree, the index needs to hold many cache entries
> allocated on heap. These cache entries are now allocated out of a
> dedicated memory pool to amortize malloc(3) overhead.
This is also on my review todo list, still.
>
> * sb/diff-color-move-more (2018-06-25) 11 commits
> - diff: fix a sparse 'dubious one-bit signed bitfield' error
> - SQUASH??? t/4015 GETTEXT_POISON emergency fix
> - SQUASH????? Documentation breakage emergency fix
[...]
>
> "git diff --color-moved" feature has further been tweaked.
>
> Needs to be cleaned-up with various fix-up bits applied inline.
I'll resend with those squashes and another (test-)fix SZEDER
mentioned soon.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 22:42 ` Stefan Beller
@ 2018-06-29 13:55 ` Derrick Stolee
2018-06-29 19:51 ` Junio C Hamano
1 sibling, 0 replies; 24+ messages in thread
From: Derrick Stolee @ 2018-06-29 13:55 UTC (permalink / raw)
To: Stefan Beller, Junio C Hamano, Jonathan Tan; +Cc: git
On 6/28/2018 6:42 PM, Stefan Beller wrote:
> On Thu, Jun 28, 2018 at 2:40 PM Junio C Hamano <gitster@pobox.com> wrote:
>
>> * ds/commit-graph-fsck (2018-06-27) 22 commits
> [...]
>> "git fsck" learns to make sure the optional commit-graph file is in
>> a sane state.
>>
>> Is this ready for 'next'?
> I hope so, as I plan to reroll the next object store series based
> on it. I'll also review that.
I think the series is ready. The only new feedback in a while is your
style comments on "commit-graph: add '--reachable' option", which can
hopefully be squashed in. I'm also open to sending a new series on top
for new feedback.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 22:42 ` Stefan Beller
2018-06-29 13:55 ` Derrick Stolee
@ 2018-06-29 19:51 ` Junio C Hamano
1 sibling, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2018-06-29 19:51 UTC (permalink / raw)
To: Stefan Beller; +Cc: Jonathan Tan, git
Stefan Beller <sbeller@google.com> writes:
>> * jt/remove-pack-bitmap-global (2018-06-21) 2 commits
>> - pack-bitmap: add free function
>> - pack-bitmap: remove bitmap_git global variable
>>
>> The effort to move globals to per-repository in-core structure
>> continues.
>
> This is mostly done, though Peff seems to expect a reroll with
> clarification on how the series is structured?
> https://public-inbox.org/git/20180611211033.GB26235@sigill.intra.peff.net/
That one has been resolved by squashing the updated log message in,
I think, so we should be able to merge it down.
>> * sb/submodule-move-head-error-msg (2018-06-25) 1 commit
>> - submodule.c: report the submodule that an error occurs in
>>
>> Needs a reroll.
>> cf. <20180622081713.5360-1-szeder.dev@gmail.com>
>
> https://public-inbox.org/git/xmqqmuviq2n7.fsf@gitster-ct.c.googlers.com/
>
> suggests that you applied that change and a reroll would not be needed.
Yup, I forgot about that one. Thanks.
> It is easy to quantify how often we are bitten by code churn
> (that you call useless here); and very hard to quantify bugs
By definition, churn is useless. Useful ones are refactoring ;-).
And when you do want to operate on _the_ single in-core instance,
not having to say &the_index in the argument and use $foo_cache()
function does *not* become a source of "bugs". It just saves
typing, and turning it to $foo_index(&the_index,...) does *not* make
it less error prone.
>> * sb/diff-color-move-more (2018-06-25) 11 commits
>> - diff: fix a sparse 'dubious one-bit signed bitfield' error
>> - SQUASH??? t/4015 GETTEXT_POISON emergency fix
>> - SQUASH????? Documentation breakage emergency fix
> [...]
>>
>> "git diff --color-moved" feature has further been tweaked.
>>
>> Needs to be cleaned-up with various fix-up bits applied inline.
>
> I'll resend with those squashes and another (test-)fix SZEDER
> mentioned soon.
The interdiff of the topic looked alright. Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* ag/rebase-i-append-todo-help, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
2018-06-28 22:42 ` Stefan Beller
@ 2018-06-29 11:10 ` Alban Gruin
2018-06-29 21:04 ` Ævar Arnfjörð Bjarmason
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Alban Gruin @ 2018-06-29 11:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi Junio,
Le 28/06/2018 à 23:40, Junio C Hamano a écrit :
> * ag/rebase-i-append-todo-help (2018-06-14) 2 commits
> - rebase--interactive: rewrite append_todo_help() in C
> - Merge branch 'ag/rebase-p' into ag/rebase-i-append-todo-help
> (this branch is used by ag/rebase-i-rewrite-todo.)
>
> Stepwise rewriting of the machinery of "rebase -i" into C continues.
>
> Needs a reroll.
> cf. <nycvar.QRO.7.76.6.1806261125330.21419@tvgsbejvaqbjf.bet>
>
>
As I moved append_todo_help() to `rebase-interactive.c`, it should be
because I did not changed `msg = _(…)` to `msg = N_(…)`.
It was pointed out on IRC that it was not necessary, after all[0].
Basically, N_() is needed for static variables, not for "dynamic"
variables like `msg` in append_todo_help().
[0] http://colabti.org/irclogger/irclogger_log/git-devel?date=2018-06-26#l44
Cheers,
Alban
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
2018-06-28 22:42 ` Stefan Beller
2018-06-29 11:10 ` ag/rebase-i-append-todo-help, was " Alban Gruin
@ 2018-06-29 21:04 ` Ævar Arnfjörð Bjarmason
2018-07-03 11:44 ` as/safecrlf-quiet-fix, was " Johannes Schindelin
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-06-29 21:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Thu, Jun 28 2018, Junio C Hamano wrote:
> The tip of 'next' has been rewound and it currently has only 4
> topics. Quite a many topics are cooking in 'pu' and need to be
> sifted into good bins (for 'next') and the remainder. Help is
> appreciated in that area ;-)
Per my
https://public-inbox.org/git/CACBZZX4yG5h5kk4NFQz_NzAweMa+Nh3H-39OHtcH4XWsA6FGpg@mail.gmail.com/
(seems to not have been seen) I'd like to suggest that:
[...]
> * ab/fetch-tags-noclobber (2018-05-16) 9 commits
This be ejected for now.
[...]
> * ab/checkout-default-remote (2018-06-11) 8 commits
And this be merged down to "next".
^ permalink raw reply [flat|nested] 24+ messages in thread
* as/safecrlf-quiet-fix, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
` (2 preceding siblings ...)
2018-06-29 21:04 ` Ævar Arnfjörð Bjarmason
@ 2018-07-03 11:44 ` Johannes Schindelin
2018-07-03 17:59 ` Junio C Hamano
2018-07-03 11:46 ` jh/partial-clone, " Johannes Schindelin
` (5 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-03 11:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi Junio,
On Thu, 28 Jun 2018, Junio C Hamano wrote:
> * as/safecrlf-quiet-fix (2018-06-11) 1 commit
> (merged to 'next' on 2018-06-13 at b163674843)
> + config.c: fix regression for core.safecrlf false
>
> Fix for 2.17-era regression around `core.safecrlf`.
Is this `maint` material?
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: as/safecrlf-quiet-fix, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-03 11:44 ` as/safecrlf-quiet-fix, was " Johannes Schindelin
@ 2018-07-03 17:59 ` Junio C Hamano
2018-07-04 9:25 ` Johannes Schindelin
0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2018-07-03 17:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Junio,
>
> On Thu, 28 Jun 2018, Junio C Hamano wrote:
>
>> * as/safecrlf-quiet-fix (2018-06-11) 1 commit
>> (merged to 'next' on 2018-06-13 at b163674843)
>> + config.c: fix regression for core.safecrlf false
>>
>> Fix for 2.17-era regression around `core.safecrlf`.
>
> Is this `maint` material?
Possibly. Why do you ask?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: as/safecrlf-quiet-fix, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-03 17:59 ` Junio C Hamano
@ 2018-07-04 9:25 ` Johannes Schindelin
2018-07-06 15:38 ` Junio C Hamano
0 siblings, 1 reply; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-04 9:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi Junio,
On Tue, 3 Jul 2018, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Thu, 28 Jun 2018, Junio C Hamano wrote:
> >
> >> * as/safecrlf-quiet-fix (2018-06-11) 1 commit
> >> (merged to 'next' on 2018-06-13 at b163674843)
> >> + config.c: fix regression for core.safecrlf false
> >>
> >> Fix for 2.17-era regression around `core.safecrlf`.
> >
> > Is this `maint` material?
>
> Possibly. Why do you ask?
This is my feeble attempt at helping you triage. My question was more like
a suggestion.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: as/safecrlf-quiet-fix, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-04 9:25 ` Johannes Schindelin
@ 2018-07-06 15:38 ` Junio C Hamano
0 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2018-07-06 15:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Tue, 3 Jul 2018, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > On Thu, 28 Jun 2018, Junio C Hamano wrote:
>> >
>> >> * as/safecrlf-quiet-fix (2018-06-11) 1 commit
>> >> (merged to 'next' on 2018-06-13 at b163674843)
>> >> + config.c: fix regression for core.safecrlf false
>> >>
>> >> Fix for 2.17-era regression around `core.safecrlf`.
>> >
>> > Is this `maint` material?
>>
>> Possibly. Why do you ask?
>
> This is my feeble attempt at helping you triage. My question was more like
> a suggestion.
Ah, thanks. That line of suggestion does help, and it would even be
more helpful when given before the topic hits 'next' or after
'master' from the workflow's point of view.
While the topic is in 'pu' I can change where the topic forks from,
and to minimize temptation to stupidly merge an inappropriate topic
(e.g. new developments and somewhat dubious fixes to commonly used
part of Git) down to 'maint' during the pre-release freeze
(i.e. when the tree is otherwise hopefully sleepily quiet and
boring), I try to base the (beginning of) good 'maint' material on
'maint' or older, and everything else on the tip of 'master' (or the
most recent feature release), when each topic is originally queued
in my tree. If I missed a good topic and based it on 'master', it
is easier to correct such a mistake before it hits 'next', and a
suggestion like above would help me tremendously.
After the topic hits 'next', its course is pretty much set, until it
graduates to 'master'. About a week or two after such event, I'd go
through "merge ... later to maint" entries in the draft release notes
and if the topic still looked relevant to the maintenance branch,
merge them to 'maint'. Before that happens is another chance to stop
me from making a stupid mistake, or remind me of an urgent one that
I've been letting wait in 'master' before merging to 'maint'.
Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* jh/partial-clone, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
` (3 preceding siblings ...)
2018-07-03 11:44 ` as/safecrlf-quiet-fix, was " Johannes Schindelin
@ 2018-07-03 11:46 ` Johannes Schindelin
2018-07-03 11:54 ` as/sequencer-customizable-comment-char, " Johannes Schindelin
` (4 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-03 11:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi Junio,
On Thu, 28 Jun 2018, Junio C Hamano wrote:
> * jh/partial-clone (2018-06-12) 1 commit
> (merged to 'next' on 2018-06-13 at 818f864b0c)
> + list-objects: check if filter is NULL before using
>
> The recent addition of "partial clone" experimental feature kicked
> in when it shouldn't, namely, when there is no partial-clone filter
> defined even if extensions.partialclone is set.
Is this `maint` material?
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* as/sequencer-customizable-comment-char, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
` (4 preceding siblings ...)
2018-07-03 11:46 ` jh/partial-clone, " Johannes Schindelin
@ 2018-07-03 11:54 ` Johannes Schindelin
2018-07-03 12:52 ` ag/rebase-i-rewrite-todo, " Johannes Schindelin
` (3 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-03 11:54 UTC (permalink / raw)
To: Junio C Hamano, Aaron Schrab; +Cc: git
Hi Junio & Aaron,
On Thu, 28 Jun 2018, Junio C Hamano wrote:
> * as/sequencer-customizable-comment-char (2018-06-28) 1 commit
> - sequencer: use configured comment character
>
> Honor core.commentchar when preparing the list of commits to replay
> in "rebase -i".
As per
https://public-inbox.org/git/nycvar.QRO.7.76.6.1806291607501.74@tvgsbejvaqbjf.bet/
I am inclined to ask for a "v2" (i.e. second iteration of the patch, see
e.g.
https://github.com/git-for-windows/git/blob/master/CONTRIBUTING.md#submit-the-patch
where it talks about submitting an "nth version") that adds an explanation
to the commit message why the "auto" value is handled correctly by this
patch, as this is not really obvious from reading the diff alone.
After that change, I think this is ready for `next`.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* ag/rebase-i-rewrite-todo, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
` (5 preceding siblings ...)
2018-07-03 11:54 ` as/sequencer-customizable-comment-char, " Johannes Schindelin
@ 2018-07-03 12:52 ` Johannes Schindelin
2018-07-06 17:50 ` Junio C Hamano
2018-07-03 13:05 ` js/branch-diff, " Johannes Schindelin
` (2 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-03 12:52 UTC (permalink / raw)
To: Junio C Hamano, Alban Gruin; +Cc: git
Hi Junio,
On Thu, 28 Jun 2018, Junio C Hamano wrote:
> * ag/rebase-i-rewrite-todo (2018-06-15) 3 commits
> - rebase--interactive: rewrite the edit-todo functionality in C
> - editor: add a function to launch the sequence editor
> - Merge branch 'bc/t3430-fixup' into ag/rebase-i-rewrite-todo
> (this branch uses ag/rebase-i-append-todo-help.)
>
> Stepwise rewriting of the machinery of "rebase -i" into C continues.
Alban abided by your wish and smooshed those patch series into one honking
big one, to grow even further with his ongoing work. I have to admit that
I am not enthusiastic about this, as it will make it even harder to
squeeze in the time for me to review those patches.
The latest iteration of this is here:
https://public-inbox.org/git/20180702105717.26386-5-alban.gruin@gmail.com/T/#r8eea71077745d6f2c839acb6200bb8b2bea579d3
I would *strongly* encourage you to allow Alban to go back to the small,
incremental patch series he sent before, because it will make it
*substantially* easier to not only review, but also develop, and for you
to merge.
In this case, for example, the two patches in
https://public-inbox.org/git/20180702105717.26386-4-alban.gruin@gmail.com/
and
https://public-inbox.org/git/20180702105717.26386-5-alban.gruin@gmail.com/
are ready to go into `next`, like, right now.
Yet because they are now embedded in a larger patch series, they will have
to be re-rolled over and over again until *all* of
`git-rebase--interactive.sh` is rewritten in C? That's really wasteful of
reviewers' time.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: ag/rebase-i-rewrite-todo, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-03 12:52 ` ag/rebase-i-rewrite-todo, " Johannes Schindelin
@ 2018-07-06 17:50 ` Junio C Hamano
2018-07-06 19:21 ` Johannes Schindelin
0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2018-07-06 17:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alban Gruin, git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The latest iteration of this is here:
> https://public-inbox.org/git/20180702105717.26386-5-alban.gruin@gmail.com/T/#r8eea71077745d6f2c839acb6200bb8b2bea579d3
Good. I think we have it in tree now.
> I would *strongly* encourage you to allow Alban to go back to the small,
> incremental patch series he sent before, because it will make it
> *substantially* easier to not only review, but also develop, and for you
> to merge.
An organization in which you can make sure that the order of
dependency and which ones have been updated since previous rounds
are clear, even to those who are looking from the sidelines ("these
4 patches are to replace patch 3, 7 and 8 from the previous round"
is already hostile to late reviewers and doing so without a pointer
to the archive is even worse---a full reroll with the unchanged ones
marked below the three-dash lines would be perfect), would be good.
A random collection of seemingly separate but actually
interdependent topics is very hard to work with with limited mental
bandwidth.
Once the core of _a_ topic hits 'next', we can go incremental
(because by definition things get quiet and require small updates by
then), but not before.
I think the 7 patches in ag/rebase-i-in-c are more or less in good
shape, modulo the issues pointed out on the list yet to be
addressed, which I do not think require redesign. Which is good.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: ag/rebase-i-rewrite-todo, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-06 17:50 ` Junio C Hamano
@ 2018-07-06 19:21 ` Johannes Schindelin
2018-07-06 19:40 ` Junio C Hamano
0 siblings, 1 reply; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-06 19:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alban Gruin, git
Hi Junio,
On Fri, 6 Jul 2018, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > I would *strongly* encourage you to allow Alban to go back to the small,
> > incremental patch series he sent before, because it will make it
> > *substantially* easier to not only review, but also develop, and for
> > you to merge.
>
> An organization in which you can make sure that the order of dependency
> and which ones have been updated since previous rounds are clear, even
> to those who are looking from the sidelines ("these 4 patches are to
> replace patch 3, 7 and 8 from the previous round" is already hostile to
> late reviewers and doing so without a pointer to the archive is even
> worse---a full reroll with the unchanged ones marked below the
> three-dash lines would be perfect), would be good. A random collection
> of seemingly separate but actually interdependent topics is very hard to
> work with with limited mental bandwidth.
>
> Once the core of _a_ topic hits 'next', we can go incremental (because
> by definition things get quiet and require small updates by then), but
> not before.
>
> I think the 7 patches in ag/rebase-i-in-c are more or less in good
> shape, modulo the issues pointed out on the list yet to be addressed,
> which I do not think require redesign. Which is good.
You do understand that with your proposed "let's just roll them up into
one big patch series, and just add freely whatever you need on top", these
7 patches (3 of which I reviewed I think four times on the list now, and
more times on GitHub, which is quite taxing on my time) will be soon
joined by 6 more patches: https://github.com/git/git/pull/518
Of course, at that point I will have to look through those 7 patches
again, if only to verify that yes, they are still the same.
And Alban is not sitting on his hands, either.
So after reviewing those 13 patches, which undoubtedly will not be
integrated into `next` under the premise that they are still in flux, they
will most likely be joined by another dozen patches until the interactive
rebase is rewritten completely in C. After which time, I will have
reviewed the first 3 patches over 15 times.
I wish there was a better way.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: ag/rebase-i-rewrite-todo, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-06 19:21 ` Johannes Schindelin
@ 2018-07-06 19:40 ` Junio C Hamano
2018-07-06 22:38 ` Johannes Schindelin
0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2018-07-06 19:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alban Gruin, git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Of course, at that point I will have to look through those 7 patches
> again, if only to verify that yes, they are still the same.
That is something the patch author must help the reviewer with, no?
Have uncontroversial stuff early in the series, concentrate on
stabilizing them before moving on to shiny new toys, then mark them
"unchanged since the last round" after three dashes when sending a
reroll to update later parts of the series that are in flux. After
a few rounds, it may become apparent to reviewers that these early
parts can stand on their own merit as a separate series, on top of
which the remaining patches can build as a separate (sub)topic, at
which time we may have two or more topics, among which the early one
that has become stable may already be 'next'.
> And Alban is not sitting on his hands, either.
If you are saying that all of these 13 are constantly in flux,
whether these 13 patches are spread across 3 series or a single one
(assuming that all 13 are about the same topic that are
interdependant), the need to look at their updated incarnation does
not change, so I do not know what you are complaining about.
> So after reviewing those 13 patches, which undoubtedly will not be
> integrated into `next` under the premise that they are still in flux, they
> will most likely be joined by another dozen patches until the interactive
> rebase is rewritten completely in C. After which time, I will have
> reviewed the first 3 patches over 15 times.
>
> I wish there was a better way.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: ag/rebase-i-rewrite-todo, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-06 19:40 ` Junio C Hamano
@ 2018-07-06 22:38 ` Johannes Schindelin
0 siblings, 0 replies; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-06 22:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alban Gruin, git
Hi Junio,
On Fri, 6 Jul 2018, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Of course, at that point I will have to look through those 7 patches
> > again, if only to verify that yes, they are still the same.
>
> That is something the patch author must help the reviewer with, no?
>
> Have uncontroversial stuff early in the series, concentrate on
> stabilizing them before moving on to shiny new toys, then mark them
> "unchanged since the last round" after three dashes when sending a
> reroll to update later parts of the series that are in flux. After
> a few rounds, it may become apparent to reviewers that these early
> parts can stand on their own merit as a separate series, on top of
> which the remaining patches can build as a separate (sub)topic, at
> which time we may have two or more topics, among which the early one
> that has become stable may already be 'next'.
Okay, I give up. I tried to show you that I thought that Alban's
partitioning made sense on its own, that they could (and should) stabilize
independently, even if they technically build on one another. In fact, it
was I who suggested to keep them in bite-sized patch series. And you
simply disagree from your maintaining point of view, and I violently
disagree with your decision from the reviewer's point of view.
In the end, you win.
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* js/branch-diff, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
` (6 preceding siblings ...)
2018-07-03 12:52 ` ag/rebase-i-rewrite-todo, " Johannes Schindelin
@ 2018-07-03 13:05 ` Johannes Schindelin
2018-07-03 22:12 ` Junio C Hamano
2018-07-03 18:30 ` Elijah Newren
2018-07-09 17:31 ` Jonathan Tan
9 siblings, 1 reply; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-03 13:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi Junio,
On Thu, 28 Jun 2018, Junio C Hamano wrote:
> * js/branch-diff (2018-05-16) 19 commits
> - fixup! Add a function to solve least-cost assignment problems
> - completion: support branch-diff
> - branch-diff: add a man page
> - branch-diff --dual-color: work around bogus white-space warning
> - branch-diff: offer to dual-color the diffs
> - diff: add an internal option to dual-color diffs of diffs
> - color: provide inverted colors, too
> - branch-diff: use color for the commit pairs
> - branch-diff: add tests
> - branch-diff: do not show "function names" in hunk headers
> - branch-diff: adjust the output of the commit pairs
> - branch-diff: suppress the diff headers
> - branch-diff: indent the diffs just like tbdiff
> - branch-diff: right-trim commit messages
> - branch-diff: also show the diff between patches
> - branch-diff: improve the order of the shown commits
> - branch-diff: first rudimentary implementation
> - Add a new builtin: branch-diff
> - Add a function to solve least-cost assignment problems
>
> "git tbdiff" that lets us compare individual patches in two
> iterations of a topic has been rewritten and made into a built-in
> command.
>
> Expecting a reroll.
> cf. <nycvar.QRO.7.76.6.1805052351560.77@tvgsbejvaqbjf.bet>
I rolled it around so hard that it got dizzy.
(Yes, I just made fun of this Git speak "to re-roll".)
Seriously again, I sent a new iteration:
https://public-inbox.org/git/pull.1.v3.git.gitgitgadget@gmail.com/
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: js/branch-diff, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-03 13:05 ` js/branch-diff, " Johannes Schindelin
@ 2018-07-03 22:12 ` Junio C Hamano
2018-07-04 9:27 ` Johannes Schindelin
0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2018-07-03 22:12 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Seriously again, I sent a new iteration:
>
> https://public-inbox.org/git/pull.1.v3.git.gitgitgadget@gmail.com/
Thanks, will take a look but it is likely that I'll run out of time
today, so please be a bit patient.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: js/branch-diff, was Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-03 22:12 ` Junio C Hamano
@ 2018-07-04 9:27 ` Johannes Schindelin
0 siblings, 0 replies; 24+ messages in thread
From: Johannes Schindelin @ 2018-07-04 9:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Hi Junio,
On Tue, 3 Jul 2018, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Seriously again, I sent a new iteration:
> >
> > https://public-inbox.org/git/pull.1.v3.git.gitgitgadget@gmail.com/
>
> Thanks, will take a look but it is likely that I'll run out of time
> today, so please be a bit patient.
Sorry, I should have clarified that I do not at all expect reviews to be
performed *right now*. It took *me* over a month to get this iteration
out, after all, and as the changes list suggests: there had been a *ton*
of changes.
While I need the functionality of `range-diff` myself, I'm quite fine over
here with my custom build (which I use via an alias from my default
git.exe).
Ciao,
Dscho
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
` (7 preceding siblings ...)
2018-07-03 13:05 ` js/branch-diff, " Johannes Schindelin
@ 2018-07-03 18:30 ` Elijah Newren
2018-07-03 18:33 ` Eric Sunshine
2018-07-09 17:31 ` Jonathan Tan
9 siblings, 1 reply; 24+ messages in thread
From: Elijah Newren @ 2018-07-03 18:30 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Git Mailing List, Junio C Hamano
Hi Derrick,
On Thu, Jun 28, 2018 at 2:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> * ds/multi-pack-index (2018-06-25) 24 commits
> - midx: clear midx on repack
> - packfile: skip loading index if in multi-pack-index
> - midx: prevent duplicate packfile loads
> - midx: use midx in approximate_object_count
> - midx: use existing midx when writing new one
> - midx: use midx in abbreviation calculations
> - midx: read objects from multi-pack-index
> - midx: prepare midxed_git struct
> - config: create core.multiPackIndex setting
> - midx: write object offsets
> - midx: write object id fanout chunk
> - midx: write object ids in a chunk
> - midx: sort and deduplicate objects from packfiles
> - midx: read pack names into array
> - multi-pack-index: write pack names in chunk
> - multi-pack-index: read packfile list
> - packfile: generalize pack directory list
> - multi-pack-index: expand test data
> - multi-pack-index: load into memory
> - midx: write header information to lockfile
> - multi-pack-index: add 'write' verb
> - multi-pack-index: add builtin
> - multi-pack-index: add format details
> - multi-pack-index: add design document
>
> When there are too many packfiles in a repository (which is not
> recommended), looking up an object in these would require
> consulting many pack .idx files; a new mechanism to have a single
> file that consolidates all of these .idx files is introduced.
pu fails to build for me (with the standard 'make DEVELOPER=1 ...'),
and it appears to come from this series:
midx.c:567:15: error: ‘write_midx_pack_lookup’ defined but not used
[-Werror=unused-function]
static size_t write_midx_pack_lookup(struct hashfile *f,
It looks like the use of this function was removed in v2 of your
series, but the function itself was left behind? Could you take a
look?
Thanks,
Elijah
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-03 18:30 ` Elijah Newren
@ 2018-07-03 18:33 ` Eric Sunshine
0 siblings, 0 replies; 24+ messages in thread
From: Eric Sunshine @ 2018-07-03 18:33 UTC (permalink / raw)
To: Elijah Newren; +Cc: Derrick Stolee, Git List, Junio C Hamano
On Tue, Jul 3, 2018 at 2:31 PM Elijah Newren <newren@gmail.com> wrote:
> On Thu, Jun 28, 2018 at 2:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > * ds/multi-pack-index (2018-06-25) 24 commits
>
> pu fails to build for me (with the standard 'make DEVELOPER=1 ...'),
> and it appears to come from this series:
>
> midx.c:567:15: error: ‘write_midx_pack_lookup’ defined but not used
> [-Werror=unused-function]
> static size_t write_midx_pack_lookup(struct hashfile *f,
>
> It looks like the use of this function was removed in v2 of your
> series, but the function itself was left behind? Could you take a
> look?
Also reported here [1].
[1]: https://public-inbox.org/git/CAPig+cRWwTAFRJOwQOi4USk5UZke=2sz_JVDh3+XRKCcBGD5ow@mail.gmail.com/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-06-28 21:40 What's cooking in git.git (Jun 2018, #07; Thu, 28) Junio C Hamano
` (8 preceding siblings ...)
2018-07-03 18:30 ` Elijah Newren
@ 2018-07-09 17:31 ` Jonathan Tan
2018-07-09 18:13 ` Junio C Hamano
9 siblings, 1 reply; 24+ messages in thread
From: Jonathan Tan @ 2018-07-09 17:31 UTC (permalink / raw)
To: gitster; +Cc: git, Jonathan Tan
> * jt/fetch-pack-negotiator (2018-06-15) 7 commits
> - fetch-pack: introduce negotiator API
> - fetch-pack: move common check and marking together
> - fetch-pack: make negotiation-related vars local
> - fetch-pack: use ref adv. to prune "have" sent
> - fetch-pack: directly end negotiation if ACK ready
> - fetch-pack: clear marks before re-marking
> - fetch-pack: split up everything_local()
> (this branch is used by jt/fetch-nego-tip.)
>
> Code restructuring and a small fix to transport protocol v2 during
> fetching.
>
> Is this ready for 'next'?
Sorry for the late reply - I think that this is ready for "next". I have
addressed all the comments that you, Jonathan Nieder and Brandon
Williams have given, and Brandon has given his OK [1].
[1] https://public-inbox.org/git/20180625182434.GE19910@google.com/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: What's cooking in git.git (Jun 2018, #07; Thu, 28)
2018-07-09 17:31 ` Jonathan Tan
@ 2018-07-09 18:13 ` Junio C Hamano
0 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2018-07-09 18:13 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git
Jonathan Tan <jonathantanmy@google.com> writes:
>> * jt/fetch-pack-negotiator (2018-06-15) 7 commits
>> - fetch-pack: introduce negotiator API
>> - fetch-pack: move common check and marking together
>> - fetch-pack: make negotiation-related vars local
>> - fetch-pack: use ref adv. to prune "have" sent
>> - fetch-pack: directly end negotiation if ACK ready
>> - fetch-pack: clear marks before re-marking
>> - fetch-pack: split up everything_local()
>> (this branch is used by jt/fetch-nego-tip.)
>>
>> Code restructuring and a small fix to transport protocol v2 during
>> fetching.
>>
>> Is this ready for 'next'?
>
> Sorry for the late reply - I think that this is ready for "next". I have
> addressed all the comments that you, Jonathan Nieder and Brandon
> Williams have given, and Brandon has given his OK [1].
>
> [1] https://public-inbox.org/git/20180625182434.GE19910@google.com/
Thanks, I've re-read it over the weekend and agree that we are in a
good shape here.
^ permalink raw reply [flat|nested] 24+ messages in thread