* [PATCH v4 1/6] clone: cut down on global variables in clone.c
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 ` Toon Claes
2025-01-31 15:30 ` [PATCH v4 2/6] clone: make it possible to specify --tags Toon Claes
` (5 subsequent siblings)
6 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-01-31 15:30 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
In clone.c the `struct option` which is used to parse the input options
for git-clone(1) is a global variable. Due to this, many variables that
are used to parse the value into, are also global.
Make `builtin_clone_options` a local variable in cmd_clone() and carry
along all variables that are only used in that function.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 195 +++++++++++++++++++++++++++++---------------------------
1 file changed, 101 insertions(+), 94 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index fd001d800c635e46bbc7027a8fdda2a8c9fbf069..5ed0802f1d0ddebaf512aac93bf8c8b340494323 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,42 +56,22 @@
* - dropping use-separate-remote and no-separate-remote compatibility
*
*/
-static const char * const builtin_clone_usage[] = {
- N_("git clone [<options>] [--] <repo> [<dir>]"),
- NULL
-};
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_no_tags;
static int option_shallow_submodules;
-static int option_reject_shallow = -1; /* unspecified */
static int config_reject_shallow = -1; /* unspecified */
-static int deepen;
-static char *option_template, *option_depth, *option_since;
-static char *option_origin = NULL;
static char *remote_name = NULL;
static char *option_branch = NULL;
-static struct string_list option_not = STRING_LIST_INIT_NODUP;
-static const char *real_git_dir;
-static const char *ref_format;
-static const char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
-static int option_progress = -1;
-static int option_sparse_checkout;
-static enum transport_family family;
-static struct string_list option_config = STRING_LIST_INIT_NODUP;
static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
-static int option_dissociate;
static int max_jobs = -1;
static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;
static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
-static int option_filter_submodules = -1; /* unspecified */
static int config_filter_submodules = -1; /* unspecified */
-static struct string_list server_options = STRING_LIST_INIT_NODUP;
static int option_remote_submodules;
-static const char *bundle_uri;
static int recurse_submodules_cb(const struct option *opt,
const char *arg, int unset)
@@ -107,78 +87,6 @@ static int recurse_submodules_cb(const struct option *opt,
return 0;
}
-static struct option builtin_clone_options[] = {
- OPT__VERBOSITY(&option_verbosity),
- OPT_BOOL(0, "progress", &option_progress,
- N_("force progress reporting")),
- OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
- N_("don't clone shallow repository")),
- OPT_BOOL('n', "no-checkout", &option_no_checkout,
- N_("don't create a checkout")),
- OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
- OPT_HIDDEN_BOOL(0, "naked", &option_bare,
- N_("create a bare repository")),
- OPT_BOOL(0, "mirror", &option_mirror,
- N_("create a mirror repository (implies --bare)")),
- OPT_BOOL('l', "local", &option_local,
- N_("to clone from a local repository")),
- OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
- N_("don't use local hardlinks, always copy")),
- OPT_BOOL('s', "shared", &option_shared,
- N_("setup as shared repository")),
- { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
- N_("pathspec"), N_("initialize submodules in the clone"),
- PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
- OPT_ALIAS(0, "recursive", "recurse-submodules"),
- OPT_INTEGER('j', "jobs", &max_jobs,
- N_("number of submodules cloned in parallel")),
- OPT_STRING(0, "template", &option_template, N_("template-directory"),
- N_("directory from which templates will be used")),
- OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
- N_("reference repository")),
- OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
- N_("repo"), N_("reference repository")),
- OPT_BOOL(0, "dissociate", &option_dissociate,
- N_("use --reference only while cloning")),
- OPT_STRING('o', "origin", &option_origin, N_("name"),
- N_("use <name> instead of 'origin' to track upstream")),
- OPT_STRING('b', "branch", &option_branch, N_("branch"),
- N_("checkout <branch> instead of the remote's HEAD")),
- OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
- N_("path to git-upload-pack on the remote")),
- OPT_STRING(0, "depth", &option_depth, N_("depth"),
- N_("create a shallow clone of that depth")),
- OPT_STRING(0, "shallow-since", &option_since, N_("time"),
- N_("create a shallow clone since a specific time")),
- OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
- N_("deepen history of shallow clone, excluding ref")),
- OPT_BOOL(0, "single-branch", &option_single_branch,
- N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
- OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
- N_("any cloned submodules will be shallow")),
- OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
- N_("separate git dir from working tree")),
- OPT_STRING(0, "ref-format", &ref_format, N_("format"),
- N_("specify the reference format to use")),
- OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
- N_("set config inside the new repository")),
- OPT_STRING_LIST(0, "server-option", &server_options,
- N_("server-specific"), N_("option to transmit")),
- OPT_IPVERSION(&family),
- OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
- OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
- N_("apply partial clone filters to submodules")),
- OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
- N_("any cloned submodules will use their remote-tracking branch")),
- OPT_BOOL(0, "sparse", &option_sparse_checkout,
- N_("initialize sparse-checkout file to include only files at root")),
- OPT_STRING(0, "bundle-uri", &bundle_uri,
- N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
- OPT_END()
-};
-
static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
{
static const char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -989,10 +897,103 @@ int cmd_clone(int argc,
int hash_algo;
enum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
const int do_not_override_repo_unix_permissions = -1;
+ int option_reject_shallow = -1; /* unspecified */
+ int deepen = 0;
+ char *option_template = NULL, *option_depth = NULL, *option_since = NULL;
+ char *option_origin = NULL;
+ struct string_list option_not = STRING_LIST_INIT_NODUP;
+ const char *real_git_dir = NULL;
+ const char *ref_format = NULL;
+ const char *option_upload_pack = "git-upload-pack";
+ int option_progress = -1;
+ int option_sparse_checkout = 0;
+ enum transport_family family = TRANSPORT_FAMILY_ALL;
+ struct string_list option_config = STRING_LIST_INIT_DUP;
+ int option_dissociate = 0;
+ int option_filter_submodules = -1; /* unspecified */
+ struct string_list server_options = STRING_LIST_INIT_NODUP;
+ const char *bundle_uri = NULL;
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
+ struct option builtin_clone_options[] = {
+ OPT__VERBOSITY(&option_verbosity),
+ OPT_BOOL(0, "progress", &option_progress,
+ N_("force progress reporting")),
+ OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
+ N_("don't clone shallow repository")),
+ OPT_BOOL('n', "no-checkout", &option_no_checkout,
+ N_("don't create a checkout")),
+ OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
+ OPT_HIDDEN_BOOL(0, "naked", &option_bare,
+ N_("create a bare repository")),
+ OPT_BOOL(0, "mirror", &option_mirror,
+ N_("create a mirror repository (implies --bare)")),
+ OPT_BOOL('l', "local", &option_local,
+ N_("to clone from a local repository")),
+ OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
+ N_("don't use local hardlinks, always copy")),
+ OPT_BOOL('s', "shared", &option_shared,
+ N_("setup as shared repository")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
+ N_("pathspec"), N_("initialize submodules in the clone"),
+ PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
+ OPT_ALIAS(0, "recursive", "recurse-submodules"),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("number of submodules cloned in parallel")),
+ OPT_STRING(0, "template", &option_template, N_("template-directory"),
+ N_("directory from which templates will be used")),
+ OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
+ N_("reference repository")),
+ OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
+ N_("repo"), N_("reference repository")),
+ OPT_BOOL(0, "dissociate", &option_dissociate,
+ N_("use --reference only while cloning")),
+ OPT_STRING('o', "origin", &option_origin, N_("name"),
+ N_("use <name> instead of 'origin' to track upstream")),
+ OPT_STRING('b', "branch", &option_branch, N_("branch"),
+ N_("checkout <branch> instead of the remote's HEAD")),
+ OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
+ N_("path to git-upload-pack on the remote")),
+ OPT_STRING(0, "depth", &option_depth, N_("depth"),
+ N_("create a shallow clone of that depth")),
+ OPT_STRING(0, "shallow-since", &option_since, N_("time"),
+ N_("create a shallow clone since a specific time")),
+ OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
+ N_("deepen history of shallow clone, excluding ref")),
+ OPT_BOOL(0, "single-branch", &option_single_branch,
+ N_("clone only one branch, HEAD or --branch")),
+ OPT_BOOL(0, "no-tags", &option_no_tags,
+ N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
+ N_("any cloned submodules will be shallow")),
+ OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
+ N_("separate git dir from working tree")),
+ OPT_STRING(0, "ref-format", &ref_format, N_("format"),
+ N_("specify the reference format to use")),
+ OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
+ N_("set config inside the new repository")),
+ OPT_STRING_LIST(0, "server-option", &server_options,
+ N_("server-specific"), N_("option to transmit")),
+ OPT_IPVERSION(&family),
+ OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
+ OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
+ N_("apply partial clone filters to submodules")),
+ OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
+ N_("any cloned submodules will use their remote-tracking branch")),
+ OPT_BOOL(0, "sparse", &option_sparse_checkout,
+ N_("initialize sparse-checkout file to include only files at root")),
+ OPT_STRING(0, "bundle-uri", &bundle_uri,
+ N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
+ OPT_END()
+ };
+
+ const char * const builtin_clone_usage[] = {
+ N_("git clone [<options>] [--] <repo> [<dir>]"),
+ NULL
+ };
+
packet_trace_identity("clone");
git_config(git_clone_config, NULL);
@@ -1138,8 +1139,8 @@ int cmd_clone(int argc,
for_each_string_list_item(item, &option_recurse_submodules) {
strbuf_addf(&sb, "submodule.active=%s",
item->string);
- string_list_append(&option_config,
- strbuf_detach(&sb, NULL));
+ string_list_append(&option_config, sb.buf);
+ strbuf_reset(&sb);
}
if (!git_config_get_bool("submodule.stickyRecursiveClone", &val) &&
@@ -1161,6 +1162,8 @@ int cmd_clone(int argc,
string_list_append(&option_config,
"submodule.alternateErrorStrategy=info");
}
+
+ strbuf_release(&sb);
}
/*
@@ -1578,6 +1581,10 @@ int cmd_clone(int argc,
err = checkout(submodule_progress, filter_submodules,
ref_storage_format);
+ string_list_clear(&option_not, 0);
+ string_list_clear(&option_config, 0);
+ string_list_clear(&server_options, 0);
+
free(remote_name);
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
--
2.48.1.164.g9a5474a648.dirty
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v4 2/6] clone: make it possible to specify --tags
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 ` 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
` (4 subsequent siblings)
6 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-01-31 15:30 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
Option --no-tags was added in 0dab2468ee (clone: add a --no-tags option
to clone without tags, 2017-04-26). At the time there was no need to
support --tags as well, although there was some conversation about
it[1].
To simplify the code and to prepare for future commits, invert the flag
internally. Functionally there is no change, because the flag is
default-enabled passing `--tags` has no effect, so there's no need to
add tests for this.
[1]: https://lore.kernel.org/git/CAGZ79kbHuMpiavJ90kQLEL_AR0BEyArcZoEWAjPPhOFacN16YQ@mail.gmail.com/
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 7 ++++---
builtin/clone.c | 14 +++++++-------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index de8d8f58930ecff305f79480b13ddce10cd96c60..99a9222e63429b3447deb3e7c08962d4ec60a295 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -13,7 +13,7 @@ git clone [--template=<template-directory>]
[-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
[-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
[--dissociate] [--separate-git-dir <git-dir>]
- [--depth <depth>] [--[no-]single-branch] [--no-tags]
+ [--depth <depth>] [--[no-]single-branch] [--[no-]-tags]
[--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
[--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
[--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository>
@@ -273,8 +273,9 @@ corresponding `--mirror` and `--no-tags` options instead.
branch when `--single-branch` clone was made, no remote-tracking
branch is created.
-`--no-tags`::
- Don't clone any tags, and set
+`--[no-]tags`::
+ By default tags are cloned, and passing `--tags` doesn't change that.
+ With `--no-tags`, no tags are cloned, and set
`remote.<remote>.tagOpt=--no-tags` in the config, ensuring
that future `git pull` and `git fetch` operations won't follow
any tags. Subsequent explicit tag fetches will still work,
diff --git a/builtin/clone.c b/builtin/clone.c
index 5ed0802f1d0ddebaf512aac93bf8c8b340494323..69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,7 +59,7 @@
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
-static int option_no_tags;
+static int option_tags = 1; /* default enabled */
static int option_shallow_submodules;
static int config_reject_shallow = -1; /* unspecified */
static char *remote_name = NULL;
@@ -470,7 +470,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && !option_no_tags)
+ if (!option_mirror && !option_single_branch && option_tags)
get_fetch_map(refs, &tag_refspec, &tail, 0);
refspec_item_clear(&tag_refspec);
@@ -562,7 +562,7 @@ static void update_remote_refs(const struct ref *refs,
if (refs) {
write_remote_refs(mapped_refs);
- if (option_single_branch && !option_no_tags)
+ if (option_single_branch && option_tags)
write_followtags(refs, msg);
}
@@ -964,8 +964,8 @@ int cmd_clone(int argc,
N_("deepen history of shallow clone, excluding ref")),
OPT_BOOL(0, "single-branch", &option_single_branch,
N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "tags", &option_tags,
+ N_("clone tags, and make later fetches not to follow them")),
OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
N_("any cloned submodules will be shallow")),
OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
@@ -1296,7 +1296,7 @@ int cmd_clone(int argc,
git_config_set(key.buf, repo);
strbuf_reset(&key);
- if (option_no_tags) {
+ if (!option_tags) {
strbuf_addf(&key, "remote.%s.tagOpt", remote_name);
git_config_set(key.buf, "--no-tags");
strbuf_reset(&key);
@@ -1389,7 +1389,7 @@ int cmd_clone(int argc,
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (!option_no_tags)
+ if (option_tags)
strvec_push(&transport_ls_refs_options.ref_prefixes,
"refs/tags/");
--
2.48.1.164.g9a5474a648.dirty
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v4 2/6] clone: make it possible to specify --tags
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
0 siblings, 0 replies; 57+ messages in thread
From: Jean-Noël AVILA @ 2025-02-01 16:47 UTC (permalink / raw)
To: git, Toon Claes
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
Hello,
I'm only reviewing the doc part.
On Friday, 31 January 2025 16:30:30 UTC+1 Toon Claes wrote:
> Option --no-tags was added in 0dab2468ee (clone: add a --no-tags option
> to clone without tags, 2017-04-26). At the time there was no need to
> support --tags as well, although there was some conversation about
> it[1].
>
> To simplify the code and to prepare for future commits, invert the flag
> internally. Functionally there is no change, because the flag is
> default-enabled passing `--tags` has no effect, so there's no need to
> add tests for this.
>
> [1]:
> https://lore.kernel.org/git/
CAGZ79kbHuMpiavJ90kQLEL_AR0BEyArcZoEWAjPPhOFacN16
> YQ@mail.gmail.com/
>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> Documentation/git-clone.txt | 7 ++++---
> builtin/clone.c | 14 +++++++-------
> 2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index
> de8d8f58930ecff305f79480b13ddce10cd96c60..99a9222e63429b3447deb3e7c08962d4ec6
> 0a295 100644 --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -13,7 +13,7 @@ git clone [--template=<template-directory>]
> [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
> [-o <name>] [-b <name>] [-u <upload-pack>] [--reference
<repository>]
> [--dissociate] [--separate-git-dir <git-dir>]
> - [--depth <depth>] [--[no-]single-branch] [--no-tags]
> + [--depth <depth>] [--[no-]single-branch] [--[no-]-tags]
There's an extra '-' : it should read '--[no-]tags'
> [--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
> [--[no-]remote-submodules] [--jobs <n>] [--sparse] [--
[no-]reject-shallow]
> [--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository> @@
> -273,8 +273,9 @@ corresponding `--mirror` and `--no-tags` options instead.
> branch when `--single-branch` clone was made, no remote-tracking
> branch is created.
>
> -`--no-tags`::
> - Don't clone any tags, and set
> +`--[no-]tags`::
> + By default tags are cloned, and passing `--tags` doesn't change
that.
> + With `--no-tags`, no tags are cloned, and set
Better keep and start the description with the imperative mood , as in the
previous version and add the "By default,..." at the end of the description.
> `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
> that future `git pull` and `git fetch` operations won't follow
> any tags. Subsequent explicit tag fetches will still work,
> diff --git a/builtin/clone.c b/builtin/clone.c
> index
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 3/6] clone: refactor wanted_peer_refs()
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-01-31 15:30 ` 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
` (3 subsequent siblings)
6 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-01-31 15:30 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
The function wanted_peer_refs() is used to map the refs returned by the
server to refs we will save in our clone.
Over time this function grown to be very complex. Refactor it.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4..d652682494d0d27dd73cd0585e28b23f2883786d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -434,46 +434,37 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
{
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
- struct ref **tail = head ? &head->next : &local_refs;
+ struct ref **tail = local_refs ? &local_refs->next : &local_refs;
struct refspec_item tag_refspec;
+ struct ref *to_free = NULL;
refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
if (option_single_branch) {
- struct ref *remote_head = NULL;
-
if (!option_branch)
- remote_head = guess_remote_head(head, refs, 0);
+ refs = to_free = guess_remote_head(head, refs, 0);
else {
free_one_ref(head);
local_refs = head = NULL;
tail = &local_refs;
- remote_head = copy_ref(find_remote_branch(refs, option_branch));
- }
-
- if (!remote_head && option_branch)
- warning(_("Could not find remote branch %s to clone."),
- option_branch);
- else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(remote_head, &refspec->items[i],
- &tail, 0);
-
- /* if --branch=tag, pull the requested tag explicitly */
- get_fetch_map(remote_head, &tag_refspec, &tail, 0);
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
- free_refs(remote_head);
- } else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && option_tags)
+ for (int i = 0; i < refspec->nr; i++)
+ get_fetch_map(refs, &refspec->items[i], &tail, 0);
+
+ /*
+ * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
+ * about won't be present in `refs` anyway.
+ * Except with option --mirror, where we grab all refs already.
+ */
+ if (!option_mirror)
get_fetch_map(refs, &tag_refspec, &tail, 0);
+ free_one_ref(to_free);
refspec_item_clear(&tag_refspec);
+
return local_refs;
}
--
2.48.1.164.g9a5474a648.dirty
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v4 3/6] clone: refactor wanted_peer_refs()
2025-01-31 15:30 ` [PATCH v4 3/6] clone: refactor wanted_peer_refs() Toon Claes
@ 2025-02-03 7:51 ` Patrick Steinhardt
0 siblings, 0 replies; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-03 7:51 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King
On Fri, Jan 31, 2025 at 04:30:31PM +0100, Toon Claes wrote:
> The function wanted_peer_refs() is used to map the refs returned by the
> server to refs we will save in our clone.
>
> Over time this function grown to be very complex. Refactor it.
The diff of this commit is a bit on the harder side to read, so it would
be nice if the message guided the reader a bit.
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> builtin/clone.c | 39 +++++++++++++++------------------------
> 1 file changed, 15 insertions(+), 24 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4..d652682494d0d27dd73cd0585e28b23f2883786d 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -434,46 +434,37 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
[snip]
> - if (!option_mirror && !option_single_branch && option_tags)
> + for (int i = 0; i < refspec->nr; i++)
While at it: this should be `size_t i` to match the type of
`refspec->nr`.
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 4/6] clone: add tags refspec earlier to fetch refspec
2025-01-31 15:30 ` [PATCH v4 0/6] Enable doing a shallow clone of a specific git revision Toon Claes
` (2 preceding siblings ...)
2025-01-31 15:30 ` [PATCH v4 3/6] clone: refactor wanted_peer_refs() Toon Claes
@ 2025-01-31 15:30 ` 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
` (2 subsequent siblings)
6 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-01-31 15:30 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
In clone.c we call refspec_ref_prefixes() to copy the fetch refspecs
from the `remote->fetch` refspec into `ref_prefixes` of
`transport_ls_refs_options`. Afterward we add the tags prefix
`refs/tags/` prefix as well. At a later point, in wanted_peer_refs() we
process refs using both `remote->fetch` and `TAG_REFSPEC`.
Simplify the code by appending `TAG_REFSPEC` to `remote->fetch` before
calling refspec_ref_prefixes().
To be able to do this, we set `option_tags` to 0 when --mirror is given.
This is because --mirror mirrors (hence the name) all the refs,
including tags and they do not need to be treated separately.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index d652682494d0d27dd73cd0585e28b23f2883786d..7ab156ac00240de89baca6533ed2541839286fc4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -435,11 +435,8 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
struct ref **tail = local_refs ? &local_refs->next : &local_refs;
- struct refspec_item tag_refspec;
struct ref *to_free = NULL;
- refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
-
if (option_single_branch) {
if (!option_branch)
refs = to_free = guess_remote_head(head, refs, 0);
@@ -454,16 +451,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
for (int i = 0; i < refspec->nr; i++)
get_fetch_map(refs, &refspec->items[i], &tail, 0);
- /*
- * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
- * about won't be present in `refs` anyway.
- * Except with option --mirror, where we grab all refs already.
- */
- if (!option_mirror)
- get_fetch_map(refs, &tag_refspec, &tail, 0);
-
free_one_ref(to_free);
- refspec_item_clear(&tag_refspec);
return local_refs;
}
@@ -1011,8 +999,10 @@ int cmd_clone(int argc,
die(_("unknown ref storage format '%s'"), ref_format);
}
- if (option_mirror)
+ if (option_mirror) {
option_bare = 1;
+ option_tags = 0;
+ }
if (option_bare) {
if (real_git_dir)
@@ -1375,14 +1365,15 @@ int cmd_clone(int argc,
transport->smart_options->check_self_contained_and_connected = 1;
strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
+ if (option_tags || option_branch)
+ refspec_append(&remote->fetch, TAG_REFSPEC);
+
refspec_ref_prefixes(&remote->fetch,
&transport_ls_refs_options.ref_prefixes);
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (option_tags)
- strvec_push(&transport_ls_refs_options.ref_prefixes,
- "refs/tags/");
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
--
2.48.1.164.g9a5474a648.dirty
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v4 4/6] clone: add tags refspec earlier to fetch refspec
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
0 siblings, 0 replies; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-03 7:51 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King
On Fri, Jan 31, 2025 at 04:30:32PM +0100, Toon Claes wrote:
> In clone.c we call refspec_ref_prefixes() to copy the fetch refspecs
> from the `remote->fetch` refspec into `ref_prefixes` of
> `transport_ls_refs_options`. Afterward we add the tags prefix
s/Afterward/&s/
> diff --git a/builtin/clone.c b/builtin/clone.c
> index d652682494d0d27dd73cd0585e28b23f2883786d..7ab156ac00240de89baca6533ed2541839286fc4 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -1375,14 +1365,15 @@ int cmd_clone(int argc,
> transport->smart_options->check_self_contained_and_connected = 1;
>
> strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
> +
> + if (option_tags || option_branch)
> + refspec_append(&remote->fetch, TAG_REFSPEC);
It's a bit surprising that we also do this with `option_branch`, which
only seems to indicate which branch git-clone(1) is supposed to check
out. But in fact, the documentation mentions that it may also be used to
check out a tag. Principle of least surprise at its best.
In any case, I think it would be nice to have a comment here explaining
why this is the correct thing to do.
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 5/6] clone: introduce struct clone_opts in builtin/clone.c
2025-01-31 15:30 ` [PATCH v4 0/6] Enable doing a shallow clone of a specific git revision Toon Claes
` (3 preceding siblings ...)
2025-01-31 15:30 ` [PATCH v4 4/6] clone: add tags refspec earlier to fetch refspec Toon Claes
@ 2025-01-31 15:30 ` 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-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
6 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-01-31 15:30 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
There is a lot of state stored in global variables in builtin/clone.c.
In the long run we'd like to remove many of those.
Introduce `struct clone_opts` in this file. This struct will be used to
contain all details needed to perform the clone. The struct object can
be thrown around to all the functions that need these details.
The first field we're adding is `wants_head`. In some scenarios
(specifically when both `--single-branch` and `--branch` are given) we
are not interested in `HEAD` on the remote. The field `wants_head` in
`struct clone_opts` will hold this information. We could have put
`option_branch` and `option_single_branch` into that struct instead, but
in a following commit we'll be using `wants_head` as well.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 47 ++++++++++++++++++++++++++++++++---------------
remote.c | 2 +-
remote.h | 1 +
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 7ab156ac00240de89baca6533ed2541839286fc4..f92017c751dd31cb25a3ba31667b015d5766ce84 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -57,6 +57,13 @@
*
*/
+struct clone_opts {
+ int wants_head;
+};
+#define CLONE_OPTS_INIT { \
+ .wants_head = 1 /* default enabled */ \
+}
+
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_tags = 1; /* default enabled */
@@ -429,23 +436,27 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch
return ref;
}
-static struct ref *wanted_peer_refs(const struct ref *refs,
- struct refspec *refspec)
+static struct ref *wanted_peer_refs(struct clone_opts *opts,
+ const struct ref *refs,
+ struct refspec *refspec)
{
- struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
- struct ref *local_refs = head;
- struct ref **tail = local_refs ? &local_refs->next : &local_refs;
+ struct ref *local_refs = NULL;
+ struct ref **tail = &local_refs;
struct ref *to_free = NULL;
- if (option_single_branch) {
- if (!option_branch)
+ if (opts->wants_head) {
+ struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
+ if (head)
+ tail_link_ref(head, &tail);
+
+ if (option_single_branch)
refs = to_free = guess_remote_head(head, refs, 0);
- else {
- free_one_ref(head);
- local_refs = head = NULL;
- tail = &local_refs;
- refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
- }
+ }
+
+ else if (option_single_branch) {
+ local_refs = NULL;
+ tail = &local_refs;
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
for (int i = 0; i < refspec->nr; i++)
@@ -893,6 +904,8 @@ int cmd_clone(int argc,
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ struct clone_opts opts = CLONE_OPTS_INIT;
+
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ -1343,9 +1356,13 @@ int cmd_clone(int argc,
if (option_not.nr)
transport_set_option(transport, TRANS_OPT_DEEPEN_NOT,
(const char *)&option_not);
- if (option_single_branch)
+ if (option_single_branch) {
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
+ if (option_branch)
+ opts.wants_head = 0;
+ }
+
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
@@ -1450,7 +1467,7 @@ int cmd_clone(int argc,
}
if (refs)
- mapped_refs = wanted_peer_refs(refs, &remote->fetch);
+ mapped_refs = wanted_peer_refs(&opts, refs, &remote->fetch);
if (mapped_refs) {
/*
diff --git a/remote.c b/remote.c
index 0f6fba85625b523122e50e28a0f64b6e143cd9fb..cf823764826222bab84d27ae665f7292da44edc6 100644
--- a/remote.c
+++ b/remote.c
@@ -1234,7 +1234,7 @@ int count_refspec_match(const char *pattern,
}
}
-static void tail_link_ref(struct ref *ref, struct ref ***tail)
+void tail_link_ref(struct ref *ref, struct ref ***tail)
{
**tail = ref;
while (ref->next)
diff --git a/remote.h b/remote.h
index bda10dd5c85ffd8988a6c3d39583e7b9701278b8..65d2de2c2051cf28def8b43fcca3fd5e1c86a0d8 100644
--- a/remote.h
+++ b/remote.h
@@ -219,6 +219,7 @@ struct ref *alloc_ref(const char *name);
struct ref *copy_ref(const struct ref *ref);
struct ref *copy_ref_list(const struct ref *ref);
int count_refspec_match(const char *, struct ref *refs, struct ref **matched_ref);
+void tail_link_ref(struct ref *ref, struct ref ***tail);
int check_ref_type(const struct ref *ref, int flags);
--
2.48.1.164.g9a5474a648.dirty
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v4 5/6] clone: introduce struct clone_opts in builtin/clone.c
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
0 siblings, 0 replies; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-03 7:51 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King
On Fri, Jan 31, 2025 at 04:30:33PM +0100, Toon Claes wrote:
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 7ab156ac00240de89baca6533ed2541839286fc4..f92017c751dd31cb25a3ba31667b015d5766ce84 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -429,23 +436,27 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch
> return ref;
> }
>
> -static struct ref *wanted_peer_refs(const struct ref *refs,
> - struct refspec *refspec)
> +static struct ref *wanted_peer_refs(struct clone_opts *opts,
> + const struct ref *refs,
> + struct refspec *refspec)
> {
> - struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
> - struct ref *local_refs = head;
> - struct ref **tail = local_refs ? &local_refs->next : &local_refs;
> + struct ref *local_refs = NULL;
> + struct ref **tail = &local_refs;
> struct ref *to_free = NULL;
>
> - if (option_single_branch) {
> - if (!option_branch)
> + if (opts->wants_head) {
> + struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
> + if (head)
> + tail_link_ref(head, &tail);
> +
> + if (option_single_branch)
> refs = to_free = guess_remote_head(head, refs, 0);
> - else {
> - free_one_ref(head);
> - local_refs = head = NULL;
> - tail = &local_refs;
> - refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
> - }
> + }
> +
> + else if (option_single_branch) {
Formatting: the `else if` should be on the same line as the closing
brace.
> + local_refs = NULL;
> + tail = &local_refs;
> + refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
> }
>
> for (int i = 0; i < refspec->nr; i++)
> @@ -893,6 +904,8 @@ int cmd_clone(int argc,
> struct string_list server_options = STRING_LIST_INIT_NODUP;
> const char *bundle_uri = NULL;
>
> + struct clone_opts opts = CLONE_OPTS_INIT;
> +
Nit: There's a bit too many empty lines to my taste, without a clear
reason why.
> struct transport_ls_refs_options transport_ls_refs_options =
> TRANSPORT_LS_REFS_OPTIONS_INIT;
>
> @@ -1343,9 +1356,13 @@ int cmd_clone(int argc,
> if (option_not.nr)
> transport_set_option(transport, TRANS_OPT_DEEPEN_NOT,
> (const char *)&option_not);
> - if (option_single_branch)
> + if (option_single_branch) {
> transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
>
> + if (option_branch)
> + opts.wants_head = 0;
Makes sense. If we only want to clone a single branch, and we specify
which branch that is, then there is no need to fetch the remote HEAD.
> + }
> +
> if (option_upload_pack)
> transport_set_option(transport, TRANS_OPT_UPLOADPACK,
> option_upload_pack);
> @@ -1450,7 +1467,7 @@ int cmd_clone(int argc,
> }
>
> if (refs)
> - mapped_refs = wanted_peer_refs(refs, &remote->fetch);
> + mapped_refs = wanted_peer_refs(&opts, refs, &remote->fetch);
>
> if (mapped_refs) {
> /*
> index bda10dd5c85ffd8988a6c3d39583e7b9701278b8..65d2de2c2051cf28def8b43fcca3fd5e1c86a0d8 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -219,6 +219,7 @@ struct ref *alloc_ref(const char *name);
> struct ref *copy_ref(const struct ref *ref);
> struct ref *copy_ref_list(const struct ref *ref);
> int count_refspec_match(const char *, struct ref *refs, struct ref **matched_ref);
> +void tail_link_ref(struct ref *ref, struct ref ***tail);
Can we maybe add a comment explaining what this function does?
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option
2025-01-31 15:30 ` [PATCH v4 0/6] Enable doing a shallow clone of a specific git revision Toon Claes
` (4 preceding siblings ...)
2025-01-31 15:30 ` [PATCH v4 5/6] clone: introduce struct clone_opts in builtin/clone.c Toon Claes
@ 2025-01-31 15:30 ` Toon Claes
2025-01-31 21:05 ` Junio C Hamano
` (2 more replies)
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
6 siblings, 3 replies; 57+ messages in thread
From: Toon Claes @ 2025-01-31 15:30 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
The git-clone(1) command has the option `--branch` that allows the user
to select the branch they want HEAD to point to. In a non-bare
repository this also checks out that branch.
Option `--branch` also accepts a tag. When a tag name is provided, the
commit this tag points to is checked out and HEAD is detached. Thus
`--branch` can be used to clone a repository and check out a ref kept
under `refs/heads` or `refs/tags`. But some other refs might be in use
as well. For example Git forges might use refs like `refs/pull/<id>` and
`refs/merge-requests/<id>` to track pull/merge requests. These refs
cannot be selected upon git-clone(1).
Add option `--revision` to git-clone(1). This option accepts a fully
qualified reference, or a hexadecimal commit ID. This enables the user
to clone and check out any revision they want. `--revision` can be used
in conjunction with `--depth` to do a minimal clone that only contains
the blob and tree for a single revision. This can be useful for
automated tests running in CI systems.
Using option `--branch` and `--single-branch` together is a similar
scenario, but serves a different purpose. Using these two options, a
singlet remote tracking branch is created and the fetch refspec is set
up so git-fetch(1) will receive updates on that branch from the remote.
This allows the user work on that single branch.
Option `--revision` on contrary detaches HEAD, creates no tracking
branches, and writes no fetch refspec.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 10 ++++
builtin/clone.c | 59 +++++++++++++++++----
parse-options.h | 9 ++++
t/meson.build | 1 +
t/t5621-clone-revision.sh | 123 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 193 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 99a9222e63429b3447deb3e7c08962d4ec60a295..6c882b6fc56c2595059124cd0ecdaf825e310160 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -221,6 +221,16 @@ objects from the source repository into a pack in the cloned repository.
`--branch` can also take tags and detaches the `HEAD` at that commit
in the resulting repository.
+`--revision` _<rev>_::
+ Create a new repository, and fetch the history leading to the given
+ revision _<rev>_ (and nothing else), without making any remote-tracking
+ branch, and without making any local branch, and point `HEAD` to
+ _<rev>_. When creating a non-bare repository, the revision is checked
+ out on a detached `HEAD`. The argument can be a ref name
+ (e.g. `refs/heads/main` or `refs/tags/v1.0`) that peels down to a
+ commit, or a hexadecimal object name.
+ This option is incompatible with `--branch` and `--mirror`.
+
`-u` _<upload-pack>_::
`--upload-pack` _<upload-pack>_::
When given, and the repository to clone from is accessed
diff --git a/builtin/clone.c b/builtin/clone.c
index f92017c751dd31cb25a3ba31667b015d5766ce84..40d6ecfa74608193a88715a29b4ca765687a0c86 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,6 +59,7 @@
struct clone_opts {
int wants_head;
+ int detach;
};
#define CLONE_OPTS_INIT { \
.wants_head = 1 /* default enabled */ \
@@ -568,11 +569,11 @@ static void update_remote_refs(const struct ref *refs,
}
}
-static void update_head(const struct ref *our, const struct ref *remote,
+static void update_head(struct clone_opts *opts, const struct ref *our, const struct ref *remote,
const char *unborn, const char *msg)
{
const char *head;
- if (our && skip_prefix(our->name, "refs/heads/", &head)) {
+ if (our && !opts->detach && skip_prefix(our->name, "refs/heads/", &head)) {
/* Local default branch link */
if (refs_update_symref(get_main_ref_store(the_repository), "HEAD", our->name, NULL) < 0)
die(_("unable to update HEAD"));
@@ -585,6 +586,10 @@ static void update_head(const struct ref *our, const struct ref *remote,
} else if (our) {
struct commit *c = lookup_commit_reference(the_repository,
&our->old_oid);
+
+ if (!c)
+ die(_("couldn't look up commit object for '%s'"), our->name);
+
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
refs_update_ref(get_main_ref_store(the_repository), msg,
"HEAD", &c->object.oid, NULL, REF_NO_DEREF,
@@ -903,6 +908,7 @@ int cmd_clone(int argc,
int option_filter_submodules = -1; /* unspecified */
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ char *option_rev = NULL;
struct clone_opts opts = CLONE_OPTS_INIT;
@@ -946,6 +952,8 @@ int cmd_clone(int argc,
N_("use <name> instead of 'origin' to track upstream")),
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")),
OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
N_("path to git-upload-pack on the remote")),
OPT_STRING(0, "depth", &option_depth, N_("depth"),
@@ -1282,7 +1290,7 @@ int cmd_clone(int argc,
strbuf_addstr(&branch_top, src_ref_prefix);
git_config_set("core.bare", "true");
- } else {
+ } else if (!option_rev) {
strbuf_addf(&branch_top, "refs/remotes/%s/", remote_name);
}
@@ -1301,8 +1309,9 @@ int cmd_clone(int argc,
remote = remote_get_early(remote_name);
- 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);
path = get_repo_path(remote->url.v[0], &is_bundle);
is_local = option_local != 0 && path && !is_bundle;
@@ -1345,6 +1354,12 @@ int cmd_clone(int argc,
transport_set_option(transport, TRANS_OPT_KEEP, "yes");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ !!option_branch, "--branch");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ option_mirror, "--mirror");
+ // TODO --no-single-branch
+
if (reject_shallow)
transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
if (option_depth)
@@ -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;
+
+ refspec_append(&remote->fetch, option_rev);
+ }
if (option_tags || option_branch)
refspec_append(&remote->fetch, TAG_REFSPEC);
@@ -1392,6 +1415,17 @@ int cmd_clone(int argc,
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
+ /*
+ * As part of transport_get_remote_refs() the server tells us the hash
+ * algorithm, which we require to initialize the repo. But calling that
+ * function without any ref prefix, will cause the server to announce
+ * all known refs. If the argument passed to --revision was a hex oid,
+ * ref_prefixes will be empty so we fall back to asking about HEAD to
+ * reduce traffic from the server.
+ */
+ if (opts.wants_head || transport_ls_refs_options.ref_prefixes.nr == 0)
+ strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
/*
@@ -1500,6 +1534,12 @@ int cmd_clone(int argc,
if (!our_head_points_at)
die(_("Remote branch %s not found in upstream %s"),
option_branch, remote_name);
+ } else if (option_rev) {
+ our_head_points_at = mapped_refs;
+ if (!our_head_points_at)
+ die(_("Remote revision %s not found in upstream %s"),
+ option_rev, remote_name);
+ //mapped_refs->name[0] = 0;
} else if (remote_head_points_at) {
our_head_points_at = remote_head_points_at;
} else if (remote_head) {
@@ -1538,8 +1578,9 @@ int cmd_clone(int argc,
free(to_free);
}
- write_refspec_config(src_ref_prefix, our_head_points_at,
- remote_head_points_at, &branch_top);
+ if (!option_rev)
+ write_refspec_config(src_ref_prefix, our_head_points_at,
+ remote_head_points_at, &branch_top);
if (filter_options.choice)
partial_clone_register(remote_name, &filter_options);
@@ -1555,7 +1596,7 @@ int cmd_clone(int argc,
branch_top.buf, reflog_msg.buf, transport,
!is_local);
- update_head(our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
+ update_head(&opts, our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
/*
* We want to show progress for recursive submodule clones iff
diff --git a/parse-options.h b/parse-options.h
index 39f088625494f20dea96b9a9cbe986916773bf60..fca944d9a93d643d984c58de2ead9154c8b16c94 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -436,6 +436,15 @@ static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,
0, "");
}
+static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
+ int opt2, const char *opt2_name)
+{
+ die_for_incompatible_opt4(opt1, opt1_name,
+ opt2, opt2_name,
+ 0, "",
+ 0, "");
+}
+
/*
* Use these assertions for callbacks that expect to be called with NONEG and
* NOARG respectively, and do not otherwise handle the "unset" and "arg"
diff --git a/t/meson.build b/t/meson.build
index 35f25ca4a1d960564190288e9456620a46ccc80a..b5f917926b61de379b6cef45e5f750912422a7d1 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -721,6 +721,7 @@ integration_tests = [
't5617-clone-submodules-remote.sh',
't5618-alternate-refs.sh',
't5619-clone-local-ambiguous-transport.sh',
+ 't5621-clone-revision.sh',
't5700-protocol-v1.sh',
't5701-git-serve.sh',
't5702-protocol-v2.sh',
diff --git a/t/t5621-clone-revision.sh b/t/t5621-clone-revision.sh
new file mode 100755
index 0000000000000000000000000000000000000000..d4889a954e6300e0e327ebe7dfcf73569d966829
--- /dev/null
+++ b/t/t5621-clone-revision.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+
+test_description='tests for git clone --revision'
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit --no-tag "initial commit" README "Hello" &&
+ test_commit --annotate "second commit" README "Hello world" v1.0 &&
+ test_commit --no-tag "third commit" README "Hello world!" &&
+ git switch -c feature v1.0 &&
+ test_commit --no-tag "feature commit" README "Hello world!" &&
+ git switch main
+'
+
+test_expect_success 'clone with --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --depth and --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --no-local --depth=1 --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch &&
+ git -C dst rev-list HEAD >actual &&
+ test_line_count = 1 actual
+'
+
+test_expect_success 'clone with --revision being a tag' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/tags/v1.0 . dst &&
+ git rev-parse refs/tags/v1.0^{} >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being HEAD' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=HEAD . dst &&
+ git rev-parse HEAD >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature) &&
+ git clone --revision=$oid . dst &&
+ echo $oid >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision and --bare' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/main --bare . dst &&
+ oid=$(git rev-parse refs/heads/main) &&
+ git -C dst cat-file -t $oid >actual &&
+ echo "commit" >expect &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a short raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse --short refs/heads/feature) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "fatal: Remote revision $oid not found in upstream origin" err
+'
+
+test_expect_success 'clone with --revision being a tree hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature^{tree}) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "error: object $oid is a tree, not a commit" err
+'
+
+test_expect_success 'clone with --revision being the parent of a ref fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main^ . dst
+'
+
+test_expect_success 'clone with --revision and --branch fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --branch=main . dst
+'
+
+test_expect_success 'clone with --revision and --mirror fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --mirror . dst
+'
+
+test_done
--
2.48.1.164.g9a5474a648.dirty
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option
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
2025-02-01 16:50 ` Jean-Noël AVILA
2025-02-03 7:51 ` Patrick Steinhardt
2 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2025-01-31 21:05 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek,
Patrick Steinhardt, Jeff King
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.
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option
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
@ 2025-02-01 16:50 ` Jean-Noël AVILA
2025-02-03 7:51 ` Patrick Steinhardt
2 siblings, 0 replies; 57+ messages in thread
From: Jean-Noël AVILA @ 2025-02-01 16:50 UTC (permalink / raw)
To: git, Toon Claes
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Toon Claes
On Friday, 31 January 2025 16:30:34 UTC+1 Toon Claes wrote:
> The git-clone(1) command has the option `--branch` that allows the user
> to select the branch they want HEAD to point to. In a non-bare
> repository this also checks out that branch.
>
> Option `--branch` also accepts a tag. When a tag name is provided, the
> commit this tag points to is checked out and HEAD is detached. Thus
> `--branch` can be used to clone a repository and check out a ref kept
> under `refs/heads` or `refs/tags`. But some other refs might be in use
> as well. For example Git forges might use refs like `refs/pull/<id>` and
> `refs/merge-requests/<id>` to track pull/merge requests. These refs
> cannot be selected upon git-clone(1).
>
> Add option `--revision` to git-clone(1). This option accepts a fully
> qualified reference, or a hexadecimal commit ID. This enables the user
> to clone and check out any revision they want. `--revision` can be used
> in conjunction with `--depth` to do a minimal clone that only contains
> the blob and tree for a single revision. This can be useful for
> automated tests running in CI systems.
>
> Using option `--branch` and `--single-branch` together is a similar
> scenario, but serves a different purpose. Using these two options, a
> singlet remote tracking branch is created and the fetch refspec is set
> up so git-fetch(1) will receive updates on that branch from the remote.
> This allows the user work on that single branch.
>
> Option `--revision` on contrary detaches HEAD, creates no tracking
> branches, and writes no fetch refspec.
>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> Documentation/git-clone.txt | 10 ++++
> builtin/clone.c | 59 +++++++++++++++++----
> parse-options.h | 9 ++++
> t/meson.build | 1 +
> t/t5621-clone-revision.sh | 123
> ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 193
> insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index
>
99a9222e63429b3447deb3e7c08962d4ec60a295..6c882b6fc56c2595059124cd0ecdaf825e3
> 10160 100644 --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -221,6 +221,16 @@ objects from the source repository into a pack in the
> cloned repository. `--branch` can also take tags and detaches the `HEAD` at
> that commit in the resulting repository.
>
> +`--revision` _<rev>_::
You can use the backticks for full synopsis style: `--revision <rev>`
> + Create a new repository, and fetch the history leading to the given
> + revision _<rev>_ (and nothing else), without making any remote-
tracking
> + branch, and without making any local branch, and point `HEAD` to
> + _<rev>_. When creating a non-bare repository, the revision is
checked
> + out on a detached `HEAD`. The argument can be a ref name
> + (e.g. `refs/heads/main` or `refs/tags/v1.0`) that peels down to a
> + commit, or a hexadecimal object name.
> + This option is incompatible with `--branch` and `--mirror`.
> +
> `-u` _<upload-pack>_::
> `--upload-pack` _<upload-pack>_::
> When given, and the repository to clone from is accessed
> diff --git a/builtin/clone.c b/builtin/clone.c
> index
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option
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
2025-02-01 16:50 ` Jean-Noël AVILA
@ 2025-02-03 7:51 ` Patrick Steinhardt
2 siblings, 0 replies; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-03 7:51 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King
On Fri, Jan 31, 2025 at 04:30:34PM +0100, Toon Claes wrote:
> diff --git a/builtin/clone.c b/builtin/clone.c
> index f92017c751dd31cb25a3ba31667b015d5766ce84..40d6ecfa74608193a88715a29b4ca765687a0c86 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -585,6 +586,10 @@ static void update_head(const struct ref *our, const struct ref *remote,
> } else if (our) {
> struct commit *c = lookup_commit_reference(the_repository,
> &our->old_oid);
> +
> + if (!c)
> + die(_("couldn't look up commit object for '%s'"), our->name);
You can use `lookup_commit_or_die()` instead.
> @@ -1500,6 +1534,12 @@ int cmd_clone(int argc,
> if (!our_head_points_at)
> die(_("Remote branch %s not found in upstream %s"),
> option_branch, remote_name);
> + } else if (option_rev) {
> + our_head_points_at = mapped_refs;
> + if (!our_head_points_at)
> + die(_("Remote revision %s not found in upstream %s"),
> + option_rev, remote_name);
> + //mapped_refs->name[0] = 0;
This looks like a left-over debug statement?
> diff --git a/parse-options.h b/parse-options.h
> index 39f088625494f20dea96b9a9cbe986916773bf60..fca944d9a93d643d984c58de2ead9154c8b16c94 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -436,6 +436,15 @@ static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,
> 0, "");
> }
>
> +static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
> + int opt2, const char *opt2_name)
> +{
> + die_for_incompatible_opt4(opt1, opt1_name,
> + opt2, opt2_name,
> + 0, "",
> + 0, "");
> +}
> +
> /*
> * Use these assertions for callbacks that expect to be called with NONEG and
> * NOARG respectively, and do not otherwise handle the "unset" and "arg"
It might make sense to introduce this in a separate commit, ideally with
an example callsite. `grep die\(.*incompatible` surfaces a couple of
candidates.
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision
2025-01-31 15:30 ` [PATCH v4 0/6] Enable doing a shallow clone of a specific git revision Toon Claes
` (5 preceding siblings ...)
2025-01-31 15:30 ` [PATCH v4 6/6] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
@ 2025-02-04 21:33 ` Toon Claes
2025-02-04 21:34 ` [PATCH v5 1/7] clone: cut down on global variables in clone.c Toon Claes
` (9 more replies)
6 siblings, 10 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The goal of this series is to add an option `--revision` to
git-clone(1).
This series starts with a handful of preparatory refactoring commits
that make it more straight-forward to add this new option. In the last
commit we're actually adding the feature.
This series sets an example on how I think we can further refactor
builtin/clone.c to increase the maintainability of the code.
---
Changes in v5:
- Add separate commit to introduce die_for_incompatible_opt2()
- Small tweaks in documentation about `--[no-]tags` and `--revision`.
- Better explain the refactoring of wanted_peer_refs() in the commit
message.
- Change type from `int` to `size_t` in wanted_peer_refs().
- Use lookup_commit_or_die() instead lookup_commit_reference() to avoid
checking the result ourself.
- Add a few code comments to explain some things.
- Stylish cleanups like removal of unneeded empty lines, commented out
test-code and remarks.
- Link to v4: https://lore.kernel.org/r/20250131-toon-clone-refs-v4-0-2a4ff851498f@iotcl.com
Changes in v4:
- Introduce a new commit to reduce the use of global variables.
- Introduce a new commit to invert the flag --no-tags to --tags.
- Introduce a new commit to refactor wanted_peer_refs() in
builtin/clone.c.
- Introduce a new commit to shuffle the handling of tags refspec.
- Introduce a new commit to introduce a `struct clone_opts`.
- Link to v3: https://lore.kernel.org/r/20241219-toon-clone-refs-v3-1-1484faea3008@iotcl.com
Changes in v3:
- Fail early when the revision was not found on the remote, instead of
creating a clone that's in an invalid state.
- State more clearly in the commit message adding this option is useful
for a not uncommon use-case.
- Be explicit in the documentation the ref needs to peel down to a
commit.
- Die in case we try to update_head() to an object that's not a commit.
- Allow combining `--revision` with `--bare`.
- Add die_for_incompatible_opt2() to parse-options.h and use it for the
options that are not compatible with the new `--revision` option.
- Small tweaks to the added tests.
- Small touchups on commit messages.
- Link to v2: https://lore.kernel.org/r/20241129-toon-clone-refs-v2-1-dca4c19a3510@iotcl.com
---
Toon Claes (7):
clone: cut down on global variables in clone.c
clone: make it possible to specify --tags
clone: refactor wanted_peer_refs()
clone: add tags refspec earlier to fetch refspec
clone: introduce struct clone_opts in builtin/clone.c
parse-options: introduce die_for_incompatible_opt2()
builtin/clone: teach git-clone(1) the --revision= option
Documentation/git-clone.txt | 17 ++-
builtin/clone.c | 350 +++++++++++++++++++++++++-------------------
builtin/replay.c | 3 +-
parse-options.h | 9 ++
remote.c | 2 +-
remote.h | 5 +
t/meson.build | 1 +
t/t5621-clone-revision.sh | 123 ++++++++++++++++
8 files changed, 351 insertions(+), 159 deletions(-)
---
Range-diff versus v4:
1: a563ae1023 = 1: 43fbfe2a1c clone: cut down on global variables in clone.c
2: 031fec1961 ! 2: 68bbf04606 clone: make it possible to specify --tags
@@ Documentation/git-clone.txt: git clone [--template=<template-directory>]
[-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
[--dissociate] [--separate-git-dir <git-dir>]
- [--depth <depth>] [--[no-]single-branch] [--no-tags]
-+ [--depth <depth>] [--[no-]single-branch] [--[no-]-tags]
++ [--depth <depth>] [--[no-]single-branch] [--[no-]tags]
[--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
[--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
[--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository>
@@ Documentation/git-clone.txt: corresponding `--mirror` and `--no-tags` options in
-`--no-tags`::
- Don't clone any tags, and set
+`--[no-]tags`::
-+ By default tags are cloned, and passing `--tags` doesn't change that.
+ With `--no-tags`, no tags are cloned, and set
`remote.<remote>.tagOpt=--no-tags` in the config, ensuring
that future `git pull` and `git fetch` operations won't follow
any tags. Subsequent explicit tag fetches will still work,
+ (see linkgit:git-fetch[1]).
++ By default tags are cloned, and passing `--tags` doesn't change that.
+ +
+ Can be used in conjunction with `--single-branch` to clone and
+ maintain a branch with no references other than a single cloned
## builtin/clone.c ##
@@
3: b926bcc1df ! 3: ac0babfc2a clone: refactor wanted_peer_refs()
@@ Commit message
Over time this function grown to be very complex. Refactor it.
+ Previously, there was a separate code path for when
+ `option_single_branch` was set. It resulted in duplicated code and
+ deeper nested conditions. After this refactor the code path for when
+ `option_single_branch` is truthy modifies `refs` and then falls through
+ to the common code path. This approach relies on the `refspec` being set
+ correctly and thus only mapping refs that are relevant.
+
Signed-off-by: Toon Claes <toon@iotcl.com>
## builtin/clone.c ##
@@ builtin/clone.c: static struct ref *wanted_peer_refs(const struct ref *refs,
}
- if (!option_mirror && !option_single_branch && option_tags)
-+ for (int i = 0; i < refspec->nr; i++)
++ for (size_t i = 0; i < refspec->nr; i++)
+ get_fetch_map(refs, &refspec->items[i], &tail, 0);
+
+ /*
4: 2201b996b3 ! 4: 8a98961fd5 clone: add tags refspec earlier to fetch refspec
@@ Commit message
In clone.c we call refspec_ref_prefixes() to copy the fetch refspecs
from the `remote->fetch` refspec into `ref_prefixes` of
- `transport_ls_refs_options`. Afterward we add the tags prefix
+ `transport_ls_refs_options`. Afterwards we add the tags prefix
`refs/tags/` prefix as well. At a later point, in wanted_peer_refs() we
process refs using both `remote->fetch` and `TAG_REFSPEC`.
@@ builtin/clone.c: static struct ref *wanted_peer_refs(const struct ref *refs,
if (!option_branch)
refs = to_free = guess_remote_head(head, refs, 0);
@@ builtin/clone.c: static struct ref *wanted_peer_refs(const struct ref *refs,
- for (int i = 0; i < refspec->nr; i++)
+ for (size_t i = 0; i < refspec->nr; i++)
get_fetch_map(refs, &refspec->items[i], &tail, 0);
- /*
@@ builtin/clone.c: int cmd_clone(int argc,
strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
+ if (option_tags || option_branch)
++ /*
++ * Add tags refspec when user asked for tags (implicitly) or
++ * specified --branch, which argument might be a tag.
++ */
+ refspec_append(&remote->fetch, TAG_REFSPEC);
+
refspec_ref_prefixes(&remote->fetch,
5: 14fb827c41 ! 5: b338eec186 clone: introduce struct clone_opts in builtin/clone.c
@@ builtin/clone.c: static struct ref *find_remote_branch(const struct ref *refs, c
+ struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
+ if (head)
+ tail_link_ref(head, &tail);
-+
+ if (option_single_branch)
refs = to_free = guess_remote_head(head, refs, 0);
- else {
@@ builtin/clone.c: static struct ref *find_remote_branch(const struct ref *refs, c
- tail = &local_refs;
- refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
- }
-+ }
-+
-+ else if (option_single_branch) {
++ } else if (option_single_branch) {
+ local_refs = NULL;
+ tail = &local_refs;
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
- for (int i = 0; i < refspec->nr; i++)
+ for (size_t i = 0; i < refspec->nr; i++)
@@ builtin/clone.c: int cmd_clone(int argc,
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
@@ remote.h: struct ref *alloc_ref(const char *name);
struct ref *copy_ref(const struct ref *ref);
struct ref *copy_ref_list(const struct ref *ref);
int count_refspec_match(const char *, struct ref *refs, struct ref **matched_ref);
++/*
++ * Put a ref in the tail and prepare tail for adding another one.
++ * *tail is the pointer to the tail of the list of refs.
++ */
+void tail_link_ref(struct ref *ref, struct ref ***tail);
int check_ref_type(const struct ref *ref, int flags);
-: ---------- > 6: d312865d63 parse-options: introduce die_for_incompatible_opt2()
6: d87d155dfc ! 7: 8ddbc6eb41 builtin/clone: teach git-clone(1) the --revision= option
@@ Documentation/git-clone.txt: objects from the source repository into a pack in t
`--branch` can also take tags and detaches the `HEAD` at that commit
in the resulting repository.
-+`--revision` _<rev>_::
++`--revision=<rev>`::
+ Create a new repository, and fetch the history leading to the given
+ revision _<rev>_ (and nothing else), without making any remote-tracking
+ branch, and without making any local branch, and point `HEAD` to
@@ builtin/clone.c: static void update_remote_refs(const struct ref *refs,
if (refs_update_symref(get_main_ref_store(the_repository), "HEAD", our->name, NULL) < 0)
die(_("unable to update HEAD"));
@@ builtin/clone.c: static void update_head(const struct ref *our, const struct ref *remote,
+ install_branch_config(0, head, remote_name, our->name);
+ }
} else if (our) {
- struct commit *c = lookup_commit_reference(the_repository,
- &our->old_oid);
-+
-+ if (!c)
-+ die(_("couldn't look up commit object for '%s'"), our->name);
+- struct commit *c = lookup_commit_reference(the_repository,
+- &our->old_oid);
++ struct commit *c = lookup_commit_or_die(&our->old_oid,
++ our->name);
+
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
refs_update_ref(get_main_ref_store(the_repository), msg,
@@ builtin/clone.c: int cmd_clone(int argc,
+ !!option_branch, "--branch");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ option_mirror, "--mirror");
-+ // TODO --no-single-branch
+
if (reject_shallow)
transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
@@ builtin/clone.c: int cmd_clone(int argc,
- 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;
@@ builtin/clone.c: int cmd_clone(int argc,
+ }
if (option_tags || option_branch)
- refspec_append(&remote->fetch, TAG_REFSPEC);
+ /*
@@ builtin/clone.c: int cmd_clone(int argc,
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
@@ builtin/clone.c: int cmd_clone(int argc,
+ if (!our_head_points_at)
+ die(_("Remote revision %s not found in upstream %s"),
+ option_rev, remote_name);
-+ //mapped_refs->name[0] = 0;
} else if (remote_head_points_at) {
our_head_points_at = remote_head_points_at;
} else if (remote_head) {
@@ builtin/clone.c: int cmd_clone(int argc,
/*
* We want to show progress for recursive submodule clones iff
- ## parse-options.h ##
-@@ parse-options.h: static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,
- 0, "");
- }
-
-+static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
-+ int opt2, const char *opt2_name)
-+{
-+ die_for_incompatible_opt4(opt1, opt1_name,
-+ opt2, opt2_name,
-+ 0, "",
-+ 0, "");
-+}
-+
- /*
- * Use these assertions for callbacks that expect to be called with NONEG and
- * NOARG respectively, and do not otherwise handle the "unset" and "arg"
-
## t/meson.build ##
@@ t/meson.build: integration_tests = [
't5617-clone-submodules-remote.sh',
---
base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
change-id: 20241129-toon-clone-refs-ad3623772f92
Thanks
--
Toon
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v5 1/7] clone: cut down on global variables in clone.c
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 ` Toon Claes
2025-02-04 21:34 ` [PATCH v5 2/7] clone: make it possible to specify --tags Toon Claes
` (8 subsequent siblings)
9 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
In clone.c the `struct option` which is used to parse the input options
for git-clone(1) is a global variable. Due to this, many variables that
are used to parse the value into, are also global.
Make `builtin_clone_options` a local variable in cmd_clone() and carry
along all variables that are only used in that function.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 195 +++++++++++++++++++++++++++++---------------------------
1 file changed, 101 insertions(+), 94 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index fd001d800c635e46bbc7027a8fdda2a8c9fbf069..5ed0802f1d0ddebaf512aac93bf8c8b340494323 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,42 +56,22 @@
* - dropping use-separate-remote and no-separate-remote compatibility
*
*/
-static const char * const builtin_clone_usage[] = {
- N_("git clone [<options>] [--] <repo> [<dir>]"),
- NULL
-};
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_no_tags;
static int option_shallow_submodules;
-static int option_reject_shallow = -1; /* unspecified */
static int config_reject_shallow = -1; /* unspecified */
-static int deepen;
-static char *option_template, *option_depth, *option_since;
-static char *option_origin = NULL;
static char *remote_name = NULL;
static char *option_branch = NULL;
-static struct string_list option_not = STRING_LIST_INIT_NODUP;
-static const char *real_git_dir;
-static const char *ref_format;
-static const char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
-static int option_progress = -1;
-static int option_sparse_checkout;
-static enum transport_family family;
-static struct string_list option_config = STRING_LIST_INIT_NODUP;
static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
-static int option_dissociate;
static int max_jobs = -1;
static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;
static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
-static int option_filter_submodules = -1; /* unspecified */
static int config_filter_submodules = -1; /* unspecified */
-static struct string_list server_options = STRING_LIST_INIT_NODUP;
static int option_remote_submodules;
-static const char *bundle_uri;
static int recurse_submodules_cb(const struct option *opt,
const char *arg, int unset)
@@ -107,78 +87,6 @@ static int recurse_submodules_cb(const struct option *opt,
return 0;
}
-static struct option builtin_clone_options[] = {
- OPT__VERBOSITY(&option_verbosity),
- OPT_BOOL(0, "progress", &option_progress,
- N_("force progress reporting")),
- OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
- N_("don't clone shallow repository")),
- OPT_BOOL('n', "no-checkout", &option_no_checkout,
- N_("don't create a checkout")),
- OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
- OPT_HIDDEN_BOOL(0, "naked", &option_bare,
- N_("create a bare repository")),
- OPT_BOOL(0, "mirror", &option_mirror,
- N_("create a mirror repository (implies --bare)")),
- OPT_BOOL('l', "local", &option_local,
- N_("to clone from a local repository")),
- OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
- N_("don't use local hardlinks, always copy")),
- OPT_BOOL('s', "shared", &option_shared,
- N_("setup as shared repository")),
- { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
- N_("pathspec"), N_("initialize submodules in the clone"),
- PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
- OPT_ALIAS(0, "recursive", "recurse-submodules"),
- OPT_INTEGER('j', "jobs", &max_jobs,
- N_("number of submodules cloned in parallel")),
- OPT_STRING(0, "template", &option_template, N_("template-directory"),
- N_("directory from which templates will be used")),
- OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
- N_("reference repository")),
- OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
- N_("repo"), N_("reference repository")),
- OPT_BOOL(0, "dissociate", &option_dissociate,
- N_("use --reference only while cloning")),
- OPT_STRING('o', "origin", &option_origin, N_("name"),
- N_("use <name> instead of 'origin' to track upstream")),
- OPT_STRING('b', "branch", &option_branch, N_("branch"),
- N_("checkout <branch> instead of the remote's HEAD")),
- OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
- N_("path to git-upload-pack on the remote")),
- OPT_STRING(0, "depth", &option_depth, N_("depth"),
- N_("create a shallow clone of that depth")),
- OPT_STRING(0, "shallow-since", &option_since, N_("time"),
- N_("create a shallow clone since a specific time")),
- OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
- N_("deepen history of shallow clone, excluding ref")),
- OPT_BOOL(0, "single-branch", &option_single_branch,
- N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
- OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
- N_("any cloned submodules will be shallow")),
- OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
- N_("separate git dir from working tree")),
- OPT_STRING(0, "ref-format", &ref_format, N_("format"),
- N_("specify the reference format to use")),
- OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
- N_("set config inside the new repository")),
- OPT_STRING_LIST(0, "server-option", &server_options,
- N_("server-specific"), N_("option to transmit")),
- OPT_IPVERSION(&family),
- OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
- OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
- N_("apply partial clone filters to submodules")),
- OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
- N_("any cloned submodules will use their remote-tracking branch")),
- OPT_BOOL(0, "sparse", &option_sparse_checkout,
- N_("initialize sparse-checkout file to include only files at root")),
- OPT_STRING(0, "bundle-uri", &bundle_uri,
- N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
- OPT_END()
-};
-
static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
{
static const char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -989,10 +897,103 @@ int cmd_clone(int argc,
int hash_algo;
enum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
const int do_not_override_repo_unix_permissions = -1;
+ int option_reject_shallow = -1; /* unspecified */
+ int deepen = 0;
+ char *option_template = NULL, *option_depth = NULL, *option_since = NULL;
+ char *option_origin = NULL;
+ struct string_list option_not = STRING_LIST_INIT_NODUP;
+ const char *real_git_dir = NULL;
+ const char *ref_format = NULL;
+ const char *option_upload_pack = "git-upload-pack";
+ int option_progress = -1;
+ int option_sparse_checkout = 0;
+ enum transport_family family = TRANSPORT_FAMILY_ALL;
+ struct string_list option_config = STRING_LIST_INIT_DUP;
+ int option_dissociate = 0;
+ int option_filter_submodules = -1; /* unspecified */
+ struct string_list server_options = STRING_LIST_INIT_NODUP;
+ const char *bundle_uri = NULL;
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
+ struct option builtin_clone_options[] = {
+ OPT__VERBOSITY(&option_verbosity),
+ OPT_BOOL(0, "progress", &option_progress,
+ N_("force progress reporting")),
+ OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
+ N_("don't clone shallow repository")),
+ OPT_BOOL('n', "no-checkout", &option_no_checkout,
+ N_("don't create a checkout")),
+ OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
+ OPT_HIDDEN_BOOL(0, "naked", &option_bare,
+ N_("create a bare repository")),
+ OPT_BOOL(0, "mirror", &option_mirror,
+ N_("create a mirror repository (implies --bare)")),
+ OPT_BOOL('l', "local", &option_local,
+ N_("to clone from a local repository")),
+ OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
+ N_("don't use local hardlinks, always copy")),
+ OPT_BOOL('s', "shared", &option_shared,
+ N_("setup as shared repository")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
+ N_("pathspec"), N_("initialize submodules in the clone"),
+ PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
+ OPT_ALIAS(0, "recursive", "recurse-submodules"),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("number of submodules cloned in parallel")),
+ OPT_STRING(0, "template", &option_template, N_("template-directory"),
+ N_("directory from which templates will be used")),
+ OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
+ N_("reference repository")),
+ OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
+ N_("repo"), N_("reference repository")),
+ OPT_BOOL(0, "dissociate", &option_dissociate,
+ N_("use --reference only while cloning")),
+ OPT_STRING('o', "origin", &option_origin, N_("name"),
+ N_("use <name> instead of 'origin' to track upstream")),
+ OPT_STRING('b', "branch", &option_branch, N_("branch"),
+ N_("checkout <branch> instead of the remote's HEAD")),
+ OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
+ N_("path to git-upload-pack on the remote")),
+ OPT_STRING(0, "depth", &option_depth, N_("depth"),
+ N_("create a shallow clone of that depth")),
+ OPT_STRING(0, "shallow-since", &option_since, N_("time"),
+ N_("create a shallow clone since a specific time")),
+ OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
+ N_("deepen history of shallow clone, excluding ref")),
+ OPT_BOOL(0, "single-branch", &option_single_branch,
+ N_("clone only one branch, HEAD or --branch")),
+ OPT_BOOL(0, "no-tags", &option_no_tags,
+ N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
+ N_("any cloned submodules will be shallow")),
+ OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
+ N_("separate git dir from working tree")),
+ OPT_STRING(0, "ref-format", &ref_format, N_("format"),
+ N_("specify the reference format to use")),
+ OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
+ N_("set config inside the new repository")),
+ OPT_STRING_LIST(0, "server-option", &server_options,
+ N_("server-specific"), N_("option to transmit")),
+ OPT_IPVERSION(&family),
+ OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
+ OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
+ N_("apply partial clone filters to submodules")),
+ OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
+ N_("any cloned submodules will use their remote-tracking branch")),
+ OPT_BOOL(0, "sparse", &option_sparse_checkout,
+ N_("initialize sparse-checkout file to include only files at root")),
+ OPT_STRING(0, "bundle-uri", &bundle_uri,
+ N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
+ OPT_END()
+ };
+
+ const char * const builtin_clone_usage[] = {
+ N_("git clone [<options>] [--] <repo> [<dir>]"),
+ NULL
+ };
+
packet_trace_identity("clone");
git_config(git_clone_config, NULL);
@@ -1138,8 +1139,8 @@ int cmd_clone(int argc,
for_each_string_list_item(item, &option_recurse_submodules) {
strbuf_addf(&sb, "submodule.active=%s",
item->string);
- string_list_append(&option_config,
- strbuf_detach(&sb, NULL));
+ string_list_append(&option_config, sb.buf);
+ strbuf_reset(&sb);
}
if (!git_config_get_bool("submodule.stickyRecursiveClone", &val) &&
@@ -1161,6 +1162,8 @@ int cmd_clone(int argc,
string_list_append(&option_config,
"submodule.alternateErrorStrategy=info");
}
+
+ strbuf_release(&sb);
}
/*
@@ -1578,6 +1581,10 @@ int cmd_clone(int argc,
err = checkout(submodule_progress, filter_submodules,
ref_storage_format);
+ string_list_clear(&option_not, 0);
+ string_list_clear(&option_config, 0);
+ string_list_clear(&server_options, 0);
+
free(remote_name);
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v5 2/7] clone: make it possible to specify --tags
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 ` Toon Claes
2025-02-05 8:03 ` Patrick Steinhardt
2025-02-04 21:34 ` [PATCH v5 3/7] clone: refactor wanted_peer_refs() Toon Claes
` (7 subsequent siblings)
9 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
Option --no-tags was added in 0dab2468ee (clone: add a --no-tags option
to clone without tags, 2017-04-26). At the time there was no need to
support --tags as well, although there was some conversation about
it[1].
To simplify the code and to prepare for future commits, invert the flag
internally. Functionally there is no change, because the flag is
default-enabled passing `--tags` has no effect, so there's no need to
add tests for this.
[1]: https://lore.kernel.org/git/CAGZ79kbHuMpiavJ90kQLEL_AR0BEyArcZoEWAjPPhOFacN16YQ@mail.gmail.com/
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 7 ++++---
builtin/clone.c | 14 +++++++-------
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index de8d8f58930ecff305f79480b13ddce10cd96c60..e89ae2e8c664f4d4e15e5f05264c8df988295e3d 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -13,7 +13,7 @@ git clone [--template=<template-directory>]
[-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
[-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
[--dissociate] [--separate-git-dir <git-dir>]
- [--depth <depth>] [--[no-]single-branch] [--no-tags]
+ [--depth <depth>] [--[no-]single-branch] [--[no-]tags]
[--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
[--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
[--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository>
@@ -273,12 +273,13 @@ corresponding `--mirror` and `--no-tags` options instead.
branch when `--single-branch` clone was made, no remote-tracking
branch is created.
-`--no-tags`::
- Don't clone any tags, and set
+`--[no-]tags`::
+ With `--no-tags`, no tags are cloned, and set
`remote.<remote>.tagOpt=--no-tags` in the config, ensuring
that future `git pull` and `git fetch` operations won't follow
any tags. Subsequent explicit tag fetches will still work,
(see linkgit:git-fetch[1]).
+ By default tags are cloned, and passing `--tags` doesn't change that.
+
Can be used in conjunction with `--single-branch` to clone and
maintain a branch with no references other than a single cloned
diff --git a/builtin/clone.c b/builtin/clone.c
index 5ed0802f1d0ddebaf512aac93bf8c8b340494323..69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,7 +59,7 @@
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
-static int option_no_tags;
+static int option_tags = 1; /* default enabled */
static int option_shallow_submodules;
static int config_reject_shallow = -1; /* unspecified */
static char *remote_name = NULL;
@@ -470,7 +470,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && !option_no_tags)
+ if (!option_mirror && !option_single_branch && option_tags)
get_fetch_map(refs, &tag_refspec, &tail, 0);
refspec_item_clear(&tag_refspec);
@@ -562,7 +562,7 @@ static void update_remote_refs(const struct ref *refs,
if (refs) {
write_remote_refs(mapped_refs);
- if (option_single_branch && !option_no_tags)
+ if (option_single_branch && option_tags)
write_followtags(refs, msg);
}
@@ -964,8 +964,8 @@ int cmd_clone(int argc,
N_("deepen history of shallow clone, excluding ref")),
OPT_BOOL(0, "single-branch", &option_single_branch,
N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "tags", &option_tags,
+ N_("clone tags, and make later fetches not to follow them")),
OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
N_("any cloned submodules will be shallow")),
OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
@@ -1296,7 +1296,7 @@ int cmd_clone(int argc,
git_config_set(key.buf, repo);
strbuf_reset(&key);
- if (option_no_tags) {
+ if (!option_tags) {
strbuf_addf(&key, "remote.%s.tagOpt", remote_name);
git_config_set(key.buf, "--no-tags");
strbuf_reset(&key);
@@ -1389,7 +1389,7 @@ int cmd_clone(int argc,
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (!option_no_tags)
+ if (option_tags)
strvec_push(&transport_ls_refs_options.ref_prefixes,
"refs/tags/");
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v5 2/7] clone: make it possible to specify --tags
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
0 siblings, 2 replies; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-05 8:03 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano
On Tue, Feb 04, 2025 at 10:34:01PM +0100, Toon Claes wrote:
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index de8d8f58930ecff305f79480b13ddce10cd96c60..e89ae2e8c664f4d4e15e5f05264c8df988295e3d 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -273,12 +273,13 @@ corresponding `--mirror` and `--no-tags` options instead.
> branch when `--single-branch` clone was made, no remote-tracking
> branch is created.
>
> -`--no-tags`::
> - Don't clone any tags, and set
> +`--[no-]tags`::
> + With `--no-tags`, no tags are cloned, and set
> `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
This reads a bit awkward now. How about:
Control whether or not tags will be cloned. When `--no-tags` is
given, the option will be become permanent by setting the
`remote.<remote>.tagOpt=--no-tags` configuration. This ensures that
future `git pull` and `git fetch` won't follow any tags. Subsequent
explicit tag fetches will still work (see linkgit:git-fetch[1]).
By default, tags are cloned and passing `--tags` is thus typically a
no-op, unless it cancels out a previous `--no-tags`.
> that future `git pull` and `git fetch` operations won't follow
> any tags. Subsequent explicit tag fetches will still work,
> (see linkgit:git-fetch[1]).
> + By default tags are cloned, and passing `--tags` doesn't change that.
I think it would make sense to have an empty line before this addition.
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 2/7] clone: make it possible to specify --tags
2025-02-05 8:03 ` Patrick Steinhardt
@ 2025-02-05 16:29 ` Toon Claes
2025-02-05 21:15 ` Jean-Noël AVILA
1 sibling, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:29 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano, Jean-Noël AVILA
Patrick Steinhardt <ps@pks.im> writes:
> On Tue, Feb 04, 2025 at 10:34:01PM +0100, Toon Claes wrote:
>> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
>> index de8d8f58930ecff305f79480b13ddce10cd96c60..e89ae2e8c664f4d4e15e5f05264c8df988295e3d 100644
>> --- a/Documentation/git-clone.txt
>> +++ b/Documentation/git-clone.txt
>> @@ -273,12 +273,13 @@ corresponding `--mirror` and `--no-tags` options instead.
>> branch when `--single-branch` clone was made, no remote-tracking
>> branch is created.
>>
>> -`--no-tags`::
>> - Don't clone any tags, and set
>> +`--[no-]tags`::
>> + With `--no-tags`, no tags are cloned, and set
>> `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
>
> This reads a bit awkward now. How about:
>
> Control whether or not tags will be cloned. When `--no-tags` is
> given, the option will be become permanent by setting the
> `remote.<remote>.tagOpt=--no-tags` configuration. This ensures that
> future `git pull` and `git fetch` won't follow any tags. Subsequent
> explicit tag fetches will still work (see linkgit:git-fetch[1]).
>
> By default, tags are cloned and passing `--tags` is thus typically a
> no-op, unless it cancels out a previous `--no-tags`.
Sounds good to me, I'll amend my commit.
I'm Cc'ing Jean-Noël to hear their opinion as well.
--
Toon
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 2/7] clone: make it possible to specify --tags
2025-02-05 8:03 ` Patrick Steinhardt
2025-02-05 16:29 ` Toon Claes
@ 2025-02-05 21:15 ` Jean-Noël AVILA
1 sibling, 0 replies; 57+ messages in thread
From: Jean-Noël AVILA @ 2025-02-05 21:15 UTC (permalink / raw)
To: Toon Claes, Patrick Steinhardt
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano
On Wednesday, 5 February 2025 09:03:21 UTC+1 Patrick Steinhardt wrote:
> On Tue, Feb 04, 2025 at 10:34:01PM +0100, Toon Claes wrote:
> > diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> > index
> > de8d8f58930ecff305f79480b13ddce10cd96c60..e89ae2e8c664f4d4e15e5f05264c8df98
> > 8295e3d 100644 --- a/Documentation/git-clone.txt
> > +++ b/Documentation/git-clone.txt
> > @@ -273,12 +273,13 @@ corresponding `--mirror` and `--no-tags` options
> > instead.>
> > branch when `--single-branch` clone was made, no remote-tracking
> > branch is created.
> >
> > -`--no-tags`::
> > - Don't clone any tags, and set
> > +`--[no-]tags`::
> > + With `--no-tags`, no tags are cloned, and set
> >
> > `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
>
> This reads a bit awkward now. How about:
>
> Control whether or not tags will be cloned. When `--no-tags` is
The future form is useless, as it applies to the current command run.
> given, the option will be become permanent by setting the
"will be become permanent": I do not understand this part. If I parse it
correctly, this a passive future perfect form, but it does not relate to the
current state.
> `remote.<remote>.tagOpt=--no-tags` configuration. This ensures that
> future `git pull` and `git fetch` won't follow any tags. Subsequent
> explicit tag fetches will still work (see linkgit:git-fetch[1]).
>
> By default, tags are cloned and passing `--tags` is thus typically a
> no-op, unless it cancels out a previous `--no-tags`.
>
If you let an empty new line followed by an indented paragraph, this creates a
citation paragraph. What you want is to attach it, like so:
(...)
explicit tag fetches will still work (see linkgit:git-fetch[1]).
+
By default, tags are cloned and passing `--tags` is thus typically a
no-op, unless it cancels out a previous `--no-tags`.
JN
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v5 3/7] clone: refactor wanted_peer_refs()
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-04 21:34 ` Toon Claes
2025-02-04 21:34 ` [PATCH v5 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
` (6 subsequent siblings)
9 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The function wanted_peer_refs() is used to map the refs returned by the
server to refs we will save in our clone.
Over time this function grown to be very complex. Refactor it.
Previously, there was a separate code path for when
`option_single_branch` was set. It resulted in duplicated code and
deeper nested conditions. After this refactor the code path for when
`option_single_branch` is truthy modifies `refs` and then falls through
to the common code path. This approach relies on the `refspec` being set
correctly and thus only mapping refs that are relevant.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4..5efa2bbceb42b230f723660ea963ca1253888235 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -434,46 +434,37 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
{
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
- struct ref **tail = head ? &head->next : &local_refs;
+ struct ref **tail = local_refs ? &local_refs->next : &local_refs;
struct refspec_item tag_refspec;
+ struct ref *to_free = NULL;
refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
if (option_single_branch) {
- struct ref *remote_head = NULL;
-
if (!option_branch)
- remote_head = guess_remote_head(head, refs, 0);
+ refs = to_free = guess_remote_head(head, refs, 0);
else {
free_one_ref(head);
local_refs = head = NULL;
tail = &local_refs;
- remote_head = copy_ref(find_remote_branch(refs, option_branch));
- }
-
- if (!remote_head && option_branch)
- warning(_("Could not find remote branch %s to clone."),
- option_branch);
- else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(remote_head, &refspec->items[i],
- &tail, 0);
-
- /* if --branch=tag, pull the requested tag explicitly */
- get_fetch_map(remote_head, &tag_refspec, &tail, 0);
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
- free_refs(remote_head);
- } else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && option_tags)
+ for (size_t i = 0; i < refspec->nr; i++)
+ get_fetch_map(refs, &refspec->items[i], &tail, 0);
+
+ /*
+ * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
+ * about won't be present in `refs` anyway.
+ * Except with option --mirror, where we grab all refs already.
+ */
+ if (!option_mirror)
get_fetch_map(refs, &tag_refspec, &tail, 0);
+ free_one_ref(to_free);
refspec_item_clear(&tag_refspec);
+
return local_refs;
}
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v5 4/7] clone: add tags refspec earlier to fetch refspec
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (2 preceding siblings ...)
2025-02-04 21:34 ` [PATCH v5 3/7] clone: refactor wanted_peer_refs() Toon Claes
@ 2025-02-04 21:34 ` 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
` (5 subsequent siblings)
9 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
In clone.c we call refspec_ref_prefixes() to copy the fetch refspecs
from the `remote->fetch` refspec into `ref_prefixes` of
`transport_ls_refs_options`. Afterwards we add the tags prefix
`refs/tags/` prefix as well. At a later point, in wanted_peer_refs() we
process refs using both `remote->fetch` and `TAG_REFSPEC`.
Simplify the code by appending `TAG_REFSPEC` to `remote->fetch` before
calling refspec_ref_prefixes().
To be able to do this, we set `option_tags` to 0 when --mirror is given.
This is because --mirror mirrors (hence the name) all the refs,
including tags and they do not need to be treated separately.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 5efa2bbceb42b230f723660ea963ca1253888235..1419b82a7bdd3d91cc08667a854b84ba33d1e7aa 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -435,11 +435,8 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
struct ref **tail = local_refs ? &local_refs->next : &local_refs;
- struct refspec_item tag_refspec;
struct ref *to_free = NULL;
- refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
-
if (option_single_branch) {
if (!option_branch)
refs = to_free = guess_remote_head(head, refs, 0);
@@ -454,16 +451,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
for (size_t i = 0; i < refspec->nr; i++)
get_fetch_map(refs, &refspec->items[i], &tail, 0);
- /*
- * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
- * about won't be present in `refs` anyway.
- * Except with option --mirror, where we grab all refs already.
- */
- if (!option_mirror)
- get_fetch_map(refs, &tag_refspec, &tail, 0);
-
free_one_ref(to_free);
- refspec_item_clear(&tag_refspec);
return local_refs;
}
@@ -1011,8 +999,10 @@ int cmd_clone(int argc,
die(_("unknown ref storage format '%s'"), ref_format);
}
- if (option_mirror)
+ if (option_mirror) {
option_bare = 1;
+ option_tags = 0;
+ }
if (option_bare) {
if (real_git_dir)
@@ -1375,14 +1365,19 @@ int cmd_clone(int argc,
transport->smart_options->check_self_contained_and_connected = 1;
strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
+ if (option_tags || option_branch)
+ /*
+ * Add tags refspec when user asked for tags (implicitly) or
+ * specified --branch, which argument might be a tag.
+ */
+ refspec_append(&remote->fetch, TAG_REFSPEC);
+
refspec_ref_prefixes(&remote->fetch,
&transport_ls_refs_options.ref_prefixes);
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (option_tags)
- strvec_push(&transport_ls_refs_options.ref_prefixes,
- "refs/tags/");
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v5 5/7] clone: introduce struct clone_opts in builtin/clone.c
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (3 preceding siblings ...)
2025-02-04 21:34 ` [PATCH v5 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
@ 2025-02-04 21:34 ` Toon Claes
2025-02-04 21:34 ` [PATCH v5 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
` (4 subsequent siblings)
9 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
There is a lot of state stored in global variables in builtin/clone.c.
In the long run we'd like to remove many of those.
Introduce `struct clone_opts` in this file. This struct will be used to
contain all details needed to perform the clone. The struct object can
be thrown around to all the functions that need these details.
The first field we're adding is `wants_head`. In some scenarios
(specifically when both `--single-branch` and `--branch` are given) we
are not interested in `HEAD` on the remote. The field `wants_head` in
`struct clone_opts` will hold this information. We could have put
`option_branch` and `option_single_branch` into that struct instead, but
in a following commit we'll be using `wants_head` as well.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 44 +++++++++++++++++++++++++++++---------------
remote.c | 2 +-
remote.h | 5 +++++
3 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 1419b82a7bdd3d91cc08667a854b84ba33d1e7aa..275940f4f5037de65d0f33eba3e7bc031f9b122b 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -57,6 +57,13 @@
*
*/
+struct clone_opts {
+ int wants_head;
+};
+#define CLONE_OPTS_INIT { \
+ .wants_head = 1 /* default enabled */ \
+}
+
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_tags = 1; /* default enabled */
@@ -429,23 +436,24 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch
return ref;
}
-static struct ref *wanted_peer_refs(const struct ref *refs,
- struct refspec *refspec)
+static struct ref *wanted_peer_refs(struct clone_opts *opts,
+ const struct ref *refs,
+ struct refspec *refspec)
{
- struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
- struct ref *local_refs = head;
- struct ref **tail = local_refs ? &local_refs->next : &local_refs;
+ struct ref *local_refs = NULL;
+ struct ref **tail = &local_refs;
struct ref *to_free = NULL;
- if (option_single_branch) {
- if (!option_branch)
+ if (opts->wants_head) {
+ struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
+ if (head)
+ tail_link_ref(head, &tail);
+ if (option_single_branch)
refs = to_free = guess_remote_head(head, refs, 0);
- else {
- free_one_ref(head);
- local_refs = head = NULL;
- tail = &local_refs;
- refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
- }
+ } else if (option_single_branch) {
+ local_refs = NULL;
+ tail = &local_refs;
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
for (size_t i = 0; i < refspec->nr; i++)
@@ -893,6 +901,8 @@ int cmd_clone(int argc,
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ struct clone_opts opts = CLONE_OPTS_INIT;
+
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ -1343,9 +1353,13 @@ int cmd_clone(int argc,
if (option_not.nr)
transport_set_option(transport, TRANS_OPT_DEEPEN_NOT,
(const char *)&option_not);
- if (option_single_branch)
+ if (option_single_branch) {
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
+ if (option_branch)
+ opts.wants_head = 0;
+ }
+
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
@@ -1454,7 +1468,7 @@ int cmd_clone(int argc,
}
if (refs)
- mapped_refs = wanted_peer_refs(refs, &remote->fetch);
+ mapped_refs = wanted_peer_refs(&opts, refs, &remote->fetch);
if (mapped_refs) {
/*
diff --git a/remote.c b/remote.c
index 1779f0e7bbb8b88965f2fedf407e50ed20ea7a13..69d8c43ea694f1b9a7699e5d5a49bfc169058b8e 100644
--- a/remote.c
+++ b/remote.c
@@ -1260,7 +1260,7 @@ int count_refspec_match(const char *pattern,
}
}
-static void tail_link_ref(struct ref *ref, struct ref ***tail)
+void tail_link_ref(struct ref *ref, struct ref ***tail)
{
**tail = ref;
while (ref->next)
diff --git a/remote.h b/remote.h
index a19353f68999f5440db7bf5f91dd4be8bcc1d8a5..ce3e7c8512981d0ac5db2da508c4fbe64cede961 100644
--- a/remote.h
+++ b/remote.h
@@ -221,6 +221,11 @@ struct ref *alloc_ref(const char *name);
struct ref *copy_ref(const struct ref *ref);
struct ref *copy_ref_list(const struct ref *ref);
int count_refspec_match(const char *, struct ref *refs, struct ref **matched_ref);
+/*
+ * Put a ref in the tail and prepare tail for adding another one.
+ * *tail is the pointer to the tail of the list of refs.
+ */
+void tail_link_ref(struct ref *ref, struct ref ***tail);
int check_ref_type(const struct ref *ref, int flags);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v5 6/7] parse-options: introduce die_for_incompatible_opt2()
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (4 preceding siblings ...)
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 ` 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
` (3 subsequent siblings)
9 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The functions die_for_incompatible_opt3() and
die_for_incompatible_opt4() already exist to die whenever a user
specifies three or four options respectively that are not compatible.
Introduce die_for_incompatible_opt2() which dies when two options that
are incompatible are set.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/replay.c | 3 ++-
parse-options.h | 9 +++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index 1afc6d1ee0cb738fa7fa3f2b5c8ce0dd7802e7da..03d93afd77290ab556565f05bd424956b66ff01c 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -164,7 +164,8 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
if (!rinfo.positive_refexprs)
die(_("need some commits to replay"));
if (onto_name && *advance_name)
- die(_("--onto and --advance are incompatible"));
+ die_for_incompatible_opt2(!!onto_name, "--onto",
+ !!*advance_name, "--advance");
else if (onto_name) {
*onto = peel_committish(onto_name);
if (rinfo.positive_refexprs <
diff --git a/parse-options.h b/parse-options.h
index 39f088625494f20dea96b9a9cbe986916773bf60..fca944d9a93d643d984c58de2ead9154c8b16c94 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -436,6 +436,15 @@ static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,
0, "");
}
+static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
+ int opt2, const char *opt2_name)
+{
+ die_for_incompatible_opt4(opt1, opt1_name,
+ opt2, opt2_name,
+ 0, "",
+ 0, "");
+}
+
/*
* Use these assertions for callbacks that expect to be called with NONEG and
* NOARG respectively, and do not otherwise handle the "unset" and "arg"
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v5 6/7] parse-options: introduce die_for_incompatible_opt2()
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
0 siblings, 0 replies; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-05 8:03 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano
On Tue, Feb 04, 2025 at 10:34:05PM +0100, Toon Claes wrote:
> diff --git a/builtin/replay.c b/builtin/replay.c
> index 1afc6d1ee0cb738fa7fa3f2b5c8ce0dd7802e7da..03d93afd77290ab556565f05bd424956b66ff01c 100644
> --- a/builtin/replay.c
> +++ b/builtin/replay.c
> @@ -164,7 +164,8 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
> if (!rinfo.positive_refexprs)
> die(_("need some commits to replay"));
> if (onto_name && *advance_name)
> - die(_("--onto and --advance are incompatible"));
> + die_for_incompatible_opt2(!!onto_name, "--onto",
> + !!*advance_name, "--advance");
The condition isn't needed anymore, is it? As far as I know,
`die_for_incompatible_opt*()` handle the condition internally.
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v5 7/7] builtin/clone: teach git-clone(1) the --revision= option
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (5 preceding siblings ...)
2025-02-04 21:34 ` [PATCH v5 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
@ 2025-02-04 21:34 ` Toon Claes
2025-02-05 8:03 ` Patrick Steinhardt
2025-02-05 8:03 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Patrick Steinhardt
` (2 subsequent siblings)
9 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-02-04 21:34 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The git-clone(1) command has the option `--branch` that allows the user
to select the branch they want HEAD to point to. In a non-bare
repository this also checks out that branch.
Option `--branch` also accepts a tag. When a tag name is provided, the
commit this tag points to is checked out and HEAD is detached. Thus
`--branch` can be used to clone a repository and check out a ref kept
under `refs/heads` or `refs/tags`. But some other refs might be in use
as well. For example Git forges might use refs like `refs/pull/<id>` and
`refs/merge-requests/<id>` to track pull/merge requests. These refs
cannot be selected upon git-clone(1).
Add option `--revision` to git-clone(1). This option accepts a fully
qualified reference, or a hexadecimal commit ID. This enables the user
to clone and check out any revision they want. `--revision` can be used
in conjunction with `--depth` to do a minimal clone that only contains
the blob and tree for a single revision. This can be useful for
automated tests running in CI systems.
Using option `--branch` and `--single-branch` together is a similar
scenario, but serves a different purpose. Using these two options, a
singlet remote tracking branch is created and the fetch refspec is set
up so git-fetch(1) will receive updates on that branch from the remote.
This allows the user work on that single branch.
Option `--revision` on contrary detaches HEAD, creates no tracking
branches, and writes no fetch refspec.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 10 ++++
builtin/clone.c | 57 ++++++++++++++++----
t/meson.build | 1 +
t/t5621-clone-revision.sh | 123 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 180 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index e89ae2e8c664f4d4e15e5f05264c8df988295e3d..7bf6adb98350a9202b01a58f86ee210d9cb85abc 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -221,6 +221,16 @@ objects from the source repository into a pack in the cloned repository.
`--branch` can also take tags and detaches the `HEAD` at that commit
in the resulting repository.
+`--revision=<rev>`::
+ Create a new repository, and fetch the history leading to the given
+ revision _<rev>_ (and nothing else), without making any remote-tracking
+ branch, and without making any local branch, and point `HEAD` to
+ _<rev>_. When creating a non-bare repository, the revision is checked
+ out on a detached `HEAD`. The argument can be a ref name
+ (e.g. `refs/heads/main` or `refs/tags/v1.0`) that peels down to a
+ commit, or a hexadecimal object name.
+ This option is incompatible with `--branch` and `--mirror`.
+
`-u` _<upload-pack>_::
`--upload-pack` _<upload-pack>_::
When given, and the repository to clone from is accessed
diff --git a/builtin/clone.c b/builtin/clone.c
index 275940f4f5037de65d0f33eba3e7bc031f9b122b..e66105a62186740845d2014e088ff7cb97ca30f2 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,6 +59,7 @@
struct clone_opts {
int wants_head;
+ int detach;
};
#define CLONE_OPTS_INIT { \
.wants_head = 1 /* default enabled */ \
@@ -565,11 +566,11 @@ static void update_remote_refs(const struct ref *refs,
}
}
-static void update_head(const struct ref *our, const struct ref *remote,
+static void update_head(struct clone_opts *opts, const struct ref *our, const struct ref *remote,
const char *unborn, const char *msg)
{
const char *head;
- if (our && skip_prefix(our->name, "refs/heads/", &head)) {
+ if (our && !opts->detach && skip_prefix(our->name, "refs/heads/", &head)) {
/* Local default branch link */
if (refs_update_symref(get_main_ref_store(the_repository), "HEAD", our->name, NULL) < 0)
die(_("unable to update HEAD"));
@@ -580,8 +581,9 @@ static void update_head(const struct ref *our, const struct ref *remote,
install_branch_config(0, head, remote_name, our->name);
}
} else if (our) {
- struct commit *c = lookup_commit_reference(the_repository,
- &our->old_oid);
+ struct commit *c = lookup_commit_or_die(&our->old_oid,
+ our->name);
+
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
refs_update_ref(get_main_ref_store(the_repository), msg,
"HEAD", &c->object.oid, NULL, REF_NO_DEREF,
@@ -900,6 +902,7 @@ int cmd_clone(int argc,
int option_filter_submodules = -1; /* unspecified */
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ char *option_rev = NULL;
struct clone_opts opts = CLONE_OPTS_INIT;
@@ -943,6 +946,8 @@ int cmd_clone(int argc,
N_("use <name> instead of 'origin' to track upstream")),
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")),
OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
N_("path to git-upload-pack on the remote")),
OPT_STRING(0, "depth", &option_depth, N_("depth"),
@@ -1279,7 +1284,7 @@ int cmd_clone(int argc,
strbuf_addstr(&branch_top, src_ref_prefix);
git_config_set("core.bare", "true");
- } else {
+ } else if (!option_rev) {
strbuf_addf(&branch_top, "refs/remotes/%s/", remote_name);
}
@@ -1298,8 +1303,9 @@ int cmd_clone(int argc,
remote = remote_get_early(remote_name);
- 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);
path = get_repo_path(remote->url.v[0], &is_bundle);
is_local = option_local != 0 && path && !is_bundle;
@@ -1342,6 +1348,11 @@ int cmd_clone(int argc,
transport_set_option(transport, TRANS_OPT_KEEP, "yes");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ !!option_branch, "--branch");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ option_mirror, "--mirror");
+
if (reject_shallow)
transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
if (option_depth)
@@ -1378,7 +1389,14 @@ 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_single_branch = 0;
+ opts.wants_head = 0;
+ opts.detach = 1;
+
+ refspec_append(&remote->fetch, option_rev);
+ }
if (option_tags || option_branch)
/*
@@ -1393,6 +1411,17 @@ int cmd_clone(int argc,
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
+ /*
+ * As part of transport_get_remote_refs() the server tells us the hash
+ * algorithm, which we require to initialize the repo. But calling that
+ * function without any ref prefix, will cause the server to announce
+ * all known refs. If the argument passed to --revision was a hex oid,
+ * ref_prefixes will be empty so we fall back to asking about HEAD to
+ * reduce traffic from the server.
+ */
+ if (opts.wants_head || transport_ls_refs_options.ref_prefixes.nr == 0)
+ strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
/*
@@ -1501,6 +1530,11 @@ int cmd_clone(int argc,
if (!our_head_points_at)
die(_("Remote branch %s not found in upstream %s"),
option_branch, remote_name);
+ } else if (option_rev) {
+ our_head_points_at = mapped_refs;
+ if (!our_head_points_at)
+ die(_("Remote revision %s not found in upstream %s"),
+ option_rev, remote_name);
} else if (remote_head_points_at) {
our_head_points_at = remote_head_points_at;
} else if (remote_head) {
@@ -1539,8 +1573,9 @@ int cmd_clone(int argc,
free(to_free);
}
- write_refspec_config(src_ref_prefix, our_head_points_at,
- remote_head_points_at, &branch_top);
+ if (!option_rev)
+ write_refspec_config(src_ref_prefix, our_head_points_at,
+ remote_head_points_at, &branch_top);
if (filter_options.choice)
partial_clone_register(remote_name, &filter_options);
@@ -1556,7 +1591,7 @@ int cmd_clone(int argc,
branch_top.buf, reflog_msg.buf, transport,
!is_local);
- update_head(our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
+ update_head(&opts, our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
/*
* We want to show progress for recursive submodule clones iff
diff --git a/t/meson.build b/t/meson.build
index 35f25ca4a1d960564190288e9456620a46ccc80a..b5f917926b61de379b6cef45e5f750912422a7d1 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -721,6 +721,7 @@ integration_tests = [
't5617-clone-submodules-remote.sh',
't5618-alternate-refs.sh',
't5619-clone-local-ambiguous-transport.sh',
+ 't5621-clone-revision.sh',
't5700-protocol-v1.sh',
't5701-git-serve.sh',
't5702-protocol-v2.sh',
diff --git a/t/t5621-clone-revision.sh b/t/t5621-clone-revision.sh
new file mode 100755
index 0000000000000000000000000000000000000000..d4889a954e6300e0e327ebe7dfcf73569d966829
--- /dev/null
+++ b/t/t5621-clone-revision.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+
+test_description='tests for git clone --revision'
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit --no-tag "initial commit" README "Hello" &&
+ test_commit --annotate "second commit" README "Hello world" v1.0 &&
+ test_commit --no-tag "third commit" README "Hello world!" &&
+ git switch -c feature v1.0 &&
+ test_commit --no-tag "feature commit" README "Hello world!" &&
+ git switch main
+'
+
+test_expect_success 'clone with --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --depth and --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --no-local --depth=1 --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch &&
+ git -C dst rev-list HEAD >actual &&
+ test_line_count = 1 actual
+'
+
+test_expect_success 'clone with --revision being a tag' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/tags/v1.0 . dst &&
+ git rev-parse refs/tags/v1.0^{} >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being HEAD' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=HEAD . dst &&
+ git rev-parse HEAD >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature) &&
+ git clone --revision=$oid . dst &&
+ echo $oid >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision and --bare' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/main --bare . dst &&
+ oid=$(git rev-parse refs/heads/main) &&
+ git -C dst cat-file -t $oid >actual &&
+ echo "commit" >expect &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a short raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse --short refs/heads/feature) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "fatal: Remote revision $oid not found in upstream origin" err
+'
+
+test_expect_success 'clone with --revision being a tree hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature^{tree}) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "error: object $oid is a tree, not a commit" err
+'
+
+test_expect_success 'clone with --revision being the parent of a ref fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main^ . dst
+'
+
+test_expect_success 'clone with --revision and --branch fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --branch=main . dst
+'
+
+test_expect_success 'clone with --revision and --mirror fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --mirror . dst
+'
+
+test_done
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v5 7/7] builtin/clone: teach git-clone(1) the --revision= option
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
0 siblings, 1 reply; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-05 8:03 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano
On Tue, Feb 04, 2025 at 10:34:06PM +0100, Toon Claes wrote:
> @@ -580,8 +581,9 @@ static void update_head(const struct ref *our, const struct ref *remote,
> install_branch_config(0, head, remote_name, our->name);
> }
> } else if (our) {
> - struct commit *c = lookup_commit_reference(the_repository,
> - &our->old_oid);
> + struct commit *c = lookup_commit_or_die(&our->old_oid,
> + our->name);
> +
> /* --branch specifies a non-branch (i.e. tags), detach HEAD */
> refs_update_ref(get_main_ref_store(the_repository), msg,
> "HEAD", &c->object.oid, NULL, REF_NO_DEREF,
I wonder: is this fixing a potential segfault? If so, it might make
sense to split this out into a separate commit and provide a test that
demonstrates the issue. If it doesn't, then the change shouldn't be
necessary, unless I misunderstand.
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 7/7] builtin/clone: teach git-clone(1) the --revision= option
2025-02-05 8:03 ` Patrick Steinhardt
@ 2025-02-05 16:43 ` Toon Claes
0 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:43 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano
Patrick Steinhardt <ps@pks.im> writes:
> On Tue, Feb 04, 2025 at 10:34:06PM +0100, Toon Claes wrote:
>> @@ -580,8 +581,9 @@ static void update_head(const struct ref *our, const struct ref *remote,
>> install_branch_config(0, head, remote_name, our->name);
>> }
>> } else if (our) {
>> - struct commit *c = lookup_commit_reference(the_repository,
>> - &our->old_oid);
>> + struct commit *c = lookup_commit_or_die(&our->old_oid,
>> + our->name);
>> +
>> /* --branch specifies a non-branch (i.e. tags), detach HEAD */
>> refs_update_ref(get_main_ref_store(the_repository), msg,
>> "HEAD", &c->object.oid, NULL, REF_NO_DEREF,
>
> I wonder: is this fixing a potential segfault? If so, it might make
> sense to split this out into a separate commit and provide a test that
> demonstrates the issue. If it doesn't, then the change shouldn't be
> necessary, unless I misunderstand.
This potential segfault would occur for the added test case:
'clone with --revision being a tree hash'
So I can't really split out this change in a separate commit, but the
change is still needed.
--
Toon
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (6 preceding siblings ...)
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 14:09 ` Junio C Hamano
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
9 siblings, 0 replies; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-05 8:03 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano
On Tue, Feb 04, 2025 at 10:33:59PM +0100, Toon Claes wrote:
> Changes in v5:
> - Add separate commit to introduce die_for_incompatible_opt2()
> - Small tweaks in documentation about `--[no-]tags` and `--revision`.
> - Better explain the refactoring of wanted_peer_refs() in the commit
> message.
> - Change type from `int` to `size_t` in wanted_peer_refs().
> - Use lookup_commit_or_die() instead lookup_commit_reference() to avoid
> checking the result ourself.
> - Add a few code comments to explain some things.
> - Stylish cleanups like removal of unneeded empty lines, commented out
> test-code and remarks.
> - Link to v4: https://lore.kernel.org/r/20250131-toon-clone-refs-v4-0-2a4ff851498f@iotcl.com
I've got a couple more nits, but this version looks mostly good to me.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (7 preceding siblings ...)
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
9 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2025-02-05 14:09 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek,
Patrick Steinhardt, Jeff King
Toon Claes <toon@iotcl.com> writes:
> The goal of this series is to add an option `--revision` to
> git-clone(1).
>
> This series starts with a handful of preparatory refactoring commits
> that make it more straight-forward to add this new option. In the last
> commit we're actually adding the feature.
>
> This series sets an example on how I think we can further refactor
> builtin/clone.c to increase the maintainability of the code.
>
> ---
> Changes in v5:
> - Add separate commit to introduce die_for_incompatible_opt2()
> - Small tweaks in documentation about `--[no-]tags` and `--revision`.
> - Better explain the refactoring of wanted_peer_refs() in the commit
> message.
> - Change type from `int` to `size_t` in wanted_peer_refs().
> - Use lookup_commit_or_die() instead lookup_commit_reference() to avoid
> checking the result ourself.
> - Add a few code comments to explain some things.
> - Stylish cleanups like removal of unneeded empty lines, commented out
> test-code and remarks.
> - Link to v4: https://lore.kernel.org/r/20250131-toon-clone-refs-v4-0-2a4ff851498f@iotcl.com
Looking good. Queued.
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v6 0/7] Enable doing a shallow clone of a specific git revision
2025-02-04 21:33 ` [PATCH v5 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (8 preceding siblings ...)
2025-02-05 14:09 ` Junio C Hamano
@ 2025-02-05 16:47 ` Toon Claes
2025-02-05 16:47 ` [PATCH v6 1/7] clone: cut down on global variables in clone.c Toon Claes
` (7 more replies)
9 siblings, 8 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The goal of this series is to add an option `--revision` to
git-clone(1).
This series starts with a handful of preparatory refactoring commits
that make it more straight-forward to add this new option. In the last
commit we're actually adding the feature.
This series sets an example on how I think we can further refactor
builtin/clone.c to increase the maintainability of the code.
---
Changes in v6:
- Rewrite the documentation for git-clone(1) --[no-]tags.
- Remove unneeded conditional around die_for_incompatible_opt2() in
builtin/replay.c.
- Fix typo in code comment in builtin/clone.c.
- Link to v5: https://lore.kernel.org/r/20250204-toon-clone-refs-v5-0-37e34af283c8@iotcl.com
Changes in v5:
- Add separate commit to introduce die_for_incompatible_opt2()
- Small tweaks in documentation about `--[no-]tags` and `--revision`.
- Better explain the refactoring of wanted_peer_refs() in the commit
message.
- Change type from `int` to `size_t` in wanted_peer_refs().
- Use lookup_commit_or_die() instead lookup_commit_reference() to avoid
checking the result ourself.
- Add a few code comments to explain some things.
- Stylish cleanups like removal of unneeded empty lines, commented out
test-code and remarks.
- Link to v4: https://lore.kernel.org/r/20250131-toon-clone-refs-v4-0-2a4ff851498f@iotcl.com
Changes in v4:
- Introduce a new commit to reduce the use of global variables.
- Introduce a new commit to invert the flag --no-tags to --tags.
- Introduce a new commit to refactor wanted_peer_refs() in
builtin/clone.c.
- Introduce a new commit to shuffle the handling of tags refspec.
- Introduce a new commit to introduce a `struct clone_opts`.
- Link to v3: https://lore.kernel.org/r/20241219-toon-clone-refs-v3-1-1484faea3008@iotcl.com
Changes in v3:
- Fail early when the revision was not found on the remote, instead of
creating a clone that's in an invalid state.
- State more clearly in the commit message adding this option is useful
for a not uncommon use-case.
- Be explicit in the documentation the ref needs to peel down to a
commit.
- Die in case we try to update_head() to an object that's not a commit.
- Allow combining `--revision` with `--bare`.
- Add die_for_incompatible_opt2() to parse-options.h and use it for the
options that are not compatible with the new `--revision` option.
- Small tweaks to the added tests.
- Small touchups on commit messages.
- Link to v2: https://lore.kernel.org/r/20241129-toon-clone-refs-v2-1-dca4c19a3510@iotcl.com
---
Toon Claes (7):
clone: cut down on global variables in clone.c
clone: make it possible to specify --tags
clone: refactor wanted_peer_refs()
clone: add tags refspec earlier to fetch refspec
clone: introduce struct clone_opts in builtin/clone.c
parse-options: introduce die_for_incompatible_opt2()
builtin/clone: teach git-clone(1) the --revision= option
Documentation/git-clone.txt | 27 +++-
builtin/clone.c | 350 +++++++++++++++++++++++++-------------------
builtin/replay.c | 7 +-
parse-options.h | 9 ++
remote.c | 2 +-
remote.h | 5 +
t/meson.build | 1 +
t/t5621-clone-revision.sh | 123 ++++++++++++++++
8 files changed, 359 insertions(+), 165 deletions(-)
---
Range-diff versus v5:
1: fa3f2f1f03 = 1: 7d4d3bcc70 clone: cut down on global variables in clone.c
2: f88780b503 ! 2: 284f2fc20f clone: make it possible to specify --tags
@@ Documentation/git-clone.txt: corresponding `--mirror` and `--no-tags` options in
-`--no-tags`::
- Don't clone any tags, and set
+- `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
+- that future `git pull` and `git fetch` operations won't follow
+- any tags. Subsequent explicit tag fetches will still work,
+- (see linkgit:git-fetch[1]).
+`--[no-]tags`::
-+ With `--no-tags`, no tags are cloned, and set
- `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
- that future `git pull` and `git fetch` operations won't follow
- any tags. Subsequent explicit tag fetches will still work,
- (see linkgit:git-fetch[1]).
-+ By default tags are cloned, and passing `--tags` doesn't change that.
++ Control whether or not tags will be cloned. When `--no-tags` is
++ given, the option will be become permanent by setting the
++ `remote.<remote>.tagOpt=--no-tags` configuration. This ensures that
++ future `git pull` and `git fetch` won't follow any tags. Subsequent
++ explicit tag fetches will still work (see linkgit:git-fetch[1]).
++
++ By default, tags are cloned and passing `--tags` is thus typically a
++ no-op, unless it cancels out a previous `--no-tags`.
+
Can be used in conjunction with `--single-branch` to clone and
maintain a branch with no references other than a single cloned
3: 1289437661 = 3: 9114e6123d clone: refactor wanted_peer_refs()
4: 9687626855 ! 4: 52807dfdba clone: add tags refspec earlier to fetch refspec
@@ builtin/clone.c: int cmd_clone(int argc,
+ if (option_tags || option_branch)
+ /*
+ * Add tags refspec when user asked for tags (implicitly) or
-+ * specified --branch, which argument might be a tag.
++ * specified --branch, whose argument might be a tag.
+ */
+ refspec_append(&remote->fetch, TAG_REFSPEC);
+
5: c95f9d4f1b = 5: 3a80a479a6 clone: introduce struct clone_opts in builtin/clone.c
6: 96f92993fd ! 6: 5ef027f7a1 parse-options: introduce die_for_incompatible_opt2()
@@ Commit message
## builtin/replay.c ##
@@ builtin/replay.c: static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
+ get_ref_information(cmd_info, &rinfo);
if (!rinfo.positive_refexprs)
die(_("need some commits to replay"));
- if (onto_name && *advance_name)
+- if (onto_name && *advance_name)
- die(_("--onto and --advance are incompatible"));
-+ die_for_incompatible_opt2(!!onto_name, "--onto",
-+ !!*advance_name, "--advance");
- else if (onto_name) {
+- else if (onto_name) {
++
++ die_for_incompatible_opt2(!!onto_name, "--onto",
++ !!*advance_name, "--advance");
++ if (onto_name) {
*onto = peel_committish(onto_name);
if (rinfo.positive_refexprs <
+ strset_get_size(&rinfo.positive_refs))
## parse-options.h ##
@@ parse-options.h: static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,
7: 9163c9bc85 = 7: 2eaae62e16 builtin/clone: teach git-clone(1) the --revision= option
---
base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
change-id: 20241129-toon-clone-refs-ad3623772f92
Thanks
--
Toon
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v6 1/7] clone: cut down on global variables in clone.c
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
@ 2025-02-05 16:47 ` Toon Claes
2025-02-05 16:47 ` [PATCH v6 2/7] clone: make it possible to specify --tags Toon Claes
` (6 subsequent siblings)
7 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
In clone.c the `struct option` which is used to parse the input options
for git-clone(1) is a global variable. Due to this, many variables that
are used to parse the value into, are also global.
Make `builtin_clone_options` a local variable in cmd_clone() and carry
along all variables that are only used in that function.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 195 +++++++++++++++++++++++++++++---------------------------
1 file changed, 101 insertions(+), 94 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index fd001d800c635e46bbc7027a8fdda2a8c9fbf069..5ed0802f1d0ddebaf512aac93bf8c8b340494323 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,42 +56,22 @@
* - dropping use-separate-remote and no-separate-remote compatibility
*
*/
-static const char * const builtin_clone_usage[] = {
- N_("git clone [<options>] [--] <repo> [<dir>]"),
- NULL
-};
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_no_tags;
static int option_shallow_submodules;
-static int option_reject_shallow = -1; /* unspecified */
static int config_reject_shallow = -1; /* unspecified */
-static int deepen;
-static char *option_template, *option_depth, *option_since;
-static char *option_origin = NULL;
static char *remote_name = NULL;
static char *option_branch = NULL;
-static struct string_list option_not = STRING_LIST_INIT_NODUP;
-static const char *real_git_dir;
-static const char *ref_format;
-static const char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
-static int option_progress = -1;
-static int option_sparse_checkout;
-static enum transport_family family;
-static struct string_list option_config = STRING_LIST_INIT_NODUP;
static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
-static int option_dissociate;
static int max_jobs = -1;
static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;
static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
-static int option_filter_submodules = -1; /* unspecified */
static int config_filter_submodules = -1; /* unspecified */
-static struct string_list server_options = STRING_LIST_INIT_NODUP;
static int option_remote_submodules;
-static const char *bundle_uri;
static int recurse_submodules_cb(const struct option *opt,
const char *arg, int unset)
@@ -107,78 +87,6 @@ static int recurse_submodules_cb(const struct option *opt,
return 0;
}
-static struct option builtin_clone_options[] = {
- OPT__VERBOSITY(&option_verbosity),
- OPT_BOOL(0, "progress", &option_progress,
- N_("force progress reporting")),
- OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
- N_("don't clone shallow repository")),
- OPT_BOOL('n', "no-checkout", &option_no_checkout,
- N_("don't create a checkout")),
- OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
- OPT_HIDDEN_BOOL(0, "naked", &option_bare,
- N_("create a bare repository")),
- OPT_BOOL(0, "mirror", &option_mirror,
- N_("create a mirror repository (implies --bare)")),
- OPT_BOOL('l', "local", &option_local,
- N_("to clone from a local repository")),
- OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
- N_("don't use local hardlinks, always copy")),
- OPT_BOOL('s', "shared", &option_shared,
- N_("setup as shared repository")),
- { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
- N_("pathspec"), N_("initialize submodules in the clone"),
- PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
- OPT_ALIAS(0, "recursive", "recurse-submodules"),
- OPT_INTEGER('j', "jobs", &max_jobs,
- N_("number of submodules cloned in parallel")),
- OPT_STRING(0, "template", &option_template, N_("template-directory"),
- N_("directory from which templates will be used")),
- OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
- N_("reference repository")),
- OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
- N_("repo"), N_("reference repository")),
- OPT_BOOL(0, "dissociate", &option_dissociate,
- N_("use --reference only while cloning")),
- OPT_STRING('o', "origin", &option_origin, N_("name"),
- N_("use <name> instead of 'origin' to track upstream")),
- OPT_STRING('b', "branch", &option_branch, N_("branch"),
- N_("checkout <branch> instead of the remote's HEAD")),
- OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
- N_("path to git-upload-pack on the remote")),
- OPT_STRING(0, "depth", &option_depth, N_("depth"),
- N_("create a shallow clone of that depth")),
- OPT_STRING(0, "shallow-since", &option_since, N_("time"),
- N_("create a shallow clone since a specific time")),
- OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
- N_("deepen history of shallow clone, excluding ref")),
- OPT_BOOL(0, "single-branch", &option_single_branch,
- N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
- OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
- N_("any cloned submodules will be shallow")),
- OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
- N_("separate git dir from working tree")),
- OPT_STRING(0, "ref-format", &ref_format, N_("format"),
- N_("specify the reference format to use")),
- OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
- N_("set config inside the new repository")),
- OPT_STRING_LIST(0, "server-option", &server_options,
- N_("server-specific"), N_("option to transmit")),
- OPT_IPVERSION(&family),
- OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
- OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
- N_("apply partial clone filters to submodules")),
- OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
- N_("any cloned submodules will use their remote-tracking branch")),
- OPT_BOOL(0, "sparse", &option_sparse_checkout,
- N_("initialize sparse-checkout file to include only files at root")),
- OPT_STRING(0, "bundle-uri", &bundle_uri,
- N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
- OPT_END()
-};
-
static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
{
static const char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -989,10 +897,103 @@ int cmd_clone(int argc,
int hash_algo;
enum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
const int do_not_override_repo_unix_permissions = -1;
+ int option_reject_shallow = -1; /* unspecified */
+ int deepen = 0;
+ char *option_template = NULL, *option_depth = NULL, *option_since = NULL;
+ char *option_origin = NULL;
+ struct string_list option_not = STRING_LIST_INIT_NODUP;
+ const char *real_git_dir = NULL;
+ const char *ref_format = NULL;
+ const char *option_upload_pack = "git-upload-pack";
+ int option_progress = -1;
+ int option_sparse_checkout = 0;
+ enum transport_family family = TRANSPORT_FAMILY_ALL;
+ struct string_list option_config = STRING_LIST_INIT_DUP;
+ int option_dissociate = 0;
+ int option_filter_submodules = -1; /* unspecified */
+ struct string_list server_options = STRING_LIST_INIT_NODUP;
+ const char *bundle_uri = NULL;
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
+ struct option builtin_clone_options[] = {
+ OPT__VERBOSITY(&option_verbosity),
+ OPT_BOOL(0, "progress", &option_progress,
+ N_("force progress reporting")),
+ OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
+ N_("don't clone shallow repository")),
+ OPT_BOOL('n', "no-checkout", &option_no_checkout,
+ N_("don't create a checkout")),
+ OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
+ OPT_HIDDEN_BOOL(0, "naked", &option_bare,
+ N_("create a bare repository")),
+ OPT_BOOL(0, "mirror", &option_mirror,
+ N_("create a mirror repository (implies --bare)")),
+ OPT_BOOL('l', "local", &option_local,
+ N_("to clone from a local repository")),
+ OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
+ N_("don't use local hardlinks, always copy")),
+ OPT_BOOL('s', "shared", &option_shared,
+ N_("setup as shared repository")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
+ N_("pathspec"), N_("initialize submodules in the clone"),
+ PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
+ OPT_ALIAS(0, "recursive", "recurse-submodules"),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("number of submodules cloned in parallel")),
+ OPT_STRING(0, "template", &option_template, N_("template-directory"),
+ N_("directory from which templates will be used")),
+ OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
+ N_("reference repository")),
+ OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
+ N_("repo"), N_("reference repository")),
+ OPT_BOOL(0, "dissociate", &option_dissociate,
+ N_("use --reference only while cloning")),
+ OPT_STRING('o', "origin", &option_origin, N_("name"),
+ N_("use <name> instead of 'origin' to track upstream")),
+ OPT_STRING('b', "branch", &option_branch, N_("branch"),
+ N_("checkout <branch> instead of the remote's HEAD")),
+ OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
+ N_("path to git-upload-pack on the remote")),
+ OPT_STRING(0, "depth", &option_depth, N_("depth"),
+ N_("create a shallow clone of that depth")),
+ OPT_STRING(0, "shallow-since", &option_since, N_("time"),
+ N_("create a shallow clone since a specific time")),
+ OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
+ N_("deepen history of shallow clone, excluding ref")),
+ OPT_BOOL(0, "single-branch", &option_single_branch,
+ N_("clone only one branch, HEAD or --branch")),
+ OPT_BOOL(0, "no-tags", &option_no_tags,
+ N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
+ N_("any cloned submodules will be shallow")),
+ OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
+ N_("separate git dir from working tree")),
+ OPT_STRING(0, "ref-format", &ref_format, N_("format"),
+ N_("specify the reference format to use")),
+ OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
+ N_("set config inside the new repository")),
+ OPT_STRING_LIST(0, "server-option", &server_options,
+ N_("server-specific"), N_("option to transmit")),
+ OPT_IPVERSION(&family),
+ OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
+ OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
+ N_("apply partial clone filters to submodules")),
+ OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
+ N_("any cloned submodules will use their remote-tracking branch")),
+ OPT_BOOL(0, "sparse", &option_sparse_checkout,
+ N_("initialize sparse-checkout file to include only files at root")),
+ OPT_STRING(0, "bundle-uri", &bundle_uri,
+ N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
+ OPT_END()
+ };
+
+ const char * const builtin_clone_usage[] = {
+ N_("git clone [<options>] [--] <repo> [<dir>]"),
+ NULL
+ };
+
packet_trace_identity("clone");
git_config(git_clone_config, NULL);
@@ -1138,8 +1139,8 @@ int cmd_clone(int argc,
for_each_string_list_item(item, &option_recurse_submodules) {
strbuf_addf(&sb, "submodule.active=%s",
item->string);
- string_list_append(&option_config,
- strbuf_detach(&sb, NULL));
+ string_list_append(&option_config, sb.buf);
+ strbuf_reset(&sb);
}
if (!git_config_get_bool("submodule.stickyRecursiveClone", &val) &&
@@ -1161,6 +1162,8 @@ int cmd_clone(int argc,
string_list_append(&option_config,
"submodule.alternateErrorStrategy=info");
}
+
+ strbuf_release(&sb);
}
/*
@@ -1578,6 +1581,10 @@ int cmd_clone(int argc,
err = checkout(submodule_progress, filter_submodules,
ref_storage_format);
+ string_list_clear(&option_not, 0);
+ string_list_clear(&option_config, 0);
+ string_list_clear(&server_options, 0);
+
free(remote_name);
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 2/7] clone: make it possible to specify --tags
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 ` Toon Claes
2025-02-05 16:47 ` [PATCH v6 3/7] clone: refactor wanted_peer_refs() Toon Claes
` (5 subsequent siblings)
7 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
Option --no-tags was added in 0dab2468ee (clone: add a --no-tags option
to clone without tags, 2017-04-26). At the time there was no need to
support --tags as well, although there was some conversation about
it[1].
To simplify the code and to prepare for future commits, invert the flag
internally. Functionally there is no change, because the flag is
default-enabled passing `--tags` has no effect, so there's no need to
add tests for this.
[1]: https://lore.kernel.org/git/CAGZ79kbHuMpiavJ90kQLEL_AR0BEyArcZoEWAjPPhOFacN16YQ@mail.gmail.com/
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 17 ++++++++++-------
builtin/clone.c | 14 +++++++-------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index de8d8f58930ecff305f79480b13ddce10cd96c60..8d0476f6dcaf6fed7ccd48a20398556dd4e20722 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -13,7 +13,7 @@ git clone [--template=<template-directory>]
[-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
[-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
[--dissociate] [--separate-git-dir <git-dir>]
- [--depth <depth>] [--[no-]single-branch] [--no-tags]
+ [--depth <depth>] [--[no-]single-branch] [--[no-]tags]
[--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
[--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
[--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository>
@@ -273,12 +273,15 @@ corresponding `--mirror` and `--no-tags` options instead.
branch when `--single-branch` clone was made, no remote-tracking
branch is created.
-`--no-tags`::
- Don't clone any tags, and set
- `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
- that future `git pull` and `git fetch` operations won't follow
- any tags. Subsequent explicit tag fetches will still work,
- (see linkgit:git-fetch[1]).
+`--[no-]tags`::
+ Control whether or not tags will be cloned. When `--no-tags` is
+ given, the option will be become permanent by setting the
+ `remote.<remote>.tagOpt=--no-tags` configuration. This ensures that
+ future `git pull` and `git fetch` won't follow any tags. Subsequent
+ explicit tag fetches will still work (see linkgit:git-fetch[1]).
+
+ By default, tags are cloned and passing `--tags` is thus typically a
+ no-op, unless it cancels out a previous `--no-tags`.
+
Can be used in conjunction with `--single-branch` to clone and
maintain a branch with no references other than a single cloned
diff --git a/builtin/clone.c b/builtin/clone.c
index 5ed0802f1d0ddebaf512aac93bf8c8b340494323..69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,7 +59,7 @@
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
-static int option_no_tags;
+static int option_tags = 1; /* default enabled */
static int option_shallow_submodules;
static int config_reject_shallow = -1; /* unspecified */
static char *remote_name = NULL;
@@ -470,7 +470,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && !option_no_tags)
+ if (!option_mirror && !option_single_branch && option_tags)
get_fetch_map(refs, &tag_refspec, &tail, 0);
refspec_item_clear(&tag_refspec);
@@ -562,7 +562,7 @@ static void update_remote_refs(const struct ref *refs,
if (refs) {
write_remote_refs(mapped_refs);
- if (option_single_branch && !option_no_tags)
+ if (option_single_branch && option_tags)
write_followtags(refs, msg);
}
@@ -964,8 +964,8 @@ int cmd_clone(int argc,
N_("deepen history of shallow clone, excluding ref")),
OPT_BOOL(0, "single-branch", &option_single_branch,
N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "tags", &option_tags,
+ N_("clone tags, and make later fetches not to follow them")),
OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
N_("any cloned submodules will be shallow")),
OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
@@ -1296,7 +1296,7 @@ int cmd_clone(int argc,
git_config_set(key.buf, repo);
strbuf_reset(&key);
- if (option_no_tags) {
+ if (!option_tags) {
strbuf_addf(&key, "remote.%s.tagOpt", remote_name);
git_config_set(key.buf, "--no-tags");
strbuf_reset(&key);
@@ -1389,7 +1389,7 @@ int cmd_clone(int argc,
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (!option_no_tags)
+ if (option_tags)
strvec_push(&transport_ls_refs_options.ref_prefixes,
"refs/tags/");
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 3/7] clone: refactor wanted_peer_refs()
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 ` Toon Claes
2025-02-05 16:47 ` [PATCH v6 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
` (4 subsequent siblings)
7 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The function wanted_peer_refs() is used to map the refs returned by the
server to refs we will save in our clone.
Over time this function grown to be very complex. Refactor it.
Previously, there was a separate code path for when
`option_single_branch` was set. It resulted in duplicated code and
deeper nested conditions. After this refactor the code path for when
`option_single_branch` is truthy modifies `refs` and then falls through
to the common code path. This approach relies on the `refspec` being set
correctly and thus only mapping refs that are relevant.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4..5efa2bbceb42b230f723660ea963ca1253888235 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -434,46 +434,37 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
{
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
- struct ref **tail = head ? &head->next : &local_refs;
+ struct ref **tail = local_refs ? &local_refs->next : &local_refs;
struct refspec_item tag_refspec;
+ struct ref *to_free = NULL;
refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
if (option_single_branch) {
- struct ref *remote_head = NULL;
-
if (!option_branch)
- remote_head = guess_remote_head(head, refs, 0);
+ refs = to_free = guess_remote_head(head, refs, 0);
else {
free_one_ref(head);
local_refs = head = NULL;
tail = &local_refs;
- remote_head = copy_ref(find_remote_branch(refs, option_branch));
- }
-
- if (!remote_head && option_branch)
- warning(_("Could not find remote branch %s to clone."),
- option_branch);
- else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(remote_head, &refspec->items[i],
- &tail, 0);
-
- /* if --branch=tag, pull the requested tag explicitly */
- get_fetch_map(remote_head, &tag_refspec, &tail, 0);
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
- free_refs(remote_head);
- } else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && option_tags)
+ for (size_t i = 0; i < refspec->nr; i++)
+ get_fetch_map(refs, &refspec->items[i], &tail, 0);
+
+ /*
+ * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
+ * about won't be present in `refs` anyway.
+ * Except with option --mirror, where we grab all refs already.
+ */
+ if (!option_mirror)
get_fetch_map(refs, &tag_refspec, &tail, 0);
+ free_one_ref(to_free);
refspec_item_clear(&tag_refspec);
+
return local_refs;
}
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 4/7] clone: add tags refspec earlier to fetch refspec
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
` (2 preceding siblings ...)
2025-02-05 16:47 ` [PATCH v6 3/7] clone: refactor wanted_peer_refs() Toon Claes
@ 2025-02-05 16:47 ` Toon Claes
2025-02-05 16:47 ` [PATCH v6 5/7] clone: introduce struct clone_opts in builtin/clone.c Toon Claes
` (3 subsequent siblings)
7 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
In clone.c we call refspec_ref_prefixes() to copy the fetch refspecs
from the `remote->fetch` refspec into `ref_prefixes` of
`transport_ls_refs_options`. Afterwards we add the tags prefix
`refs/tags/` prefix as well. At a later point, in wanted_peer_refs() we
process refs using both `remote->fetch` and `TAG_REFSPEC`.
Simplify the code by appending `TAG_REFSPEC` to `remote->fetch` before
calling refspec_ref_prefixes().
To be able to do this, we set `option_tags` to 0 when --mirror is given.
This is because --mirror mirrors (hence the name) all the refs,
including tags and they do not need to be treated separately.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 5efa2bbceb42b230f723660ea963ca1253888235..ef4af1f3e6bf0a33c13ed874587b35aba14fe43c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -435,11 +435,8 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
struct ref **tail = local_refs ? &local_refs->next : &local_refs;
- struct refspec_item tag_refspec;
struct ref *to_free = NULL;
- refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
-
if (option_single_branch) {
if (!option_branch)
refs = to_free = guess_remote_head(head, refs, 0);
@@ -454,16 +451,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
for (size_t i = 0; i < refspec->nr; i++)
get_fetch_map(refs, &refspec->items[i], &tail, 0);
- /*
- * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
- * about won't be present in `refs` anyway.
- * Except with option --mirror, where we grab all refs already.
- */
- if (!option_mirror)
- get_fetch_map(refs, &tag_refspec, &tail, 0);
-
free_one_ref(to_free);
- refspec_item_clear(&tag_refspec);
return local_refs;
}
@@ -1011,8 +999,10 @@ int cmd_clone(int argc,
die(_("unknown ref storage format '%s'"), ref_format);
}
- if (option_mirror)
+ if (option_mirror) {
option_bare = 1;
+ option_tags = 0;
+ }
if (option_bare) {
if (real_git_dir)
@@ -1375,14 +1365,19 @@ int cmd_clone(int argc,
transport->smart_options->check_self_contained_and_connected = 1;
strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
+ if (option_tags || option_branch)
+ /*
+ * Add tags refspec when user asked for tags (implicitly) or
+ * specified --branch, whose argument might be a tag.
+ */
+ refspec_append(&remote->fetch, TAG_REFSPEC);
+
refspec_ref_prefixes(&remote->fetch,
&transport_ls_refs_options.ref_prefixes);
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (option_tags)
- strvec_push(&transport_ls_refs_options.ref_prefixes,
- "refs/tags/");
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 5/7] clone: introduce struct clone_opts in builtin/clone.c
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
` (3 preceding siblings ...)
2025-02-05 16:47 ` [PATCH v6 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
@ 2025-02-05 16:47 ` Toon Claes
2025-02-05 16:47 ` [PATCH v6 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
` (2 subsequent siblings)
7 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
There is a lot of state stored in global variables in builtin/clone.c.
In the long run we'd like to remove many of those.
Introduce `struct clone_opts` in this file. This struct will be used to
contain all details needed to perform the clone. The struct object can
be thrown around to all the functions that need these details.
The first field we're adding is `wants_head`. In some scenarios
(specifically when both `--single-branch` and `--branch` are given) we
are not interested in `HEAD` on the remote. The field `wants_head` in
`struct clone_opts` will hold this information. We could have put
`option_branch` and `option_single_branch` into that struct instead, but
in a following commit we'll be using `wants_head` as well.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 44 +++++++++++++++++++++++++++++---------------
remote.c | 2 +-
remote.h | 5 +++++
3 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index ef4af1f3e6bf0a33c13ed874587b35aba14fe43c..1d421c8f758e37a7219d2da680c7ef8699016171 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -57,6 +57,13 @@
*
*/
+struct clone_opts {
+ int wants_head;
+};
+#define CLONE_OPTS_INIT { \
+ .wants_head = 1 /* default enabled */ \
+}
+
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_tags = 1; /* default enabled */
@@ -429,23 +436,24 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch
return ref;
}
-static struct ref *wanted_peer_refs(const struct ref *refs,
- struct refspec *refspec)
+static struct ref *wanted_peer_refs(struct clone_opts *opts,
+ const struct ref *refs,
+ struct refspec *refspec)
{
- struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
- struct ref *local_refs = head;
- struct ref **tail = local_refs ? &local_refs->next : &local_refs;
+ struct ref *local_refs = NULL;
+ struct ref **tail = &local_refs;
struct ref *to_free = NULL;
- if (option_single_branch) {
- if (!option_branch)
+ if (opts->wants_head) {
+ struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
+ if (head)
+ tail_link_ref(head, &tail);
+ if (option_single_branch)
refs = to_free = guess_remote_head(head, refs, 0);
- else {
- free_one_ref(head);
- local_refs = head = NULL;
- tail = &local_refs;
- refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
- }
+ } else if (option_single_branch) {
+ local_refs = NULL;
+ tail = &local_refs;
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
for (size_t i = 0; i < refspec->nr; i++)
@@ -893,6 +901,8 @@ int cmd_clone(int argc,
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ struct clone_opts opts = CLONE_OPTS_INIT;
+
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ -1343,9 +1353,13 @@ int cmd_clone(int argc,
if (option_not.nr)
transport_set_option(transport, TRANS_OPT_DEEPEN_NOT,
(const char *)&option_not);
- if (option_single_branch)
+ if (option_single_branch) {
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
+ if (option_branch)
+ opts.wants_head = 0;
+ }
+
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
@@ -1454,7 +1468,7 @@ int cmd_clone(int argc,
}
if (refs)
- mapped_refs = wanted_peer_refs(refs, &remote->fetch);
+ mapped_refs = wanted_peer_refs(&opts, refs, &remote->fetch);
if (mapped_refs) {
/*
diff --git a/remote.c b/remote.c
index 1779f0e7bbb8b88965f2fedf407e50ed20ea7a13..69d8c43ea694f1b9a7699e5d5a49bfc169058b8e 100644
--- a/remote.c
+++ b/remote.c
@@ -1260,7 +1260,7 @@ int count_refspec_match(const char *pattern,
}
}
-static void tail_link_ref(struct ref *ref, struct ref ***tail)
+void tail_link_ref(struct ref *ref, struct ref ***tail)
{
**tail = ref;
while (ref->next)
diff --git a/remote.h b/remote.h
index a19353f68999f5440db7bf5f91dd4be8bcc1d8a5..ce3e7c8512981d0ac5db2da508c4fbe64cede961 100644
--- a/remote.h
+++ b/remote.h
@@ -221,6 +221,11 @@ struct ref *alloc_ref(const char *name);
struct ref *copy_ref(const struct ref *ref);
struct ref *copy_ref_list(const struct ref *ref);
int count_refspec_match(const char *, struct ref *refs, struct ref **matched_ref);
+/*
+ * Put a ref in the tail and prepare tail for adding another one.
+ * *tail is the pointer to the tail of the list of refs.
+ */
+void tail_link_ref(struct ref *ref, struct ref ***tail);
int check_ref_type(const struct ref *ref, int flags);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 6/7] parse-options: introduce die_for_incompatible_opt2()
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
` (4 preceding siblings ...)
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 ` Toon Claes
2025-02-05 16:47 ` [PATCH v6 7/7] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
2025-02-06 6:33 ` [PATCH v7 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
7 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The functions die_for_incompatible_opt3() and
die_for_incompatible_opt4() already exist to die whenever a user
specifies three or four options respectively that are not compatible.
Introduce die_for_incompatible_opt2() which dies when two options that
are incompatible are set.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/replay.c | 7 ++++---
parse-options.h | 9 +++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index 1afc6d1ee0cb738fa7fa3f2b5c8ce0dd7802e7da..032c172b65ece4e2c3b275ffdde22a54c55a3933 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -163,9 +163,10 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
get_ref_information(cmd_info, &rinfo);
if (!rinfo.positive_refexprs)
die(_("need some commits to replay"));
- if (onto_name && *advance_name)
- die(_("--onto and --advance are incompatible"));
- else if (onto_name) {
+
+ die_for_incompatible_opt2(!!onto_name, "--onto",
+ !!*advance_name, "--advance");
+ if (onto_name) {
*onto = peel_committish(onto_name);
if (rinfo.positive_refexprs <
strset_get_size(&rinfo.positive_refs))
diff --git a/parse-options.h b/parse-options.h
index 39f088625494f20dea96b9a9cbe986916773bf60..fca944d9a93d643d984c58de2ead9154c8b16c94 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -436,6 +436,15 @@ static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,
0, "");
}
+static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
+ int opt2, const char *opt2_name)
+{
+ die_for_incompatible_opt4(opt1, opt1_name,
+ opt2, opt2_name,
+ 0, "",
+ 0, "");
+}
+
/*
* Use these assertions for callbacks that expect to be called with NONEG and
* NOARG respectively, and do not otherwise handle the "unset" and "arg"
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v6 7/7] builtin/clone: teach git-clone(1) the --revision= option
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
` (5 preceding siblings ...)
2025-02-05 16:47 ` [PATCH v6 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
@ 2025-02-05 16:47 ` 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
7 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-02-05 16:47 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The git-clone(1) command has the option `--branch` that allows the user
to select the branch they want HEAD to point to. In a non-bare
repository this also checks out that branch.
Option `--branch` also accepts a tag. When a tag name is provided, the
commit this tag points to is checked out and HEAD is detached. Thus
`--branch` can be used to clone a repository and check out a ref kept
under `refs/heads` or `refs/tags`. But some other refs might be in use
as well. For example Git forges might use refs like `refs/pull/<id>` and
`refs/merge-requests/<id>` to track pull/merge requests. These refs
cannot be selected upon git-clone(1).
Add option `--revision` to git-clone(1). This option accepts a fully
qualified reference, or a hexadecimal commit ID. This enables the user
to clone and check out any revision they want. `--revision` can be used
in conjunction with `--depth` to do a minimal clone that only contains
the blob and tree for a single revision. This can be useful for
automated tests running in CI systems.
Using option `--branch` and `--single-branch` together is a similar
scenario, but serves a different purpose. Using these two options, a
singlet remote tracking branch is created and the fetch refspec is set
up so git-fetch(1) will receive updates on that branch from the remote.
This allows the user work on that single branch.
Option `--revision` on contrary detaches HEAD, creates no tracking
branches, and writes no fetch refspec.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 10 ++++
builtin/clone.c | 57 ++++++++++++++++----
t/meson.build | 1 +
t/t5621-clone-revision.sh | 123 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 180 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 8d0476f6dcaf6fed7ccd48a20398556dd4e20722..f9cefbf2f643415be11ed0c7f9b460517830bc0f 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -221,6 +221,16 @@ objects from the source repository into a pack in the cloned repository.
`--branch` can also take tags and detaches the `HEAD` at that commit
in the resulting repository.
+`--revision=<rev>`::
+ Create a new repository, and fetch the history leading to the given
+ revision _<rev>_ (and nothing else), without making any remote-tracking
+ branch, and without making any local branch, and point `HEAD` to
+ _<rev>_. When creating a non-bare repository, the revision is checked
+ out on a detached `HEAD`. The argument can be a ref name
+ (e.g. `refs/heads/main` or `refs/tags/v1.0`) that peels down to a
+ commit, or a hexadecimal object name.
+ This option is incompatible with `--branch` and `--mirror`.
+
`-u` _<upload-pack>_::
`--upload-pack` _<upload-pack>_::
When given, and the repository to clone from is accessed
diff --git a/builtin/clone.c b/builtin/clone.c
index 1d421c8f758e37a7219d2da680c7ef8699016171..6ad041e288bc7a926413124442d4f285eea7ad95 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,6 +59,7 @@
struct clone_opts {
int wants_head;
+ int detach;
};
#define CLONE_OPTS_INIT { \
.wants_head = 1 /* default enabled */ \
@@ -565,11 +566,11 @@ static void update_remote_refs(const struct ref *refs,
}
}
-static void update_head(const struct ref *our, const struct ref *remote,
+static void update_head(struct clone_opts *opts, const struct ref *our, const struct ref *remote,
const char *unborn, const char *msg)
{
const char *head;
- if (our && skip_prefix(our->name, "refs/heads/", &head)) {
+ if (our && !opts->detach && skip_prefix(our->name, "refs/heads/", &head)) {
/* Local default branch link */
if (refs_update_symref(get_main_ref_store(the_repository), "HEAD", our->name, NULL) < 0)
die(_("unable to update HEAD"));
@@ -580,8 +581,9 @@ static void update_head(const struct ref *our, const struct ref *remote,
install_branch_config(0, head, remote_name, our->name);
}
} else if (our) {
- struct commit *c = lookup_commit_reference(the_repository,
- &our->old_oid);
+ struct commit *c = lookup_commit_or_die(&our->old_oid,
+ our->name);
+
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
refs_update_ref(get_main_ref_store(the_repository), msg,
"HEAD", &c->object.oid, NULL, REF_NO_DEREF,
@@ -900,6 +902,7 @@ int cmd_clone(int argc,
int option_filter_submodules = -1; /* unspecified */
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ char *option_rev = NULL;
struct clone_opts opts = CLONE_OPTS_INIT;
@@ -943,6 +946,8 @@ int cmd_clone(int argc,
N_("use <name> instead of 'origin' to track upstream")),
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")),
OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
N_("path to git-upload-pack on the remote")),
OPT_STRING(0, "depth", &option_depth, N_("depth"),
@@ -1279,7 +1284,7 @@ int cmd_clone(int argc,
strbuf_addstr(&branch_top, src_ref_prefix);
git_config_set("core.bare", "true");
- } else {
+ } else if (!option_rev) {
strbuf_addf(&branch_top, "refs/remotes/%s/", remote_name);
}
@@ -1298,8 +1303,9 @@ int cmd_clone(int argc,
remote = remote_get_early(remote_name);
- 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);
path = get_repo_path(remote->url.v[0], &is_bundle);
is_local = option_local != 0 && path && !is_bundle;
@@ -1342,6 +1348,11 @@ int cmd_clone(int argc,
transport_set_option(transport, TRANS_OPT_KEEP, "yes");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ !!option_branch, "--branch");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ option_mirror, "--mirror");
+
if (reject_shallow)
transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
if (option_depth)
@@ -1378,7 +1389,14 @@ 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_single_branch = 0;
+ opts.wants_head = 0;
+ opts.detach = 1;
+
+ refspec_append(&remote->fetch, option_rev);
+ }
if (option_tags || option_branch)
/*
@@ -1393,6 +1411,17 @@ int cmd_clone(int argc,
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
+ /*
+ * As part of transport_get_remote_refs() the server tells us the hash
+ * algorithm, which we require to initialize the repo. But calling that
+ * function without any ref prefix, will cause the server to announce
+ * all known refs. If the argument passed to --revision was a hex oid,
+ * ref_prefixes will be empty so we fall back to asking about HEAD to
+ * reduce traffic from the server.
+ */
+ if (opts.wants_head || transport_ls_refs_options.ref_prefixes.nr == 0)
+ strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
/*
@@ -1501,6 +1530,11 @@ int cmd_clone(int argc,
if (!our_head_points_at)
die(_("Remote branch %s not found in upstream %s"),
option_branch, remote_name);
+ } else if (option_rev) {
+ our_head_points_at = mapped_refs;
+ if (!our_head_points_at)
+ die(_("Remote revision %s not found in upstream %s"),
+ option_rev, remote_name);
} else if (remote_head_points_at) {
our_head_points_at = remote_head_points_at;
} else if (remote_head) {
@@ -1539,8 +1573,9 @@ int cmd_clone(int argc,
free(to_free);
}
- write_refspec_config(src_ref_prefix, our_head_points_at,
- remote_head_points_at, &branch_top);
+ if (!option_rev)
+ write_refspec_config(src_ref_prefix, our_head_points_at,
+ remote_head_points_at, &branch_top);
if (filter_options.choice)
partial_clone_register(remote_name, &filter_options);
@@ -1556,7 +1591,7 @@ int cmd_clone(int argc,
branch_top.buf, reflog_msg.buf, transport,
!is_local);
- update_head(our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
+ update_head(&opts, our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
/*
* We want to show progress for recursive submodule clones iff
diff --git a/t/meson.build b/t/meson.build
index 35f25ca4a1d960564190288e9456620a46ccc80a..b5f917926b61de379b6cef45e5f750912422a7d1 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -721,6 +721,7 @@ integration_tests = [
't5617-clone-submodules-remote.sh',
't5618-alternate-refs.sh',
't5619-clone-local-ambiguous-transport.sh',
+ 't5621-clone-revision.sh',
't5700-protocol-v1.sh',
't5701-git-serve.sh',
't5702-protocol-v2.sh',
diff --git a/t/t5621-clone-revision.sh b/t/t5621-clone-revision.sh
new file mode 100755
index 0000000000000000000000000000000000000000..d4889a954e6300e0e327ebe7dfcf73569d966829
--- /dev/null
+++ b/t/t5621-clone-revision.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+
+test_description='tests for git clone --revision'
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit --no-tag "initial commit" README "Hello" &&
+ test_commit --annotate "second commit" README "Hello world" v1.0 &&
+ test_commit --no-tag "third commit" README "Hello world!" &&
+ git switch -c feature v1.0 &&
+ test_commit --no-tag "feature commit" README "Hello world!" &&
+ git switch main
+'
+
+test_expect_success 'clone with --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --depth and --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --no-local --depth=1 --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch &&
+ git -C dst rev-list HEAD >actual &&
+ test_line_count = 1 actual
+'
+
+test_expect_success 'clone with --revision being a tag' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/tags/v1.0 . dst &&
+ git rev-parse refs/tags/v1.0^{} >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being HEAD' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=HEAD . dst &&
+ git rev-parse HEAD >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature) &&
+ git clone --revision=$oid . dst &&
+ echo $oid >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision and --bare' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/main --bare . dst &&
+ oid=$(git rev-parse refs/heads/main) &&
+ git -C dst cat-file -t $oid >actual &&
+ echo "commit" >expect &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a short raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse --short refs/heads/feature) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "fatal: Remote revision $oid not found in upstream origin" err
+'
+
+test_expect_success 'clone with --revision being a tree hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature^{tree}) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "error: object $oid is a tree, not a commit" err
+'
+
+test_expect_success 'clone with --revision being the parent of a ref fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main^ . dst
+'
+
+test_expect_success 'clone with --revision and --branch fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --branch=main . dst
+'
+
+test_expect_success 'clone with --revision and --mirror fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --mirror . dst
+'
+
+test_done
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v6 7/7] builtin/clone: teach git-clone(1) the --revision= option
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
0 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2025-02-05 17:24 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek,
Patrick Steinhardt, Jeff King
Toon Claes <toon@iotcl.com> writes:
> The git-clone(1) command has the option `--branch` that allows the user
> to select the branch they want HEAD to point to. In a non-bare
> repository this also checks out that branch.
>
> Option `--branch` also accepts a tag. When a tag name is provided, the
> commit this tag points to is checked out and HEAD is detached. Thus
> `--branch` can be used to clone a repository and check out a ref kept
> under `refs/heads` or `refs/tags`. But some other refs might be in use
> as well. For example Git forges might use refs like `refs/pull/<id>` and
> `refs/merge-requests/<id>` to track pull/merge requests. These refs
> cannot be selected upon git-clone(1).
These examples made the motivation a lot easier to see. Very nice
compared to a much earlier edition.
> +`--revision=<rev>`::
> + Create a new repository, and fetch the history leading to the given
> + revision _<rev>_ (and nothing else), without making any remote-tracking
> + branch, and without making any local branch, and point `HEAD` to
> + _<rev>_. When creating a non-bare repository, the revision is checked
> + out on a detached `HEAD`. The argument can be a ref name
Micronit. I think the resulting repository, even when it is bare,
would have its HEAD detached (i.e., instead of being a symbolic ref
to a local branch, points directly at a commit object).
... branch, and detach the `HEAD` to _<rev>_. When creating
a non-bare repository, the revision is checked out. The
argument can be a ref name ...
But then "When ... checked out" probably goes without saying. There
is nothing special wrt to bare/non-bare this option affects the
behaviour of the command.
> @@ -580,8 +581,9 @@ static void update_head(const struct ref *our, const struct ref *remote,
> install_branch_config(0, head, remote_name, our->name);
> }
> } else if (our) {
> - struct commit *c = lookup_commit_reference(the_repository,
> - &our->old_oid);
> + struct commit *c = lookup_commit_or_die(&our->old_oid,
> + our->name);
> +
.git/rebase-apply/patch:62: indent with spaces.
our->name);
warning: 1 line applied after fixing whitespace errors.
Other than that, looking good.
Thanks.
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v7 0/7] Enable doing a shallow clone of a specific git revision
2025-02-05 16:47 ` [PATCH v6 " Toon Claes
` (6 preceding siblings ...)
2025-02-05 16:47 ` [PATCH v6 7/7] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
@ 2025-02-06 6:33 ` Toon Claes
2025-02-06 6:33 ` [PATCH v7 1/7] clone: cut down on global variables in clone.c Toon Claes
` (6 more replies)
7 siblings, 7 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The goal of this series is to add an option `--revision` to
git-clone(1).
This series starts with a handful of preparatory refactoring commits
that make it more straight-forward to add this new option. In the last
commit we're actually adding the feature.
This series sets an example on how I think we can further refactor
builtin/clone.c to increase the maintainability of the code.
---
Changes in v7:
- Further enhance documentation of option --revision on git-clone(1).
- Indentation fix in builtin/clone.c.
- Link to v6: https://lore.kernel.org/r/20250205-toon-clone-refs-v6-0-0bbc8e6d89fd@iotcl.com
Changes in v6:
- Rewrite the documentation for git-clone(1) --[no-]tags.
- Remove unneeded conditional around die_for_incompatible_opt2() in
builtin/replay.c.
- Fix typo in code comment in builtin/clone.c.
- Link to v5: https://lore.kernel.org/r/20250204-toon-clone-refs-v5-0-37e34af283c8@iotcl.com
Changes in v5:
- Add separate commit to introduce die_for_incompatible_opt2()
- Small tweaks in documentation about `--[no-]tags` and `--revision`.
- Better explain the refactoring of wanted_peer_refs() in the commit
message.
- Change type from `int` to `size_t` in wanted_peer_refs().
- Use lookup_commit_or_die() instead lookup_commit_reference() to avoid
checking the result ourself.
- Add a few code comments to explain some things.
- Stylish cleanups like removal of unneeded empty lines, commented out
test-code and remarks.
- Link to v4: https://lore.kernel.org/r/20250131-toon-clone-refs-v4-0-2a4ff851498f@iotcl.com
Changes in v4:
- Introduce a new commit to reduce the use of global variables.
- Introduce a new commit to invert the flag --no-tags to --tags.
- Introduce a new commit to refactor wanted_peer_refs() in
builtin/clone.c.
- Introduce a new commit to shuffle the handling of tags refspec.
- Introduce a new commit to introduce a `struct clone_opts`.
- Link to v3: https://lore.kernel.org/r/20241219-toon-clone-refs-v3-1-1484faea3008@iotcl.com
Changes in v3:
- Fail early when the revision was not found on the remote, instead of
creating a clone that's in an invalid state.
- State more clearly in the commit message adding this option is useful
for a not uncommon use-case.
- Be explicit in the documentation the ref needs to peel down to a
commit.
- Die in case we try to update_head() to an object that's not a commit.
- Allow combining `--revision` with `--bare`.
- Add die_for_incompatible_opt2() to parse-options.h and use it for the
options that are not compatible with the new `--revision` option.
- Small tweaks to the added tests.
- Small touchups on commit messages.
- Link to v2: https://lore.kernel.org/r/20241129-toon-clone-refs-v2-1-dca4c19a3510@iotcl.com
---
Toon Claes (7):
clone: cut down on global variables in clone.c
clone: make it possible to specify --tags
clone: refactor wanted_peer_refs()
clone: add tags refspec earlier to fetch refspec
clone: introduce struct clone_opts in builtin/clone.c
parse-options: introduce die_for_incompatible_opt2()
builtin/clone: teach git-clone(1) the --revision= option
Documentation/git-clone.txt | 26 +++-
builtin/clone.c | 350 +++++++++++++++++++++++++-------------------
builtin/replay.c | 7 +-
parse-options.h | 9 ++
remote.c | 2 +-
remote.h | 5 +
t/meson.build | 1 +
t/t5621-clone-revision.sh | 123 ++++++++++++++++
8 files changed, 358 insertions(+), 165 deletions(-)
---
Range-diff versus v6:
1: 14e4210ef9 = 1: e9d0d1bb4e clone: cut down on global variables in clone.c
2: f85beed07e = 2: 5c570e08f6 clone: make it possible to specify --tags
3: 92b998a173 = 3: bb5d206ee6 clone: refactor wanted_peer_refs()
4: 3fb2766728 = 4: 344a2f143c clone: add tags refspec earlier to fetch refspec
5: d0341cad24 = 5: 93d074d17e clone: introduce struct clone_opts in builtin/clone.c
6: d42f291e48 = 6: 457f21943e parse-options: introduce die_for_incompatible_opt2()
7: 3f332971dc ! 7: fb4f05547e builtin/clone: teach git-clone(1) the --revision= option
@@ Documentation/git-clone.txt: objects from the source repository into a pack in t
+`--revision=<rev>`::
+ Create a new repository, and fetch the history leading to the given
+ revision _<rev>_ (and nothing else), without making any remote-tracking
-+ branch, and without making any local branch, and point `HEAD` to
-+ _<rev>_. When creating a non-bare repository, the revision is checked
-+ out on a detached `HEAD`. The argument can be a ref name
-+ (e.g. `refs/heads/main` or `refs/tags/v1.0`) that peels down to a
-+ commit, or a hexadecimal object name.
++ branch, and without making any local branch, and detach `HEAD` to
++ _<rev>_. The argument can be a ref name (e.g. `refs/heads/main` or
++ `refs/tags/v1.0`) that peels down to a commit, or a hexadecimal object
++ name.
+ This option is incompatible with `--branch` and `--mirror`.
+
`-u` _<upload-pack>_::
@@ builtin/clone.c: static void update_head(const struct ref *our, const struct ref
- struct commit *c = lookup_commit_reference(the_repository,
- &our->old_oid);
+ struct commit *c = lookup_commit_or_die(&our->old_oid,
-+ our->name);
++ our->name);
+
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
refs_update_ref(get_main_ref_store(the_repository), msg,
---
base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
change-id: 20241129-toon-clone-refs-ad3623772f92
Thanks
--
Toon
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v7 1/7] clone: cut down on global variables in clone.c
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 ` Toon Claes
2025-02-06 6:33 ` [PATCH v7 2/7] clone: make it possible to specify --tags Toon Claes
` (5 subsequent siblings)
6 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
In clone.c the `struct option` which is used to parse the input options
for git-clone(1) is a global variable. Due to this, many variables that
are used to parse the value into, are also global.
Make `builtin_clone_options` a local variable in cmd_clone() and carry
along all variables that are only used in that function.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 195 +++++++++++++++++++++++++++++---------------------------
1 file changed, 101 insertions(+), 94 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index fd001d800c635e46bbc7027a8fdda2a8c9fbf069..5ed0802f1d0ddebaf512aac93bf8c8b340494323 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -56,42 +56,22 @@
* - dropping use-separate-remote and no-separate-remote compatibility
*
*/
-static const char * const builtin_clone_usage[] = {
- N_("git clone [<options>] [--] <repo> [<dir>]"),
- NULL
-};
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_no_tags;
static int option_shallow_submodules;
-static int option_reject_shallow = -1; /* unspecified */
static int config_reject_shallow = -1; /* unspecified */
-static int deepen;
-static char *option_template, *option_depth, *option_since;
-static char *option_origin = NULL;
static char *remote_name = NULL;
static char *option_branch = NULL;
-static struct string_list option_not = STRING_LIST_INIT_NODUP;
-static const char *real_git_dir;
-static const char *ref_format;
-static const char *option_upload_pack = "git-upload-pack";
static int option_verbosity;
-static int option_progress = -1;
-static int option_sparse_checkout;
-static enum transport_family family;
-static struct string_list option_config = STRING_LIST_INIT_NODUP;
static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
-static int option_dissociate;
static int max_jobs = -1;
static struct string_list option_recurse_submodules = STRING_LIST_INIT_NODUP;
static struct list_objects_filter_options filter_options = LIST_OBJECTS_FILTER_INIT;
-static int option_filter_submodules = -1; /* unspecified */
static int config_filter_submodules = -1; /* unspecified */
-static struct string_list server_options = STRING_LIST_INIT_NODUP;
static int option_remote_submodules;
-static const char *bundle_uri;
static int recurse_submodules_cb(const struct option *opt,
const char *arg, int unset)
@@ -107,78 +87,6 @@ static int recurse_submodules_cb(const struct option *opt,
return 0;
}
-static struct option builtin_clone_options[] = {
- OPT__VERBOSITY(&option_verbosity),
- OPT_BOOL(0, "progress", &option_progress,
- N_("force progress reporting")),
- OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
- N_("don't clone shallow repository")),
- OPT_BOOL('n', "no-checkout", &option_no_checkout,
- N_("don't create a checkout")),
- OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
- OPT_HIDDEN_BOOL(0, "naked", &option_bare,
- N_("create a bare repository")),
- OPT_BOOL(0, "mirror", &option_mirror,
- N_("create a mirror repository (implies --bare)")),
- OPT_BOOL('l', "local", &option_local,
- N_("to clone from a local repository")),
- OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
- N_("don't use local hardlinks, always copy")),
- OPT_BOOL('s', "shared", &option_shared,
- N_("setup as shared repository")),
- { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
- N_("pathspec"), N_("initialize submodules in the clone"),
- PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
- OPT_ALIAS(0, "recursive", "recurse-submodules"),
- OPT_INTEGER('j', "jobs", &max_jobs,
- N_("number of submodules cloned in parallel")),
- OPT_STRING(0, "template", &option_template, N_("template-directory"),
- N_("directory from which templates will be used")),
- OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
- N_("reference repository")),
- OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
- N_("repo"), N_("reference repository")),
- OPT_BOOL(0, "dissociate", &option_dissociate,
- N_("use --reference only while cloning")),
- OPT_STRING('o', "origin", &option_origin, N_("name"),
- N_("use <name> instead of 'origin' to track upstream")),
- OPT_STRING('b', "branch", &option_branch, N_("branch"),
- N_("checkout <branch> instead of the remote's HEAD")),
- OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
- N_("path to git-upload-pack on the remote")),
- OPT_STRING(0, "depth", &option_depth, N_("depth"),
- N_("create a shallow clone of that depth")),
- OPT_STRING(0, "shallow-since", &option_since, N_("time"),
- N_("create a shallow clone since a specific time")),
- OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
- N_("deepen history of shallow clone, excluding ref")),
- OPT_BOOL(0, "single-branch", &option_single_branch,
- N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
- OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
- N_("any cloned submodules will be shallow")),
- OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
- N_("separate git dir from working tree")),
- OPT_STRING(0, "ref-format", &ref_format, N_("format"),
- N_("specify the reference format to use")),
- OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
- N_("set config inside the new repository")),
- OPT_STRING_LIST(0, "server-option", &server_options,
- N_("server-specific"), N_("option to transmit")),
- OPT_IPVERSION(&family),
- OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
- OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
- N_("apply partial clone filters to submodules")),
- OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
- N_("any cloned submodules will use their remote-tracking branch")),
- OPT_BOOL(0, "sparse", &option_sparse_checkout,
- N_("initialize sparse-checkout file to include only files at root")),
- OPT_STRING(0, "bundle-uri", &bundle_uri,
- N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
- OPT_END()
-};
-
static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
{
static const char *suffix[] = { "/.git", "", ".git/.git", ".git" };
@@ -989,10 +897,103 @@ int cmd_clone(int argc,
int hash_algo;
enum ref_storage_format ref_storage_format = REF_STORAGE_FORMAT_UNKNOWN;
const int do_not_override_repo_unix_permissions = -1;
+ int option_reject_shallow = -1; /* unspecified */
+ int deepen = 0;
+ char *option_template = NULL, *option_depth = NULL, *option_since = NULL;
+ char *option_origin = NULL;
+ struct string_list option_not = STRING_LIST_INIT_NODUP;
+ const char *real_git_dir = NULL;
+ const char *ref_format = NULL;
+ const char *option_upload_pack = "git-upload-pack";
+ int option_progress = -1;
+ int option_sparse_checkout = 0;
+ enum transport_family family = TRANSPORT_FAMILY_ALL;
+ struct string_list option_config = STRING_LIST_INIT_DUP;
+ int option_dissociate = 0;
+ int option_filter_submodules = -1; /* unspecified */
+ struct string_list server_options = STRING_LIST_INIT_NODUP;
+ const char *bundle_uri = NULL;
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
+ struct option builtin_clone_options[] = {
+ OPT__VERBOSITY(&option_verbosity),
+ OPT_BOOL(0, "progress", &option_progress,
+ N_("force progress reporting")),
+ OPT_BOOL(0, "reject-shallow", &option_reject_shallow,
+ N_("don't clone shallow repository")),
+ OPT_BOOL('n', "no-checkout", &option_no_checkout,
+ N_("don't create a checkout")),
+ OPT_BOOL(0, "bare", &option_bare, N_("create a bare repository")),
+ OPT_HIDDEN_BOOL(0, "naked", &option_bare,
+ N_("create a bare repository")),
+ OPT_BOOL(0, "mirror", &option_mirror,
+ N_("create a mirror repository (implies --bare)")),
+ OPT_BOOL('l', "local", &option_local,
+ N_("to clone from a local repository")),
+ OPT_BOOL(0, "no-hardlinks", &option_no_hardlinks,
+ N_("don't use local hardlinks, always copy")),
+ OPT_BOOL('s', "shared", &option_shared,
+ N_("setup as shared repository")),
+ { OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
+ N_("pathspec"), N_("initialize submodules in the clone"),
+ PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
+ OPT_ALIAS(0, "recursive", "recurse-submodules"),
+ OPT_INTEGER('j', "jobs", &max_jobs,
+ N_("number of submodules cloned in parallel")),
+ OPT_STRING(0, "template", &option_template, N_("template-directory"),
+ N_("directory from which templates will be used")),
+ OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
+ N_("reference repository")),
+ OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
+ N_("repo"), N_("reference repository")),
+ OPT_BOOL(0, "dissociate", &option_dissociate,
+ N_("use --reference only while cloning")),
+ OPT_STRING('o', "origin", &option_origin, N_("name"),
+ N_("use <name> instead of 'origin' to track upstream")),
+ OPT_STRING('b', "branch", &option_branch, N_("branch"),
+ N_("checkout <branch> instead of the remote's HEAD")),
+ OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
+ N_("path to git-upload-pack on the remote")),
+ OPT_STRING(0, "depth", &option_depth, N_("depth"),
+ N_("create a shallow clone of that depth")),
+ OPT_STRING(0, "shallow-since", &option_since, N_("time"),
+ N_("create a shallow clone since a specific time")),
+ OPT_STRING_LIST(0, "shallow-exclude", &option_not, N_("ref"),
+ N_("deepen history of shallow clone, excluding ref")),
+ OPT_BOOL(0, "single-branch", &option_single_branch,
+ N_("clone only one branch, HEAD or --branch")),
+ OPT_BOOL(0, "no-tags", &option_no_tags,
+ N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
+ N_("any cloned submodules will be shallow")),
+ OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
+ N_("separate git dir from working tree")),
+ OPT_STRING(0, "ref-format", &ref_format, N_("format"),
+ N_("specify the reference format to use")),
+ OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
+ N_("set config inside the new repository")),
+ OPT_STRING_LIST(0, "server-option", &server_options,
+ N_("server-specific"), N_("option to transmit")),
+ OPT_IPVERSION(&family),
+ OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
+ OPT_BOOL(0, "also-filter-submodules", &option_filter_submodules,
+ N_("apply partial clone filters to submodules")),
+ OPT_BOOL(0, "remote-submodules", &option_remote_submodules,
+ N_("any cloned submodules will use their remote-tracking branch")),
+ OPT_BOOL(0, "sparse", &option_sparse_checkout,
+ N_("initialize sparse-checkout file to include only files at root")),
+ OPT_STRING(0, "bundle-uri", &bundle_uri,
+ N_("uri"), N_("a URI for downloading bundles before fetching from origin remote")),
+ OPT_END()
+ };
+
+ const char * const builtin_clone_usage[] = {
+ N_("git clone [<options>] [--] <repo> [<dir>]"),
+ NULL
+ };
+
packet_trace_identity("clone");
git_config(git_clone_config, NULL);
@@ -1138,8 +1139,8 @@ int cmd_clone(int argc,
for_each_string_list_item(item, &option_recurse_submodules) {
strbuf_addf(&sb, "submodule.active=%s",
item->string);
- string_list_append(&option_config,
- strbuf_detach(&sb, NULL));
+ string_list_append(&option_config, sb.buf);
+ strbuf_reset(&sb);
}
if (!git_config_get_bool("submodule.stickyRecursiveClone", &val) &&
@@ -1161,6 +1162,8 @@ int cmd_clone(int argc,
string_list_append(&option_config,
"submodule.alternateErrorStrategy=info");
}
+
+ strbuf_release(&sb);
}
/*
@@ -1578,6 +1581,10 @@ int cmd_clone(int argc,
err = checkout(submodule_progress, filter_submodules,
ref_storage_format);
+ string_list_clear(&option_not, 0);
+ string_list_clear(&option_config, 0);
+ string_list_clear(&server_options, 0);
+
free(remote_name);
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v7 2/7] clone: make it possible to specify --tags
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 ` Toon Claes
2025-02-06 6:33 ` [PATCH v7 3/7] clone: refactor wanted_peer_refs() Toon Claes
` (4 subsequent siblings)
6 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
Option --no-tags was added in 0dab2468ee (clone: add a --no-tags option
to clone without tags, 2017-04-26). At the time there was no need to
support --tags as well, although there was some conversation about
it[1].
To simplify the code and to prepare for future commits, invert the flag
internally. Functionally there is no change, because the flag is
default-enabled passing `--tags` has no effect, so there's no need to
add tests for this.
[1]: https://lore.kernel.org/git/CAGZ79kbHuMpiavJ90kQLEL_AR0BEyArcZoEWAjPPhOFacN16YQ@mail.gmail.com/
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 17 ++++++++++-------
builtin/clone.c | 14 +++++++-------
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index de8d8f58930ecff305f79480b13ddce10cd96c60..8d0476f6dcaf6fed7ccd48a20398556dd4e20722 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -13,7 +13,7 @@ git clone [--template=<template-directory>]
[-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
[-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
[--dissociate] [--separate-git-dir <git-dir>]
- [--depth <depth>] [--[no-]single-branch] [--no-tags]
+ [--depth <depth>] [--[no-]single-branch] [--[no-]tags]
[--recurse-submodules[=<pathspec>]] [--[no-]shallow-submodules]
[--[no-]remote-submodules] [--jobs <n>] [--sparse] [--[no-]reject-shallow]
[--filter=<filter-spec>] [--also-filter-submodules]] [--] <repository>
@@ -273,12 +273,15 @@ corresponding `--mirror` and `--no-tags` options instead.
branch when `--single-branch` clone was made, no remote-tracking
branch is created.
-`--no-tags`::
- Don't clone any tags, and set
- `remote.<remote>.tagOpt=--no-tags` in the config, ensuring
- that future `git pull` and `git fetch` operations won't follow
- any tags. Subsequent explicit tag fetches will still work,
- (see linkgit:git-fetch[1]).
+`--[no-]tags`::
+ Control whether or not tags will be cloned. When `--no-tags` is
+ given, the option will be become permanent by setting the
+ `remote.<remote>.tagOpt=--no-tags` configuration. This ensures that
+ future `git pull` and `git fetch` won't follow any tags. Subsequent
+ explicit tag fetches will still work (see linkgit:git-fetch[1]).
+
+ By default, tags are cloned and passing `--tags` is thus typically a
+ no-op, unless it cancels out a previous `--no-tags`.
+
Can be used in conjunction with `--single-branch` to clone and
maintain a branch with no references other than a single cloned
diff --git a/builtin/clone.c b/builtin/clone.c
index 5ed0802f1d0ddebaf512aac93bf8c8b340494323..69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,7 +59,7 @@
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
-static int option_no_tags;
+static int option_tags = 1; /* default enabled */
static int option_shallow_submodules;
static int config_reject_shallow = -1; /* unspecified */
static char *remote_name = NULL;
@@ -470,7 +470,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && !option_no_tags)
+ if (!option_mirror && !option_single_branch && option_tags)
get_fetch_map(refs, &tag_refspec, &tail, 0);
refspec_item_clear(&tag_refspec);
@@ -562,7 +562,7 @@ static void update_remote_refs(const struct ref *refs,
if (refs) {
write_remote_refs(mapped_refs);
- if (option_single_branch && !option_no_tags)
+ if (option_single_branch && option_tags)
write_followtags(refs, msg);
}
@@ -964,8 +964,8 @@ int cmd_clone(int argc,
N_("deepen history of shallow clone, excluding ref")),
OPT_BOOL(0, "single-branch", &option_single_branch,
N_("clone only one branch, HEAD or --branch")),
- OPT_BOOL(0, "no-tags", &option_no_tags,
- N_("don't clone any tags, and make later fetches not to follow them")),
+ OPT_BOOL(0, "tags", &option_tags,
+ N_("clone tags, and make later fetches not to follow them")),
OPT_BOOL(0, "shallow-submodules", &option_shallow_submodules,
N_("any cloned submodules will be shallow")),
OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
@@ -1296,7 +1296,7 @@ int cmd_clone(int argc,
git_config_set(key.buf, repo);
strbuf_reset(&key);
- if (option_no_tags) {
+ if (!option_tags) {
strbuf_addf(&key, "remote.%s.tagOpt", remote_name);
git_config_set(key.buf, "--no-tags");
strbuf_reset(&key);
@@ -1389,7 +1389,7 @@ int cmd_clone(int argc,
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (!option_no_tags)
+ if (option_tags)
strvec_push(&transport_ls_refs_options.ref_prefixes,
"refs/tags/");
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v7 3/7] clone: refactor wanted_peer_refs()
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 ` Toon Claes
2025-02-06 6:33 ` [PATCH v7 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
` (3 subsequent siblings)
6 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The function wanted_peer_refs() is used to map the refs returned by the
server to refs we will save in our clone.
Over time this function grown to be very complex. Refactor it.
Previously, there was a separate code path for when
`option_single_branch` was set. It resulted in duplicated code and
deeper nested conditions. After this refactor the code path for when
`option_single_branch` is truthy modifies `refs` and then falls through
to the common code path. This approach relies on the `refspec` being set
correctly and thus only mapping refs that are relevant.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 69d1ad029dfa84a2f7136fa4a3c4c8a594b179c4..5efa2bbceb42b230f723660ea963ca1253888235 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -434,46 +434,37 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
{
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
- struct ref **tail = head ? &head->next : &local_refs;
+ struct ref **tail = local_refs ? &local_refs->next : &local_refs;
struct refspec_item tag_refspec;
+ struct ref *to_free = NULL;
refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
if (option_single_branch) {
- struct ref *remote_head = NULL;
-
if (!option_branch)
- remote_head = guess_remote_head(head, refs, 0);
+ refs = to_free = guess_remote_head(head, refs, 0);
else {
free_one_ref(head);
local_refs = head = NULL;
tail = &local_refs;
- remote_head = copy_ref(find_remote_branch(refs, option_branch));
- }
-
- if (!remote_head && option_branch)
- warning(_("Could not find remote branch %s to clone."),
- option_branch);
- else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(remote_head, &refspec->items[i],
- &tail, 0);
-
- /* if --branch=tag, pull the requested tag explicitly */
- get_fetch_map(remote_head, &tag_refspec, &tail, 0);
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
- free_refs(remote_head);
- } else {
- int i;
- for (i = 0; i < refspec->nr; i++)
- get_fetch_map(refs, &refspec->items[i], &tail, 0);
}
- if (!option_mirror && !option_single_branch && option_tags)
+ for (size_t i = 0; i < refspec->nr; i++)
+ get_fetch_map(refs, &refspec->items[i], &tail, 0);
+
+ /*
+ * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
+ * about won't be present in `refs` anyway.
+ * Except with option --mirror, where we grab all refs already.
+ */
+ if (!option_mirror)
get_fetch_map(refs, &tag_refspec, &tail, 0);
+ free_one_ref(to_free);
refspec_item_clear(&tag_refspec);
+
return local_refs;
}
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v7 4/7] clone: add tags refspec earlier to fetch refspec
2025-02-06 6:33 ` [PATCH v7 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (2 preceding siblings ...)
2025-02-06 6:33 ` [PATCH v7 3/7] clone: refactor wanted_peer_refs() Toon Claes
@ 2025-02-06 6:33 ` Toon Claes
2025-02-06 6:33 ` [PATCH v7 5/7] clone: introduce struct clone_opts in builtin/clone.c Toon Claes
` (2 subsequent siblings)
6 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
In clone.c we call refspec_ref_prefixes() to copy the fetch refspecs
from the `remote->fetch` refspec into `ref_prefixes` of
`transport_ls_refs_options`. Afterwards we add the tags prefix
`refs/tags/` prefix as well. At a later point, in wanted_peer_refs() we
process refs using both `remote->fetch` and `TAG_REFSPEC`.
Simplify the code by appending `TAG_REFSPEC` to `remote->fetch` before
calling refspec_ref_prefixes().
To be able to do this, we set `option_tags` to 0 when --mirror is given.
This is because --mirror mirrors (hence the name) all the refs,
including tags and they do not need to be treated separately.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 5efa2bbceb42b230f723660ea963ca1253888235..ef4af1f3e6bf0a33c13ed874587b35aba14fe43c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -435,11 +435,8 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
struct ref **tail = local_refs ? &local_refs->next : &local_refs;
- struct refspec_item tag_refspec;
struct ref *to_free = NULL;
- refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
-
if (option_single_branch) {
if (!option_branch)
refs = to_free = guess_remote_head(head, refs, 0);
@@ -454,16 +451,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
for (size_t i = 0; i < refspec->nr; i++)
get_fetch_map(refs, &refspec->items[i], &tail, 0);
- /*
- * Grab all refs that match the TAG_REFSPEC. Any tags we don't care
- * about won't be present in `refs` anyway.
- * Except with option --mirror, where we grab all refs already.
- */
- if (!option_mirror)
- get_fetch_map(refs, &tag_refspec, &tail, 0);
-
free_one_ref(to_free);
- refspec_item_clear(&tag_refspec);
return local_refs;
}
@@ -1011,8 +999,10 @@ int cmd_clone(int argc,
die(_("unknown ref storage format '%s'"), ref_format);
}
- if (option_mirror)
+ if (option_mirror) {
option_bare = 1;
+ option_tags = 0;
+ }
if (option_bare) {
if (real_git_dir)
@@ -1375,14 +1365,19 @@ int cmd_clone(int argc,
transport->smart_options->check_self_contained_and_connected = 1;
strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
+ if (option_tags || option_branch)
+ /*
+ * Add tags refspec when user asked for tags (implicitly) or
+ * specified --branch, whose argument might be a tag.
+ */
+ refspec_append(&remote->fetch, TAG_REFSPEC);
+
refspec_ref_prefixes(&remote->fetch,
&transport_ls_refs_options.ref_prefixes);
if (option_branch)
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
- if (option_tags)
- strvec_push(&transport_ls_refs_options.ref_prefixes,
- "refs/tags/");
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v7 5/7] clone: introduce struct clone_opts in builtin/clone.c
2025-02-06 6:33 ` [PATCH v7 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (3 preceding siblings ...)
2025-02-06 6:33 ` [PATCH v7 4/7] clone: add tags refspec earlier to fetch refspec Toon Claes
@ 2025-02-06 6:33 ` 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
6 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
There is a lot of state stored in global variables in builtin/clone.c.
In the long run we'd like to remove many of those.
Introduce `struct clone_opts` in this file. This struct will be used to
contain all details needed to perform the clone. The struct object can
be thrown around to all the functions that need these details.
The first field we're adding is `wants_head`. In some scenarios
(specifically when both `--single-branch` and `--branch` are given) we
are not interested in `HEAD` on the remote. The field `wants_head` in
`struct clone_opts` will hold this information. We could have put
`option_branch` and `option_single_branch` into that struct instead, but
in a following commit we'll be using `wants_head` as well.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/clone.c | 44 +++++++++++++++++++++++++++++---------------
remote.c | 2 +-
remote.h | 5 +++++
3 files changed, 35 insertions(+), 16 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index ef4af1f3e6bf0a33c13ed874587b35aba14fe43c..1d421c8f758e37a7219d2da680c7ef8699016171 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -57,6 +57,13 @@
*
*/
+struct clone_opts {
+ int wants_head;
+};
+#define CLONE_OPTS_INIT { \
+ .wants_head = 1 /* default enabled */ \
+}
+
static int option_no_checkout, option_bare, option_mirror, option_single_branch = -1;
static int option_local = -1, option_no_hardlinks, option_shared;
static int option_tags = 1; /* default enabled */
@@ -429,23 +436,24 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch
return ref;
}
-static struct ref *wanted_peer_refs(const struct ref *refs,
- struct refspec *refspec)
+static struct ref *wanted_peer_refs(struct clone_opts *opts,
+ const struct ref *refs,
+ struct refspec *refspec)
{
- struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
- struct ref *local_refs = head;
- struct ref **tail = local_refs ? &local_refs->next : &local_refs;
+ struct ref *local_refs = NULL;
+ struct ref **tail = &local_refs;
struct ref *to_free = NULL;
- if (option_single_branch) {
- if (!option_branch)
+ if (opts->wants_head) {
+ struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
+ if (head)
+ tail_link_ref(head, &tail);
+ if (option_single_branch)
refs = to_free = guess_remote_head(head, refs, 0);
- else {
- free_one_ref(head);
- local_refs = head = NULL;
- tail = &local_refs;
- refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
- }
+ } else if (option_single_branch) {
+ local_refs = NULL;
+ tail = &local_refs;
+ refs = to_free = copy_ref(find_remote_branch(refs, option_branch));
}
for (size_t i = 0; i < refspec->nr; i++)
@@ -893,6 +901,8 @@ int cmd_clone(int argc,
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ struct clone_opts opts = CLONE_OPTS_INIT;
+
struct transport_ls_refs_options transport_ls_refs_options =
TRANSPORT_LS_REFS_OPTIONS_INIT;
@@ -1343,9 +1353,13 @@ int cmd_clone(int argc,
if (option_not.nr)
transport_set_option(transport, TRANS_OPT_DEEPEN_NOT,
(const char *)&option_not);
- if (option_single_branch)
+ if (option_single_branch) {
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
+ if (option_branch)
+ opts.wants_head = 0;
+ }
+
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
@@ -1454,7 +1468,7 @@ int cmd_clone(int argc,
}
if (refs)
- mapped_refs = wanted_peer_refs(refs, &remote->fetch);
+ mapped_refs = wanted_peer_refs(&opts, refs, &remote->fetch);
if (mapped_refs) {
/*
diff --git a/remote.c b/remote.c
index 1779f0e7bbb8b88965f2fedf407e50ed20ea7a13..69d8c43ea694f1b9a7699e5d5a49bfc169058b8e 100644
--- a/remote.c
+++ b/remote.c
@@ -1260,7 +1260,7 @@ int count_refspec_match(const char *pattern,
}
}
-static void tail_link_ref(struct ref *ref, struct ref ***tail)
+void tail_link_ref(struct ref *ref, struct ref ***tail)
{
**tail = ref;
while (ref->next)
diff --git a/remote.h b/remote.h
index a19353f68999f5440db7bf5f91dd4be8bcc1d8a5..ce3e7c8512981d0ac5db2da508c4fbe64cede961 100644
--- a/remote.h
+++ b/remote.h
@@ -221,6 +221,11 @@ struct ref *alloc_ref(const char *name);
struct ref *copy_ref(const struct ref *ref);
struct ref *copy_ref_list(const struct ref *ref);
int count_refspec_match(const char *, struct ref *refs, struct ref **matched_ref);
+/*
+ * Put a ref in the tail and prepare tail for adding another one.
+ * *tail is the pointer to the tail of the list of refs.
+ */
+void tail_link_ref(struct ref *ref, struct ref ***tail);
int check_ref_type(const struct ref *ref, int flags);
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v7 6/7] parse-options: introduce die_for_incompatible_opt2()
2025-02-06 6:33 ` [PATCH v7 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (4 preceding siblings ...)
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 ` Toon Claes
2025-02-06 6:33 ` [PATCH v7 7/7] builtin/clone: teach git-clone(1) the --revision= option Toon Claes
6 siblings, 0 replies; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The functions die_for_incompatible_opt3() and
die_for_incompatible_opt4() already exist to die whenever a user
specifies three or four options respectively that are not compatible.
Introduce die_for_incompatible_opt2() which dies when two options that
are incompatible are set.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/replay.c | 7 ++++---
parse-options.h | 9 +++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/builtin/replay.c b/builtin/replay.c
index 1afc6d1ee0cb738fa7fa3f2b5c8ce0dd7802e7da..032c172b65ece4e2c3b275ffdde22a54c55a3933 100644
--- a/builtin/replay.c
+++ b/builtin/replay.c
@@ -163,9 +163,10 @@ static void determine_replay_mode(struct rev_cmdline_info *cmd_info,
get_ref_information(cmd_info, &rinfo);
if (!rinfo.positive_refexprs)
die(_("need some commits to replay"));
- if (onto_name && *advance_name)
- die(_("--onto and --advance are incompatible"));
- else if (onto_name) {
+
+ die_for_incompatible_opt2(!!onto_name, "--onto",
+ !!*advance_name, "--advance");
+ if (onto_name) {
*onto = peel_committish(onto_name);
if (rinfo.positive_refexprs <
strset_get_size(&rinfo.positive_refs))
diff --git a/parse-options.h b/parse-options.h
index 39f088625494f20dea96b9a9cbe986916773bf60..fca944d9a93d643d984c58de2ead9154c8b16c94 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -436,6 +436,15 @@ static inline void die_for_incompatible_opt3(int opt1, const char *opt1_name,
0, "");
}
+static inline void die_for_incompatible_opt2(int opt1, const char *opt1_name,
+ int opt2, const char *opt2_name)
+{
+ die_for_incompatible_opt4(opt1, opt1_name,
+ opt2, opt2_name,
+ 0, "",
+ 0, "");
+}
+
/*
* Use these assertions for callbacks that expect to be called with NONEG and
* NOARG respectively, and do not otherwise handle the "unset" and "arg"
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH v7 7/7] builtin/clone: teach git-clone(1) the --revision= option
2025-02-06 6:33 ` [PATCH v7 0/7] Enable doing a shallow clone of a specific git revision Toon Claes
` (5 preceding siblings ...)
2025-02-06 6:33 ` [PATCH v7 6/7] parse-options: introduce die_for_incompatible_opt2() Toon Claes
@ 2025-02-06 6:33 ` Toon Claes
2025-02-06 8:13 ` Patrick Steinhardt
6 siblings, 1 reply; 57+ messages in thread
From: Toon Claes @ 2025-02-06 6:33 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Michal Suchánek, Patrick Steinhardt,
Jeff King, Junio C Hamano, Toon Claes
The git-clone(1) command has the option `--branch` that allows the user
to select the branch they want HEAD to point to. In a non-bare
repository this also checks out that branch.
Option `--branch` also accepts a tag. When a tag name is provided, the
commit this tag points to is checked out and HEAD is detached. Thus
`--branch` can be used to clone a repository and check out a ref kept
under `refs/heads` or `refs/tags`. But some other refs might be in use
as well. For example Git forges might use refs like `refs/pull/<id>` and
`refs/merge-requests/<id>` to track pull/merge requests. These refs
cannot be selected upon git-clone(1).
Add option `--revision` to git-clone(1). This option accepts a fully
qualified reference, or a hexadecimal commit ID. This enables the user
to clone and check out any revision they want. `--revision` can be used
in conjunction with `--depth` to do a minimal clone that only contains
the blob and tree for a single revision. This can be useful for
automated tests running in CI systems.
Using option `--branch` and `--single-branch` together is a similar
scenario, but serves a different purpose. Using these two options, a
singlet remote tracking branch is created and the fetch refspec is set
up so git-fetch(1) will receive updates on that branch from the remote.
This allows the user work on that single branch.
Option `--revision` on contrary detaches HEAD, creates no tracking
branches, and writes no fetch refspec.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
Documentation/git-clone.txt | 9 ++++
builtin/clone.c | 57 ++++++++++++++++----
t/meson.build | 1 +
t/t5621-clone-revision.sh | 123 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 179 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 8d0476f6dcaf6fed7ccd48a20398556dd4e20722..1069d56e7126c006c14c6b4579547f79d7fe4fb5 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -221,6 +221,15 @@ objects from the source repository into a pack in the cloned repository.
`--branch` can also take tags and detaches the `HEAD` at that commit
in the resulting repository.
+`--revision=<rev>`::
+ Create a new repository, and fetch the history leading to the given
+ revision _<rev>_ (and nothing else), without making any remote-tracking
+ branch, and without making any local branch, and detach `HEAD` to
+ _<rev>_. The argument can be a ref name (e.g. `refs/heads/main` or
+ `refs/tags/v1.0`) that peels down to a commit, or a hexadecimal object
+ name.
+ This option is incompatible with `--branch` and `--mirror`.
+
`-u` _<upload-pack>_::
`--upload-pack` _<upload-pack>_::
When given, and the repository to clone from is accessed
diff --git a/builtin/clone.c b/builtin/clone.c
index 1d421c8f758e37a7219d2da680c7ef8699016171..f9a2ecbe9cc944793203d722b42573e779575c5a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -59,6 +59,7 @@
struct clone_opts {
int wants_head;
+ int detach;
};
#define CLONE_OPTS_INIT { \
.wants_head = 1 /* default enabled */ \
@@ -565,11 +566,11 @@ static void update_remote_refs(const struct ref *refs,
}
}
-static void update_head(const struct ref *our, const struct ref *remote,
+static void update_head(struct clone_opts *opts, const struct ref *our, const struct ref *remote,
const char *unborn, const char *msg)
{
const char *head;
- if (our && skip_prefix(our->name, "refs/heads/", &head)) {
+ if (our && !opts->detach && skip_prefix(our->name, "refs/heads/", &head)) {
/* Local default branch link */
if (refs_update_symref(get_main_ref_store(the_repository), "HEAD", our->name, NULL) < 0)
die(_("unable to update HEAD"));
@@ -580,8 +581,9 @@ static void update_head(const struct ref *our, const struct ref *remote,
install_branch_config(0, head, remote_name, our->name);
}
} else if (our) {
- struct commit *c = lookup_commit_reference(the_repository,
- &our->old_oid);
+ struct commit *c = lookup_commit_or_die(&our->old_oid,
+ our->name);
+
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
refs_update_ref(get_main_ref_store(the_repository), msg,
"HEAD", &c->object.oid, NULL, REF_NO_DEREF,
@@ -900,6 +902,7 @@ int cmd_clone(int argc,
int option_filter_submodules = -1; /* unspecified */
struct string_list server_options = STRING_LIST_INIT_NODUP;
const char *bundle_uri = NULL;
+ char *option_rev = NULL;
struct clone_opts opts = CLONE_OPTS_INIT;
@@ -943,6 +946,8 @@ int cmd_clone(int argc,
N_("use <name> instead of 'origin' to track upstream")),
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")),
OPT_STRING('u', "upload-pack", &option_upload_pack, N_("path"),
N_("path to git-upload-pack on the remote")),
OPT_STRING(0, "depth", &option_depth, N_("depth"),
@@ -1279,7 +1284,7 @@ int cmd_clone(int argc,
strbuf_addstr(&branch_top, src_ref_prefix);
git_config_set("core.bare", "true");
- } else {
+ } else if (!option_rev) {
strbuf_addf(&branch_top, "refs/remotes/%s/", remote_name);
}
@@ -1298,8 +1303,9 @@ int cmd_clone(int argc,
remote = remote_get_early(remote_name);
- 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);
path = get_repo_path(remote->url.v[0], &is_bundle);
is_local = option_local != 0 && path && !is_bundle;
@@ -1342,6 +1348,11 @@ int cmd_clone(int argc,
transport_set_option(transport, TRANS_OPT_KEEP, "yes");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ !!option_branch, "--branch");
+ die_for_incompatible_opt2(!!option_rev, "--revision",
+ option_mirror, "--mirror");
+
if (reject_shallow)
transport_set_option(transport, TRANS_OPT_REJECT_SHALLOW, "1");
if (option_depth)
@@ -1378,7 +1389,14 @@ 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_single_branch = 0;
+ opts.wants_head = 0;
+ opts.detach = 1;
+
+ refspec_append(&remote->fetch, option_rev);
+ }
if (option_tags || option_branch)
/*
@@ -1393,6 +1411,17 @@ int cmd_clone(int argc,
expand_ref_prefix(&transport_ls_refs_options.ref_prefixes,
option_branch);
+ /*
+ * As part of transport_get_remote_refs() the server tells us the hash
+ * algorithm, which we require to initialize the repo. But calling that
+ * function without any ref prefix, will cause the server to announce
+ * all known refs. If the argument passed to --revision was a hex oid,
+ * ref_prefixes will be empty so we fall back to asking about HEAD to
+ * reduce traffic from the server.
+ */
+ if (opts.wants_head || transport_ls_refs_options.ref_prefixes.nr == 0)
+ strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+
refs = transport_get_remote_refs(transport, &transport_ls_refs_options);
/*
@@ -1501,6 +1530,11 @@ int cmd_clone(int argc,
if (!our_head_points_at)
die(_("Remote branch %s not found in upstream %s"),
option_branch, remote_name);
+ } else if (option_rev) {
+ our_head_points_at = mapped_refs;
+ if (!our_head_points_at)
+ die(_("Remote revision %s not found in upstream %s"),
+ option_rev, remote_name);
} else if (remote_head_points_at) {
our_head_points_at = remote_head_points_at;
} else if (remote_head) {
@@ -1539,8 +1573,9 @@ int cmd_clone(int argc,
free(to_free);
}
- write_refspec_config(src_ref_prefix, our_head_points_at,
- remote_head_points_at, &branch_top);
+ if (!option_rev)
+ write_refspec_config(src_ref_prefix, our_head_points_at,
+ remote_head_points_at, &branch_top);
if (filter_options.choice)
partial_clone_register(remote_name, &filter_options);
@@ -1556,7 +1591,7 @@ int cmd_clone(int argc,
branch_top.buf, reflog_msg.buf, transport,
!is_local);
- update_head(our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
+ update_head(&opts, our_head_points_at, remote_head, unborn_head, reflog_msg.buf);
/*
* We want to show progress for recursive submodule clones iff
diff --git a/t/meson.build b/t/meson.build
index 35f25ca4a1d960564190288e9456620a46ccc80a..b5f917926b61de379b6cef45e5f750912422a7d1 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -721,6 +721,7 @@ integration_tests = [
't5617-clone-submodules-remote.sh',
't5618-alternate-refs.sh',
't5619-clone-local-ambiguous-transport.sh',
+ 't5621-clone-revision.sh',
't5700-protocol-v1.sh',
't5701-git-serve.sh',
't5702-protocol-v2.sh',
diff --git a/t/t5621-clone-revision.sh b/t/t5621-clone-revision.sh
new file mode 100755
index 0000000000000000000000000000000000000000..d4889a954e6300e0e327ebe7dfcf73569d966829
--- /dev/null
+++ b/t/t5621-clone-revision.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+
+test_description='tests for git clone --revision'
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit --no-tag "initial commit" README "Hello" &&
+ test_commit --annotate "second commit" README "Hello world" v1.0 &&
+ test_commit --no-tag "third commit" README "Hello world!" &&
+ git switch -c feature v1.0 &&
+ test_commit --no-tag "feature commit" README "Hello world!" &&
+ git switch main
+'
+
+test_expect_success 'clone with --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --depth and --revision being a branch' '
+ test_when_finished "rm -rf dst" &&
+ git clone --no-local --depth=1 --revision=refs/heads/feature . dst &&
+ git rev-parse refs/heads/feature >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch &&
+ git -C dst rev-list HEAD >actual &&
+ test_line_count = 1 actual
+'
+
+test_expect_success 'clone with --revision being a tag' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/tags/v1.0 . dst &&
+ git rev-parse refs/tags/v1.0^{} >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being HEAD' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=HEAD . dst &&
+ git rev-parse HEAD >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature) &&
+ git clone --revision=$oid . dst &&
+ echo $oid >expect &&
+ git -C dst rev-parse HEAD >actual &&
+ test_must_fail git -C dst symbolic-ref -q HEAD >/dev/null &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision and --bare' '
+ test_when_finished "rm -rf dst" &&
+ git clone --revision=refs/heads/main --bare . dst &&
+ oid=$(git rev-parse refs/heads/main) &&
+ git -C dst cat-file -t $oid >actual &&
+ echo "commit" >expect &&
+ test_cmp expect actual &&
+ git -C dst for-each-ref refs >expect &&
+ test_must_be_empty expect &&
+ test_must_fail git -C dst config remote.origin.fetch
+'
+
+test_expect_success 'clone with --revision being a short raw commit hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse --short refs/heads/feature) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "fatal: Remote revision $oid not found in upstream origin" err
+'
+
+test_expect_success 'clone with --revision being a tree hash' '
+ test_when_finished "rm -rf dst" &&
+ oid=$(git rev-parse refs/heads/feature^{tree}) &&
+ test_must_fail git clone --revision=$oid . dst 2>err &&
+ test_grep "error: object $oid is a tree, not a commit" err
+'
+
+test_expect_success 'clone with --revision being the parent of a ref fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main^ . dst
+'
+
+test_expect_success 'clone with --revision and --branch fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --branch=main . dst
+'
+
+test_expect_success 'clone with --revision and --mirror fails' '
+ test_when_finished "rm -rf dst" &&
+ test_must_fail git clone --revision=refs/heads/main --mirror . dst
+'
+
+test_done
--
2.48.1.447.gc0086e9015
^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: [PATCH v7 7/7] builtin/clone: teach git-clone(1) the --revision= option
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
0 siblings, 1 reply; 57+ messages in thread
From: Patrick Steinhardt @ 2025-02-06 8:13 UTC (permalink / raw)
To: Toon Claes
Cc: git, Kristoffer Haugsbakk, Michal Suchánek, Jeff King,
Junio C Hamano
On Thu, Feb 06, 2025 at 07:33:35AM +0100, Toon Claes wrote:
> diff --git a/t/t5621-clone-revision.sh b/t/t5621-clone-revision.sh
> new file mode 100755
> index 0000000000000000000000000000000000000000..d4889a954e6300e0e327ebe7dfcf73569d966829
> --- /dev/null
> +++ b/t/t5621-clone-revision.sh
> @@ -0,0 +1,123 @@
> +#!/bin/sh
> +
> +test_description='tests for git clone --revision'
> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +
> +TEST_PASSES_SANITIZE_LEAK=true
One last nit: this line is not needed anymore, as tests are required to
pass with the leak sanitizer by default now. Other than that this series
looks good to me, and this change alone does not warrant a reroll from
my point of view.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v7 7/7] builtin/clone: teach git-clone(1) the --revision= option
2025-02-06 8:13 ` Patrick Steinhardt
@ 2025-02-06 20:26 ` Junio C Hamano
0 siblings, 0 replies; 57+ messages in thread
From: Junio C Hamano @ 2025-02-06 20:26 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Toon Claes, git, Kristoffer Haugsbakk, Michal Suchánek,
Jeff King
Patrick Steinhardt <ps@pks.im> writes:
> On Thu, Feb 06, 2025 at 07:33:35AM +0100, Toon Claes wrote:
>> diff --git a/t/t5621-clone-revision.sh b/t/t5621-clone-revision.sh
>> new file mode 100755
>> index 0000000000000000000000000000000000000000..d4889a954e6300e0e327ebe7dfcf73569d966829
>> --- /dev/null
>> +++ b/t/t5621-clone-revision.sh
>> @@ -0,0 +1,123 @@
>> +#!/bin/sh
>> +
>> +test_description='tests for git clone --revision'
>> +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>> +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>> +
>> +TEST_PASSES_SANITIZE_LEAK=true
>
> One last nit: this line is not needed anymore, as tests are required to
> pass with the leak sanitizer by default now. Other than that this series
> looks good to me, and this change alone does not warrant a reroll from
> my point of view.
>
> Thanks!
I'll touch it up while queuing, and mark the topic for 'next', then.
Thanks.
^ permalink raw reply [flat|nested] 57+ messages in thread