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

Re: [PATCH 3/6] tmp-objdir: introduce API for temporary object directories

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 30, 2016, 21:25 UTC
Message-ID
<xmqqponl84h4.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20160930193613.dwpjiw5xps6a3wgj@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 14 quoted lines
> diff --git a/sha1_file.c b/sha1_file.c
> index 9a79c19..65deaf9 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -421,6 +421,12 @@ void add_to_alternates_file(const char *reference)
>  	free(alts);
>  }
>  
> +void add_to_alternates_internal(const char *reference)
> +{
> +	prepare_alt_odb();
> +	link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0);
> +}
> +

A function _internal being extern felt a bit funny. We are only appending so the first one does not have to be reprepare.

Show 31 quoted lines
> +static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)
> +{
> +	int err;
> +
> +	if (!t)
> +		return 0;
> +
> +	if (t == the_tmp_objdir)
> +		the_tmp_objdir = NULL;
> +
> +	/*
> +	 * This may use malloc via strbuf_grow(), but we should
> +	 * have pre-grown t->path sufficiently so that this
> +	 * doesn't happen in practice.
> +	 */
> +	err = remove_dir_recursively(&t->path, 0);
> +
> +	/*
> +	 * When we are cleaning up due to a signal, we won't bother
> +	 * freeing memory; it may cause a deadlock if the signal
> +	 * arrived while libc's allocator lock is held.
> +	 */
> +	if (!on_signal)
> +		tmp_objdir_free(t);
> +	return err;
> +}
> +
> +int tmp_objdir_destroy(struct tmp_objdir *t)
> +{
> +	return tmp_objdir_destroy_1(t, 0);
> +}
Looks sensible.
Show 5 quoted lines
> +	t = xmalloc(sizeof(*t));
> +	strbuf_init(&t->path, 0);
> +	argv_array_init(&t->env);
> +
> +	strbuf_addf(&t->path, "%s/incoming-XXXXXX", get_object_directory());

I was wondering where you would put this in. Inside .git/objects/ sounds good.

Show 19 quoted lines
> +/*
> + * Make sure we copy packfiles and their associated metafiles in the correct
> + * order. All of these ends_with checks are slightly expensive to do in
> + * the midst of a sorting routine, but in practice it shouldn't matter.
> + * We will have a relatively small number of packfiles to order, and loose
> + * objects exit early in the first line.
> + */
> +static int pack_copy_priority(const char *name)
> +{
> +	if (!starts_with(name, "pack"))
> +		return 0;
> +	if (ends_with(name, ".keep"))
> +		return 1;
> +	if (ends_with(name, ".pack"))
> +		return 2;
> +	if (ends_with(name, ".idx"))
> +		return 3;
> +	return 4;
> +}

Thanks for being careful. A blind "cp -r" would have ruined the day.

We do not do bitmaps upon receiving, I guess.
Show 6 quoted lines
> + *	struct tmp_objdir *t = tmp_objdir_create();
> + *	if (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) &&
> + *	    !tmp_objdir_migrate(t))
> + *		printf("success!\n");
> + *	else
> + *		die("failed...tmp_objdir will clean up for us");

Made me briefly wonder if a caller might want to use appropriate environment to use the tmp-objdir given by the API in addition to its own, but then such a caller just needs to prepare its own argv-array and concatenate tmp_objdir_env() before making the opt_cd_env call, so this is perfectly fine.

Previous: Jeff KingNext: Jeff King
Message 9 of 32 in “receive-pack: quarantine pushed objects”
  1. 0/6 receive-pack: quarantine pushed objectsJeff King, Sep 30, 2016
  2. 1/6 check_connected: accept an env argumentJeff King, Sep 30, 2016
  3. 2/6 sha1_file: always allow relative paths to alternatesJeff King, Sep 30, 2016
  4. René ScharfeOct 2, 2016
  5. Jeff KingOct 2, 2016
  6. Jeff KingOct 2, 2016
  7. Jeff KingOct 2, 2016
  8. 3/6 tmp-objdir: introduce API for temporary object directoriesJeff King, Sep 30, 2016
  9. Junio C HamanoSep 30, 2016
  10. Jeff KingSep 30, 2016
  11. David TurnerSep 30, 2016
  12. Jeff KingSep 30, 2016
  13. David TurnerSep 30, 2016
  14. 4/6 receive-pack: quarantine objects until pre-receive acceptsJeff King, Sep 30, 2016
  15. Jeff KingOct 1, 2016
  16. Ævar Arnfjörð BjarmasonApr 8, 2017
  17. Jeff KingApr 10, 2017
  18. 5/6 tmp-objdir: put quarantine information in the environmentJeff King, Sep 30, 2016
  19. 6/6 tmp-objdir: do not migrate files starting with '.'Jeff King, Sep 30, 2016
  20. Christian CouderOct 2, 2016
  21. Jeff KingOct 2, 2016
  22. Christian CouderOct 3, 2016
  23. 0/5 receive-pack: quarantine pushed objectsJeff King, Oct 3, 2016
  24. 1/5 check_connected: accept an env argumentJeff King, Oct 3, 2016
  25. Jakub NarębskiOct 5, 2016
  26. Jeff KingOct 5, 2016
  27. 2/5 tmp-objdir: introduce API for temporary object directoriesJeff King, Oct 3, 2016
  28. 3/5 receive-pack: quarantine objects until pre-receive acceptsJeff King, Oct 3, 2016
  29. 5/5 tmp-objdir: do not migrate files starting with '.'Jeff King, Oct 3, 2016
  30. 4/5 tmp-objdir: put quarantine information in the environmentJeff King, Oct 3, 2016
  31. Junio C HamanoOct 3, 2016
  32. Jeff KingOct 3, 2016

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.