threads / patch / 42893

patchsubmodule: do no re-read name in shell script

Subject: [PATCH] submodule: do no re-read name in shell script

## tl;dr

3 messages between Jul 21, 2016 and Jul 21, 2016. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Stefan Beller· Jul 21, 2016, 00:51 UTC · lore

Instead of making another call to a submodule helper (name), just propagate the value when we know it (in the update-clone helper) already.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 4 ++--
 git-submodule.sh            | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

Show changes to 2 files +3 −4

builtin/submodule--helper.c, git-submodule.sh

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b22352b..494e088 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -683,9 +683,9 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 	needs_cloning = !file_exists(sb.buf);
 
 	strbuf_reset(&sb);
-	strbuf_addf(&sb, "%06o %s %d %d\t%s\n", ce->ce_mode,
+	strbuf_addf(&sb, "%06o %s %d %d %s\t%s\n", ce->ce_mode,
 			sha1_to_hex(ce->sha1), ce_stage(ce),
-			needs_cloning, ce->name);
+			needs_cloning, sub->name, sub->path);
 	string_list_append(&suc->projectlines, sb.buf);
 
 	if (!needs_cloning)
diff --git a/git-submodule.sh b/git-submodule.sh
index 4ec7546..e23aada 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -584,11 +584,10 @@ cmd_update()
 		"$@" || echo "#unmatched"
 	} | {
 	err=
-	while read mode sha1 stage just_cloned sm_path
+	while read mode sha1 stage just_cloned name sm_path
 	do
 		die_if_unmatched "$mode"
 
-		name=$(git submodule--helper name "$sm_path") || exit
 		url=$(git config submodule."$name".url)
 		branch=$(get_submodule_config "$name" branch master)
 		if ! test -z "$update"

-- 
2.9.2.369.g0e67330

Jonathan Nieder· Jul 21, 2016, 01:00 UTC · re: Stefan Beller · lore

Re: [PATCH] submodule: do no re-read name in shell script

Stefan Beller wrote:

Show 22 quoted lines

> Instead of making another call to a submodule helper (name), just
> propagate the value when we know it (in the update-clone helper) already.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  builtin/submodule--helper.c | 4 ++--
>  git-submodule.sh            | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b22352b..494e088 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -683,9 +683,9 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>  	needs_cloning = !file_exists(sb.buf);
> 
>  	strbuf_reset(&sb);
> -	strbuf_addf(&sb, "%06o %s %d %d\t%s\n", ce->ce_mode,
> +	strbuf_addf(&sb, "%06o %s %d %d %s\t%s\n", ce->ce_mode,
>  			sha1_to_hex(ce->sha1), ce_stage(ce),
> -			needs_cloning, ce->name);
> +			needs_cloning, sub->name, sub->path);

Are there any restrictions on what characters a submodule name can contain? Does this need e.g. to be quoted with sq_quote?

Thanks, Jonathan

Stefan Beller· Jul 21, 2016, 01:40 UTC · re: Jonathan Nieder · lore

Re: [PATCH] submodule: do no re-read name in shell script

On Wed, Jul 20, 2016 at 6:00 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 5 quoted lines

>> -                     needs_cloning, ce->name);
>> +                     needs_cloning, sub->name, sub->path);
>
> Are there any restrictions on what characters a submodule name can
> contain?  Does this need e.g. to be quoted with sq_quote?

Oh good point, this ought to fail with weird names. So instead of quoting it, let's just drop this patch.

>
> Thanks,
> Jonathan