Jeff King <p...@peff.net> writes:

> 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.

> +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.

> +     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.

> +/*
> + * 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.

> + *   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.

Reply via email to