threads / rfc / 25630

RFC patchclone: Deprecate the --recursive option in favor of --recurse-submodules

Subject: [RFC PATCH] clone: Deprecate the --recursive option in favor of --recurse-submodules

## tl;dr

5 messages between Nov 3, 2010 and Nov 4, 2010. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Jens Lehmann· Nov 3, 2010, 18:41 UTC · lore

Since 1.6.5 "git clone" honors the --recursive option to recursively check out submodules too. As this option can easily be misinterpreted when it is added to other commands like "git grep", add the new --recurse-submodules option to avoid confusing it with other types of recursion.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

At the GitTogether some of us were talking about a sane option name for recursing submodules which should then be used consistently by all commands (maybe with the exception of "git submodule", as --recursive there has a pretty obvious meaning). For my first recursion patches a few months ago I started with --recurse-submodules but then I noticed that "git clone" already used "--recursive" for the same purpose, and for consistency reasons I switched to using that too. But especially when looking at recursive grep it is really easy to misinterpret --recursive, so the idea came up to use --recurse-submodules everywhere.

Opinions?
 Documentation/git-clone.txt |    5 +++--
 builtin/clone.c             |   10 ++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)
Show changes to 2 files +11 −3

Documentation/git-clone.txt, builtin/clone.c

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index ab72933..1768cef 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git clone' [--template=<template_directory>]
 	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
 	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
-	  [--depth <depth>] [--recursive] [--] <repository> [<directory>]
+	  [--depth <depth>] [--recurse-submodules] [--] <repository> [<directory>]

 DESCRIPTION
 -----------
@@ -166,13 +166,14 @@ objects from the source repository into a pack in the cloned repository.
 	with a long history, and would want to send in fixes
 	as patches.

