Re: [PATCH] Consider object stores in alternates during a dissociating clone

[Junio C Hamano <gitster@pobox.com>]

Alexander Riesen <alexander.riesen@cetitec.com> writes:

> I think I understand. How about this?
>
>  builtin/clone.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 9eaecd9..a7d0c07 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -801,11 +801,15 @@ static void write_refspec_config(const char *src_ref_prefix,
>  static void dissociate_from_references(void)
>  {
>      static const char* argv[] = { "repack", "-a", "-d", NULL };
> +    char *alts = git_pathdup("objects/info/alternates");
>
> +    if (access(alts, F_OK) < 0)
> +        return;

You leak alts here.

>      if (run_command_v_opt(argv, RUN_GIT_CMD|RUN_COMMAND_NO_STDIN))
>          die(_("cannot repack to clean up"));
> -    if (unlink(git_path("objects/info/alternates")) && errno != ENOENT)
> +    if (unlink(alts) && errno != ENOENT)
>          die_errno(_("cannot unlink temporary alternates file"));
> +    free(alts);
>  }
>
>  int cmd_clone(int argc, const char **argv, const char *prefix)
> @@ -954,10 +958,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>
>      if (option_reference.nr)
>          setup_reference();
> -    else if (option_dissociate) {
> -        warning(_("--dissociate given, but there is no --reference"));
> -        option_dissociate = 0;
> -    }
>
>      fetch_pattern = value.buf;
>      refspec = parse_fetch_refspec(1, &fetch_pattern);

Perhaps you would want a new test somewhere that (1) prepares the
ultimate source, (2) prepares a borrowing source with "clone
--reference" from the previous, (3) creates a local clone of the
previous with "clone --local" without "--reference" but with
"--dissociate", and (4) checks the end result by ensuring the
absense of $GIT_DIR/objects/info/alternates and runs "fsck" on it.

Other than these two points, the patch looks good to me.