Re: [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 31, 2025, 21:05 UTC
- Message-ID
- <xmqq1pwi7kyv.fsf@gitster.g>
- In-Reply-To
- <20250131-toon-clone-refs-v4-6-2a4ff851498f@iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> OPT_STRING('b', "branch", &option_branch, N_("branch"),
> N_("checkout <branch> instead of the remote's HEAD")),
> + OPT_STRING(0, "revision", &option_rev, N_("rev"),
> + N_("clone single revision <rev> and check out")),OK, this thing comes as a string; we'll parse it down to a commit later, hopefully?
Show 5 quoted lines
> - refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix, > - branch_top.buf); > + if (!option_rev) > + refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix, > + branch_top.buf);
> + die_for_incompatible_opt2(!!option_rev, "--revision", > + !!option_branch, "--branch"); > + die_for_incompatible_opt2(!!option_rev, "--revision", > + option_mirror, "--mirror");
So here is where we mark the new thing incompatible with these two, and when either of them is given with "--revision", we would bail out. OK.
> + // TODO --no-single-branch
Style.
Show 11 quoted lines
> @@ -1381,7 +1396,15 @@ int cmd_clone(int argc,
> if (transport->smart_options && !deepen && !filter_options.choice)
> transport->smart_options->check_self_contained_and_connected = 1;
>
> - strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
> + if (option_rev) {
> + option_tags = 0;
> + option_branch = 0;
> + option_single_branch = 0;
> + opts.wants_head = 0;
> + opts.detach = 1;option_branch is of type "char *" so sparse rightfully complains that you are assigning an integer 0 to it, which follows stronger rules than plain vanilla C standard to help us avoid mistakes.
But stepping back a bit, hasn't we already been ruled out earlier that when option_rev is set, we cannot possibly be affected by the "--branch" option that was given at the same time? I do not now about other assignments we see in this block, but are there others that are unnecessary? For example, you do not clear option_mirror in this block. Is option_branch so special that it needs clearing, and if so why?
Thanks.