Re: [PATCH] Consider object stores in alternates during a dissociating clone
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 22, 2015, 16:12 UTC
- Message-ID
- <xmqq7fmeubkf.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <5628EBAF.1030205@cetitec.com>
Alexander Riesen <alexander.riesen@cetitec.com> writes:
Show 17 quoted lines
> 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.
Show 20 quoted lines
> 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.