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

Re: [PATCH v5 2/2] gc: introduce `gc.recentObjectsHook`

From
Glen Choo <chooglen@google.com>
Date
Jun 9, 2023, 23:33 UTC
Message-ID
<kl6lwn0cfbyh.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To
<f661b54941a12c9bd8e226aebb4f0d53c10479c8.1686178684.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> When performing a garbage collection operation on a repository with
> unreachable objects, Git makes its decision on what to do with those
> object(s) bed on how recent the objects are or not. Generally speaking,
s/bed/based/ ?
Show 8 quoted lines
> +gc.recentObjectsHook::
> +	When considering whether or not to remove an object (either when
> +	generating a cruft pack or storing unreachable objects as
> +	loose), use the shell to execute the specified command(s).
> +	Interpret their output as object IDs which Git will consider as
> +	"recent", regardless of their age. By treating their mtimes as
> +	"now", any objects (and their descendants) mentioned in the
> +	output will be kept regardless of their true age.
Thanks! I think this version will be clear to prospective users.
Show 23 quoted lines
> +static int run_one_gc_recent_objects_hook(struct oidset *set,
> +					    const char *args)
> +{
> +	struct child_process cmd = CHILD_PROCESS_INIT;
> +	struct strbuf buf = STRBUF_INIT;
> +	FILE *out;
> +	int ret = 0;
> +
> +	cmd.use_shell = 1;
> +	cmd.out = -1;
> +
> +	strvec_push(&cmd.args, args);
> +
> +	if (start_command(&cmd))
> +		return -1;
> +
> +	out = xfdopen(cmd.out, "r");
> +	while (strbuf_getline(&buf, out) != EOF) {
> +		struct object_id oid;
> +		const char *rest;
> +
> +		if (parse_oid_hex(buf.buf, &oid, &rest) || *rest) {
> +			ret = error(_("invalid extra cruft tip: '%s'"), buf.buf);

To be consistent with the other error message, perhaps s/extra cruft tip/additional recent object/?

Show 16 quoted lines
> +static void load_gc_recent_objects(struct recent_data *data)
> +{
> +	const struct string_list *programs;
> +	int ret = 0;
> +	size_t i;
> +
> +	data->extra_recent_oids_loaded = 1;
> +
> +	if (git_config_get_string_multi("gc.recentobjectshook", &programs))
> +		return;
> +
> +	for (i = 0; i < programs->nr; i++) {
> +		ret = run_one_gc_recent_objects_hook(&data->extra_recent_oids,
> +						       programs->items[i].string);
> +		if (ret)
> +			die(_("unable to enumerate additional recent objects"));
This error message to be exact.
Show 14 quoted lines
> +test_expect_success 'gc.recentObjectsHook' '
> +	git init repo &&
> +	test_when_finished "rm -fr repo" &&
> +	(
> +		cd repo &&
> +
> +		# Create a handful of objects.
> +		#
> +		#   - one reachable commit, "base", designated for the reachable
> +		#     pack
> +		#   - one unreachable commit, "cruft.discard", which is marked
> +		#     for deletion
> +		#   - one unreachable commit, "cruft.old", which would be marked
> +		#     for deletion, but is rescued as an extra cruft tip

Perhaps we intended to drop "cruft tips" in the test comments too? e.g. here and..

Show 32 quoted lines
> +test_expect_success 'multi-valued gc.recentObjectsHook' '
> +	git init repo &&
> +	test_when_finished "rm -fr repo" &&
> +	(
> +		cd repo &&
> +
> +		test_commit base &&
> +		git branch -M main &&
> +
> +		git checkout --orphan cruft.a &&
> +		git rm -fr . &&
> +		test_commit --no-tag cruft.a &&
> +		cruft_a="$(git rev-parse HEAD)" &&
> +
> +		git checkout --orphan cruft.b &&
> +		git rm -fr . &&
> +		test_commit --no-tag cruft.b &&
> +		cruft_b="$(git rev-parse HEAD)" &&
> +
> +		git checkout main &&
> +		git branch -D cruft.a cruft.b &&
> +		git reflog expire --all --expire=all &&
> +
> +		echo "echo $cruft_a" | write_script extra-tips.a &&
> +		echo "echo $cruft_b" | write_script extra-tips.b &&
> +		echo "false" | write_script extra-tips.c &&
> +
> +		git rev-list --objects --no-object-names $cruft_a $cruft_b \
> +			>cruft.raw &&
> +		sort cruft.raw >cruft.expect &&
> +
> +		# ensure that each extra cruft tip is saved by its
here.
Aside from those trivial points, everything looks good.
Previous: Taylor BlauNext: Junio C Hamano
Message 27 of 28 in “pack-objects: introduce `pack.extraRecentObjectsHook`”
  1. 0/2 pack-objects: introduce `pack.extraRecentObjectsHook`Taylor Blau, May 11, 2023
  2. 1/2 reachable.c: extract `obj_is_recent()`Taylor Blau, May 11, 2023
  3. 2/2 builtin/pack-objects.c: introduce `pack.recentObjectsHook`Taylor Blau, May 11, 2023
  4. Jeff KingMay 12, 2023
  5. Taylor BlauMay 15, 2023
  6. Jeff KingMay 12, 2023
  7. Taylor BlauMay 12, 2023
  8. Jeff KingMay 12, 2023
  9. Jeff KingMay 12, 2023
  10. Jeff KingMay 12, 2023
  11. Junio C HamanoMay 12, 2023
  12. Jeff KingMay 13, 2023
  13. Jeff KingMay 13, 2023
  14. Taylor BlauMay 15, 2023
  15. Taylor BlauMay 15, 2023
  16. Taylor BlauMay 11, 2023
  17. Junio C HamanoMay 11, 2023
  18. Taylor BlauMay 11, 2023
  19. 0/2 gc: introduce `gc.recentObjectsHook`Taylor Blau, May 16, 2023
  20. 1/2 reachable.c: extract `obj_is_recent()`Taylor Blau, May 16, 2023
  21. 2/2 gc: introduce `gc.recentObjectsHook`Taylor Blau, May 16, 2023
  22. Glen ChooMay 24, 2023
  23. Taylor BlauJun 7, 2023
  24. 0/2 gc: introduce `gc.recentObjectsHook`Taylor Blau, Jun 7, 2023
  25. 1/2 reachable.c: extract `obj_is_recent()`Taylor Blau, Jun 7, 2023
  26. 2/2 gc: introduce `gc.recentObjectsHook`Taylor Blau, Jun 7, 2023
  27. Glen ChooJun 9, 2023
  28. Junio C HamanoJun 12, 2023

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.