From: Junio C Hamano <gitster@pobox•com>
To: Toon Claes <toon@iotcl•com>
Cc: git@vger•kernel.org,
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail•com>,
"Michal Suchánek" <msuchanek@suse•de>,
"Patrick Steinhardt" <ps@pks•im>, "Jeff King" <peff@peff•net>
Subject: Re: [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option
Date: Fri, 31 Jan 2025 13:05:28 -0800 [thread overview]
Message-ID: <xmqq1pwi7kyv.fsf@gitster.g> (raw)
In-Reply-To: <20250131-toon-clone-refs-v4-6-2a4ff851498f@iotcl.com> (Toon Claes's message of "Fri, 31 Jan 2025 16:30:34 +0100")
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?
> - 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.
> @@ -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.
next prev parent reply other threads:[~2025-01-31 21:05 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-29 12:00 [PATCH v2] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
2024-12-02 14:08 ` Patrick Steinhardt
2024-12-02 21:17 ` Jeff King
2024-12-03 15:34 ` Michal Suchánek
2024-12-19 11:23 ` Toon Claes
2024-12-19 11:58 ` [PATCH v3] " Toon Claes
2024-12-19 18:30 ` Junio C Hamano
2025-01-31 15:30 ` [PATCH v4 0/6] Enable doing a shallow clone of a specific git revision Toon Claes
2025-01-31 15:30 ` [PATCH v4 1/6] clone: cut down on global variables in clone.c Toon Claes
2025-01-31 15:30 ` [PATCH v4 2/6] clone: make it possible to specify --tags Toon Claes
2025-02-01 16:47 ` Jean-Noël AVILA
2025-01-31 15:30 ` [PATCH v4 3/6] clone: refactor wanted_peer_refs() Toon Claes
2025-02-03 7:51 ` Patrick Steinhardt
2025-01-31 15:30 ` [PATCH v4 4/6] clone: add tags refspec earlier to fetch refspec Toon Claes
2025-02-03 7:51 ` Patrick Steinhardt
2025-01-31 15:30 ` [PATCH v4 5/6] clone: introduce struct clone_opts in builtin/clone.c Toon Claes
2025-02-03 7:51 ` Patrick Steinhardt
2025-01-31 15:30 ` [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
2025-01-31 21:05 ` Junio C Hamano [this message]
2025-02-01 16:50 ` Jean-Noël AVILA
2025-02-03 7:51 ` Patrick Steinhardt
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
2025-02-04 21:34 ` [PATCH v5 1/7] clone: cut down on global variables in clone.c Toon Claes
2025-02-04 21:34 ` [PATCH v5 2/7] clone: make it possible to specify --tags Toon Claes
2025-02-05 8:03 ` Patrick Steinhardt
2025-02-05 16:29 ` Toon Claes
2025-02-05 21:15 ` Jean-Noël AVILA
2025-02-04 21:34 ` [PATCH v5 3/7] clone: refactor wanted_peer_refs() Toon Claes
2025-02-04 21:34 ` [PATCH v5 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
2025-02-05 8:03 ` Patrick Steinhardt
2025-02-04 21:34 ` [PATCH v5 5/7] clone: introduce struct clone_opts in builtin/clone.c Toon Claes
2025-02-04 21:34 ` [PATCH v5 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
2025-02-05 8:03 ` Patrick Steinhardt
2025-02-04 21:34 ` [PATCH v5 7/7] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
2025-02-05 8:03 ` Patrick Steinhardt
2025-02-05 16:43 ` Toon Claes
2025-02-05 8:03 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Patrick Steinhardt
2025-02-05 14:09 ` Junio C Hamano
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
2025-02-05 16:47 ` [PATCH v6 1/7] clone: cut down on global variables in clone.c Toon Claes
2025-02-05 16:47 ` [PATCH v6 2/7] clone: make it possible to specify --tags Toon Claes
2025-02-05 16:47 ` [PATCH v6 3/7] clone: refactor wanted_peer_refs() Toon Claes
2025-02-05 16:47 ` [PATCH v6 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
2025-02-05 16:47 ` [PATCH v6 5/7] clone: introduce struct clone_opts in builtin/clone.c Toon Claes
2025-02-05 16:47 ` [PATCH v6 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
2025-02-05 16:47 ` [PATCH v6 7/7] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
2025-02-05 17:24 ` Junio C Hamano
2025-02-06 6:33 ` [PATCH v7 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
2025-02-06 6:33 ` [PATCH v7 1/7] clone: cut down on global variables in clone.c Toon Claes
2025-02-06 6:33 ` [PATCH v7 2/7] clone: make it possible to specify --tags Toon Claes
2025-02-06 6:33 ` [PATCH v7 3/7] clone: refactor wanted_peer_refs() Toon Claes
2025-02-06 6:33 ` [PATCH v7 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
2025-02-06 6:33 ` [PATCH v7 5/7] clone: introduce struct clone_opts in builtin/clone.c Toon Claes
2025-02-06 6:33 ` [PATCH v7 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
2025-02-06 6:33 ` [PATCH v7 7/7] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
2025-02-06 8:13 ` Patrick Steinhardt
2025-02-06 20:26 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq1pwi7kyv.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=kristofferhaugsbakk@fastmail$(echo .)com \
--cc=msuchanek@suse$(echo .)de \
--cc=peff@peff$(echo .)net \
--cc=ps@pks$(echo .)im \
--cc=toon@iotcl$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox