On Fri, 31 Jan 2020 at 01:30, Taylor Blau <me@xxxxxxxxxxxx> wrote: > In the previous commit, we introduced '--[no-]merge', and alluded to the > fact that '--merge' would be useful for callers who wish to always > trigger a merge of an incremental chain. Hmmm. So it looks like you've already had similar thoughts as I did about patch 1/3. At some point, you had a separate `--merge=...` option, then later made that `--split=...`. :-) Could you say something about why you changed your mind? > There is a problem with the above approach, which is that there is no > way to specify to the commit-graph builtin that a caller only wants to > include commits already in the graph. One can specify '--input=append' > to include all commits in the existing graphs, but the absence of > '--input=stdin-{commits,packs}' causes the builtin to call > 'fill_oids_from_all_packs()'. (Use one of those options with an empty stdin? Anyway, let's read on.) > Passing '--input=reachable' (as in 'git commit-graph write > --split=merge-all --input=reachable --input=append') works around this > issue by making '--input=reachable' effectively a no-op, but this can be > prohibitively expensive in large repositories, making it an undesirable > choice for some users. > > Teach '--input=none' as an option to behave as if '--input=append' were > given, but to consider no other sources in addition. `--input=none` almost makes me wonder if it should produce an empty commit-graph. But there wouldn't be much point in that... I guess another way of defining this would be that it "uses no input, and implies `--append`". > This, in conjunction with the option introduced in the previous patch > offers the convenient way to force the commit-graph machinery to > condense a chain of incrementals without requiring any new commits: > > $ git commit-graph write --split=merge-all --input=none Right. > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -39,24 +39,29 @@ COMMANDS > -------- > 'write':: > > -Write a commit-graph file based on the commits found in packfiles. > +Write a commit-graph file based on the commits specified: > +* With the `--input=stdin-packs` option, generate the new commit graph > +by walking objects only in the specified pack-indexes. (Cannot be > +combined with `--input=stdin-commits` or `--input=reachable`.) > + > -With the `--input=stdin-packs` option, generate the new commit graph by > -walking objects only in the specified pack-indexes. (Cannot be combined > -with `--input=stdin-commits` or `--input=reachable`.) > -+ > -With the `--input=stdin-commits` option, generate the new commit graph > +* With the `--input=stdin-commits` option, generate the new commit graph > by walking commits starting at the commits specified in stdin as a list > of OIDs in hex, one OID per line. (Cannot be combined with > `--input=stdin-packs` or `--input=reachable`.) > + > -With the `--input=reachable` option, generate the new commit graph by > +* With the `--input=reachable` option, generate the new commit graph by > walking commits starting at all refs. (Cannot be combined with > `--input=stdin-commits` or `--input=stdin-packs`.) > + > -With the `--input=append` option, include all commits that are present > +* With the `--input=append` option, include all commits that are present > in the existing commit-graph file. Do these changes above really belong in this commit? > +* With the `--input=none` option, behave as if `input=append` were > +given, but do not walk other packs to find additional commits. > + > +If none of the above options are given, then commits found in > +packfiles are specified. "specified"? Plus, that also happens for `--input=append` right? (It really seems like "append" is an odd one among all the inputs.) > N_("git commit-graph write [--object-dir <objdir>] [--append] " > - "[--split[=<strategy>]] [--input=<reachable|stdin-packs|stdin-commits>] " > + "[--split[=<strategy>]] " > + "[--input=<reachable|stdin-packs|stdin-commits|none>] " > "[--[no-]progress] <split options>"), Hmm, you've left "--append" the old way. Martin