On 8/18/2020 8:09 PM, Jonathan Tan wrote:

quoted

Instead of writing a new commit-graph in every 'git maintenance run
--auto' process (when maintenance.commit-graph.enalbed is configured to

s/enalbed/enabled/

quoted

+/* Remember to update object flag allocation in object.h */
+#define PARENT1		(1u<<16)

Why this name? "SEEN" seems perfectly fine.

For some reason I thought that there might be issues using
SEEN (1u<<0) but trying it locally does not seem to be a
problem. I think I've just been bit by how revision.c uses
it a bit too often. The use here is independent enough to
not cause problems.

quoted

+static int num_commits_not_in_graph = 0;
+static int limit_commits_not_in_graph = 100;
+
+static int dfs_on_ref(const char *refname,
+		      const struct object_id *oid, int flags,
+		      void *cb_data)
+{

[snip]

quoted

+static int should_write_commit_graph(void)
+{
+	int result;
+
+	git_config_get_int("maintenance.commit-graph.auto",
+			   &limit_commits_not_in_graph);
+
+	if (!limit_commits_not_in_graph)
+		return 0;
+	if (limit_commits_not_in_graph < 0)
+		return 1;
+
+	result = for_each_ref(dfs_on_ref, NULL);

I don't like introducing the mutable global num_commits_not_in_graph
especially when there seems to be at least one easy alternative (e.g.
putting it in cb_data) but I know that this is a pattern than existing
code.

I should have done this right in the first place. Thanks for
catching it.

-Stolee