---recursive::
+--recurse-submodules::
 	After the clone is created, initialize all submodules within,
 	using their default settings. This is equivalent to running
 	`git submodule update --init --recursive` immediately after
 	the clone is finished. This option is ignored if the cloned
 	repository does not have a worktree/checkout (i.e. if any of
 	`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+	This option replaces the now deprecated `--recursive`.

 <repository>::
 	The (possibly remote) repository to clone from.  See the
diff --git a/builtin/clone.c b/builtin/clone.c
index 19ed640..91e7fab 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -43,7 +43,7 @@ static char *option_template, *option_reference, *option_depth;
 static char *option_origin = NULL;
 static char *option_branch = NULL;
 static char *option_upload_pack = "git-upload-pack";
-static int option_verbosity;
+static int option_verbosity, option_recurse_submodules;
 static int option_progress;

 static struct option builtin_clone_options[] = {
@@ -65,6 +65,8 @@ static struct option builtin_clone_options[] = {
 	OPT_BOOLEAN('s', "shared", &option_shared,
 		    "setup as shared repository"),
 	OPT_BOOLEAN(0, "recursive", &option_recursive,
+		    "initialize submodules in the clone (deprecated)"),
+	OPT_BOOLEAN(0, "recurse_submodules", &option_recurse_submodules,
 		    "initialize submodules in the clone"),
 	OPT_STRING(0, "template", &option_template, "path",
 		   "path the template repository"),
@@ -659,7 +661,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				sha1_to_hex(our_head_points_at->old_sha1), "1",
 				NULL);

-		if (!err && option_recursive)
+		if (option_recursive) {
+			warning("--recursive is deprecated; use --recurse-submodules instead.");
+			option_recurse_submodules = 1;
+		}
+		if (!err && option_recurse_submodules)
 			err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
 	}
-- 
1.7.3.2.213.g5fe186
Jakub Narebski· Nov 3, 2010, 22:13 UTC · re: Jens Lehmann · lore

Re: [RFC PATCH] clone: Deprecate the --recursive option in favor of --recurse-submodules

Jens Lehmann <Jens.Lehmann@web.de> writes:
Show 9 quoted lines
> @@ -65,6 +65,8 @@ static struct option builtin_clone_options[] = {
>  	OPT_BOOLEAN('s', "shared", &option_shared,
>  		    "setup as shared repository"),
>  	OPT_BOOLEAN(0, "recursive", &option_recursive,
> +		    "initialize submodules in the clone (deprecated)"),
> +	OPT_BOOLEAN(0, "recurse_submodules", &option_recurse_submodules,
>  		    "initialize submodules in the clone"),
>  	OPT_STRING(0, "template", &option_template, "path",
>  		   "path the template repository"),
Shouldn't we use PARSE_OPT_HIDDEN?  Or should it be left for later?
-- 
Jakub Narebski
Poland
ShadeHawk on #git
Sverre Rabbelier· Nov 3, 2010, 22:24 UTC · re: Jakub Narebski · lore

Re: [RFC PATCH] clone: Deprecate the --recursive option in favor of --recurse-submodules

Heya,
On Wed, Nov 3, 2010 at 23:13, Jakub Narebski <jnareb@gmail.com> wrote:
> Shouldn't we use PARSE_OPT_HIDDEN?  Or should it be left for later?

I assumed we would add PARSE_OPT_HIDDEN in a few releases. Perhaps that patch should also be sent.

-- 
Cheers,

Sverre Rabbelier
Junio C Hamano· Nov 3, 2010, 22:36 UTC · re: Jens Lehmann · lore

Re: [RFC PATCH] clone: Deprecate the --recursive option in favor of --recurse-submodules

Jens Lehmann <Jens.Lehmann@web.de> writes:
Show 8 quoted lines
> ... For my first recursion patches a
> few months ago I started with --recurse-submodules but then I noticed
> that "git clone" already used "--recursive" for the same purpose, and
> for consistency reasons I switched to using that too. But especially
> when looking at recursive grep it is really easy to misinterpret
> --recursive, so the idea came up to use --recurse-submodules everywhere.
>
> Opinions?

I think it would make sense to _add_ --recurse-submodules to "clone" to make everybody consistent.

With the introduction of --recurse-submodules option, the user can rest assured that --recurse-submodules is understood by any command that knows how to descend into submodules. A command may have --recursive option (or it may not), and when it does, the option may or may not mean descending into submodules. E.g. if "git ls-tree" learns long options, --recursive would mean "recurse into subtrees" and you would say --no-recursive to countermand.

As it does not make any sense to say "clone --no-recursive" to limit the clone to the top-level directory, --recursive to "clone" would need to mean "descend into subprojects". So in that sense, "clone --recursive" shares a similar degree of obviousness with "submodule --recursive". I thing it is too strong to _deprecate_ the option in the context of that command.

Jens Lehmann· Nov 4, 2010, 20:27 UTC · re: Junio C Hamano · lore

[PATCH] clone: Add the --recurse-submodules option as alias for --recursive

Since 1.6.5 "git clone" honors the --recursive option to recursively check out submodules too. As this option can easily be misinterpreted when it is added to other commands like "git grep", add the new --recurse-submodules option as an alias for --recursive so the same option can be used for all commands recursing into submodules.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Am 03.11.2010 23:36, schrieb Junio C Hamano:
Show 16 quoted lines
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> ... For my first recursion patches a
>> few months ago I started with --recurse-submodules but then I noticed
>> that "git clone" already used "--recursive" for the same purpose, and
>> for consistency reasons I switched to using that too. But especially
>> when looking at recursive grep it is really easy to misinterpret
>> --recursive, so the idea came up to use --recurse-submodules everywhere.
>>
>> Opinions?
> 
> I think it would make sense to _add_ --recurse-submodules to "clone" to
> make everybody consistent.
> ... I
> thing it is too strong to _deprecate_ the option in the context of that
> command.
Fine by me, then what about this patch?

And am I right to assume that there are no objections against using --recurse-submodule for fetch, checkout & friends?

 Documentation/git-clone.txt |    4 +++-
 builtin/clone.c             |    2 ++
 2 files changed, 5 insertions(+), 1 deletions(-)
Show changes to 2 files +5 −1

Documentation/git-clone.txt, builtin/clone.c

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index ab72933..d69984b 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -12,7 +12,8 @@ SYNOPSIS
 'git clone' [--template=<template_directory>]
 	  [-l] [-s] [--no-hardlinks] [-q] [-n] [--bare] [--mirror]
 	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
-	  [--depth <depth>] [--recursive] [--] <repository> [<directory>]
+	  [--depth <depth>] [--recursive|--recurse-submodules] [--] <repository>
+	  [<directory>]

 DESCRIPTION
 -----------
@@ -167,6 +168,7 @@ objects from the source repository into a pack in the cloned repository.
 	as patches.

 --recursive::
+--recurse-submodules::
 	After the clone is created, initialize all submodules within,
 	using their default settings. This is equivalent to running
 	`git submodule update --init --recursive` immediately after
diff --git a/builtin/clone.c b/builtin/clone.c
index 19ed640..61e0989 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -66,6 +66,8 @@ static struct option builtin_clone_options[] = {
 		    "setup as shared repository"),
 	OPT_BOOLEAN(0, "recursive", &option_recursive,
 		    "initialize submodules in the clone"),
+	OPT_BOOLEAN(0, "recurse_submodules", &option_recursive,
+		    "initialize submodules in the clone"),
 	OPT_STRING(0, "template", &option_template, "path",
 		   "path the template repository"),
 	OPT_STRING(0, "reference", &option_reference, "repo",
-- 
1.7.3.2.194.ge0b67

← back to recent threads