git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] builtin/commit-graph.c: support '--input=none'

From
Taylor Blau <me@ttaylorr.com>
Date
Feb 4, 2020, 05:01 UTC
Message-ID
<20200204050110.GH5790@syl.local>
In-Reply-To
<CAN0heSpfqMChHO=iNuUExrBsMPcpZ6K2zH68GkHO2ttEiSinSA@mail.gmail.com>
On Fri, Jan 31, 2020 at 08:45:59PM +0100, Martin Ågren wrote:
Show 9 quoted lines
> On Fri, 31 Jan 2020 at 01:30, Taylor Blau <me@ttaylorr.com> 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?

Heh :-). Left overs from an earlier version of this series. I think that I already talked about why this was changed further up in the thread.

Show 22 quoted lines
> > 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`".

I suppose, although (like you) I can't imagine why anybody would want to do that.

Show 40 quoted lines
> > 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?

I think so. My thought here was to leave this documentation as-is until this patch, when adding '--input=none' would... somehow change this, but trying to construct a reply, I can't seem to come up with why I thought that this was a good idea in the first place ;-).

Show 8 quoted lines
> > +* 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.)

I reworded this slightly to not use "specified", which I agree is indeed weird.

Show 8 quoted lines
>
> >         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.
Fixed, and thanks for noticing.
>
> Martin

Thanks, Taylor

Previous: Martin ÅgrenNext: Taylor Blau
Message 28 of 58 in “builtin/commit-graph.c: new split/merge options”
  1. 0/3 builtin/commit-graph.c: new split/merge optionsTaylor Blau, Jan 31, 2020
  2. 1/3 builtin/commit-graph.c: support '--split[=<strategy>]'Taylor Blau, Jan 31, 2020
  3. Derrick StoleeJan 31, 2020
  4. Taylor BlauFeb 4, 2020
  5. Martin ÅgrenJan 31, 2020
  6. Taylor BlauFeb 4, 2020
  7. Martin ÅgrenFeb 6, 2020
  8. Taylor BlauFeb 9, 2020
  9. SZEDER GáborJan 31, 2020
  10. Johannes SchindelinFeb 1, 2020
  11. SZEDER GáborFeb 3, 2020
  12. Jeff KingFeb 3, 2020
  13. Taylor BlauFeb 4, 2020
  14. Jeff KingFeb 4, 2020
  15. Taylor BlauFeb 4, 2020
  16. Taylor BlauFeb 4, 2020
  17. 2/3 builtin/commit-graph.c: introduce '--input=<source>'Taylor Blau, Jan 31, 2020
  18. Derrick StoleeJan 31, 2020
  19. Taylor BlauFeb 4, 2020
  20. Martin ÅgrenJan 31, 2020
  21. Taylor BlauFeb 4, 2020
  22. SZEDER GáborFeb 13, 2020
  23. SZEDER GáborFeb 13, 2020
  24. Taylor BlauFeb 13, 2020
  25. 3/3 builtin/commit-graph.c: support '--input=none'Taylor Blau, Jan 31, 2020
  26. Derrick StoleeJan 31, 2020
  27. Martin ÅgrenJan 31, 2020
  28. Taylor BlauFeb 4, 2020
  29. Taylor BlauJan 31, 2020
  30. Derrick StoleeJan 31, 2020
  31. Derrick StoleeJan 31, 2020
  32. Junio C HamanoFeb 4, 2020
  33. Taylor BlauFeb 5, 2020
  34. 0/3 builtin/commit-graph.c: new split/merge optionsTaylor Blau, Feb 5, 2020
  35. 1/3 builtin/commit-graph.c: support '--split[=<strategy>]'Taylor Blau, Feb 5, 2020
  36. Martin ÅgrenFeb 6, 2020
  37. Derrick StoleeFeb 7, 2020
  38. Taylor BlauFeb 9, 2020
  39. Martin ÅgrenFeb 12, 2020
  40. Taylor BlauFeb 12, 2020
  41. Taylor BlauFeb 9, 2020
  42. 2/3 builtin/commit-graph.c: introduce '--input=<source>'Taylor Blau, Feb 5, 2020
  43. 3/3 builtin/commit-graph.c: support '--input=none'Taylor Blau, Feb 5, 2020
  44. Martin ÅgrenFeb 6, 2020
  45. Taylor BlauFeb 9, 2020
  46. Junio C HamanoFeb 5, 2020
  47. 0/3 builtin/commit-graph.c: new split/merge optionsTaylor Blau, Feb 12, 2020
  48. 1/3 builtin/commit-graph.c: support '--split[=<strategy>]'Taylor Blau, Feb 12, 2020
  49. 2/3 builtin/commit-graph.c: introduce '--input=<source>'Taylor Blau, Feb 12, 2020
  50. 3/3 builtin/commit-graph.c: support '--input=none'Taylor Blau, Feb 12, 2020
  51. SZEDER GáborFeb 13, 2020
  52. SZEDER GáborFeb 13, 2020
  53. Junio C HamanoFeb 13, 2020
  54. Taylor BlauFeb 13, 2020
  55. Taylor BlauFeb 13, 2020
  56. Junio C HamanoFeb 12, 2020
  57. Taylor BlauFeb 13, 2020
  58. Martin ÅgrenFeb 17, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.