Re: [PATCH 0/3] builtin/commit-graph.c: new split/merge options
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Jan 31, 2020, 14:41 UTC
- Message-ID
- <b873ab69-cdae-7f71-b28b-671b62cea5a8@gmail.com>
- In-Reply-To
- <cover.1580430057.git.me@ttaylorr.com>
On 1/30/2020 7:28 PM, Taylor Blau wrote:
Show 16 quoted lines
> Hi, > > Here are another few patches that came out of working on GitHub's > deployment of incremental commit-graphs. These three patches introduce > two new options: '--split[=<merge-all|no-merge>]' and > '--input=<source>'. > > The former controls whether or not commit-graph's split machinery should > either write an incremental commit graph, squash the chain of > incrementals, or defer to the other options. > > (This comes from GitHub's desire to have more fine-grained control over > the commit-graph chain's behavior. We run short jobs after every push > that we would like to limit the running time of, and hence we do not > want to ever merge a long chain of incrementals unless we specifically > opt into that.)
I can imagine many scenarios that require the amount of work to be predictable, which the current merge strategies do not guarantee.
Show 9 quoted lines
> The latter of the two new options does two things: > > * It cleans up the many options that specify input sources (e.g., > '--stdin-commits', '--stdin-packs', '--reachable' and so on) under > one unifying name. > > * It allows us to introduce a new argument '--input=none', to prevent > walking each packfile when neither '--stdin-commits' nor > '--stdin-packs' was given.
I'm happy with these goals.
Show 8 quoted lines
> Together, these have the combined effect of being able to write the > following two new invocations: > > $ git commit-graph write --split=merge-all --input=none > > $ git commit-graph write --split=no-merge --input=stdin-packs > > to (1) merge the chain, and (2) write a single new incremental.
This is much cleaner than adding yet another option to the builtin, and allows more flexibility in future extensions.
I have a few comments on the patches, but they are minor.
Thanks! -Stolee