git-submodule is missing --dissociate option

git-submodule is missing --dissociate option

[Casey Fitzpatrick]

This seems to be a hole in the git feature set. I believe it is fairly
easily worked around, but it would be best to provide the option for
ease of use (and maybe performance?).

git clone has both a --reference feature and a --dissociate option,
with dissociate allowing for a reference to *only* speed up network
transfers rather than have the resulting clone rely upon the reference
always being there (creates an independent repo).
But git submodule only allows for --reference, so there isn't a an
option to make a speedy independent submodule clone in one shot:
https://git-scm.com/docs/git-submodule
I checked the latest online documentation (currently at 2.16.3) and
the documentation in the latest sources (almost 2.18):
https://github.com/git/git/blob/next/Documentation/git-submodule.txt

As far as I am aware this can be worked around with 'git repack -a'
and manual removal of the objects/info/alternates file afterward.
Though I don't know if this results in a less speedy clone than
dissociate would.

Re: git-submodule is missing --dissociate option

[Casey Fitzpatrick]

It also seems to be missing "--progress", and I imagine others.
Perhaps submodule add/update should be reworked to automatically
accept all the options that clone would?

On Mon, Apr 30, 2018 at 4:29 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
> This seems to be a hole in the git feature set. I believe it is fairly
> easily worked around, but it would be best to provide the option for
> ease of use (and maybe performance?).
>
> git clone has both a --reference feature and a --dissociate option,
> with dissociate allowing for a reference to *only* speed up network
> transfers rather than have the resulting clone rely upon the reference
> always being there (creates an independent repo).
> But git submodule only allows for --reference, so there isn't a an
> option to make a speedy independent submodule clone in one shot:
> https://git-scm.com/docs/git-submodule
> I checked the latest online documentation (currently at 2.16.3) and
> the documentation in the latest sources (almost 2.18):
> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>
> As far as I am aware this can be worked around with 'git repack -a'
> and manual removal of the objects/info/alternates file afterward.
> Though I don't know if this results in a less speedy clone than
> dissociate would.

Re: git-submodule is missing --dissociate option

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 30, 2018 at 1:30 PM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
> It also seems to be missing "--progress", and I imagine others.
> Perhaps submodule add/update should be reworked to automatically
> accept all the options that clone would?

--progress is not missing, but I see that it isn't documented. It was
added in 72c5f88311 ("clone: pass --progress decision to recursive
submodules", 2016-09-22). What you're suggesting makes sense, but as
shown in that commit it's not easy for it to happen automatically,
there's a lot of boilerplate involved.

But since you're interested you can see how to add new options with
that patch, it should be easy for anyone not experienced with the
codebase, it's all just boilerplate + adding a test.

> On Mon, Apr 30, 2018 at 4:29 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
>> This seems to be a hole in the git feature set. I believe it is fairly
>> easily worked around, but it would be best to provide the option for
>> ease of use (and maybe performance?).
>>
>> git clone has both a --reference feature and a --dissociate option,
>> with dissociate allowing for a reference to *only* speed up network
>> transfers rather than have the resulting clone rely upon the reference
>> always being there (creates an independent repo).
>> But git submodule only allows for --reference, so there isn't a an
>> option to make a speedy independent submodule clone in one shot:
>> https://git-scm.com/docs/git-submodule
>> I checked the latest online documentation (currently at 2.16.3) and
>> the documentation in the latest sources (almost 2.18):
>> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>>
>> As far as I am aware this can be worked around with 'git repack -a'
>> and manual removal of the objects/info/alternates file afterward.
>> Though I don't know if this results in a less speedy clone than
>> dissociate would.

Re: git-submodule is missing --dissociate option

[Stefan Beller]

On Mon, Apr 30, 2018 at 1:29 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
> This seems to be a hole in the git feature set. I believe it is fairly
> easily worked around, but it would be best to provide the option for
> ease of use (and maybe performance?).
>
> git clone has both a --reference feature and a --dissociate option,
> with dissociate allowing for a reference to *only* speed up network
> transfers rather than have the resulting clone rely upon the reference
> always being there (creates an independent repo).

With the advent of git-worktree, I claim that --reference without further
--dissociate is dangerous, such that the combination of these two are
not the best UX, you could wish for.

> But git submodule only allows for --reference, so there isn't a an
> option to make a speedy independent submodule clone in one shot:
> https://git-scm.com/docs/git-submodule
> I checked the latest online documentation (currently at 2.16.3) and
> the documentation in the latest sources (almost 2.18):
> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>
> As far as I am aware this can be worked around with 'git repack -a'
> and manual removal of the objects/info/alternates file afterward.
> Though I don't know if this results in a less speedy clone than
> dissociate would.

That is an interesting workaround!
I agree we should have the --dissociate option available for submodules.
Care to implement it?

Thanks,
Stefan

Re: git-submodule is missing --dissociate option

[Casey Fitzpatrick]

Sure, I'll take a crack at it and submit a patch.

On Mon, Apr 30, 2018 at 2:19 PM, Stefan Beller <sbeller@google.com> wrote:
> On Mon, Apr 30, 2018 at 1:29 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
>> This seems to be a hole in the git feature set. I believe it is fairly
>> easily worked around, but it would be best to provide the option for
>> ease of use (and maybe performance?).
>>
>> git clone has both a --reference feature and a --dissociate option,
>> with dissociate allowing for a reference to *only* speed up network
>> transfers rather than have the resulting clone rely upon the reference
>> always being there (creates an independent repo).
>
> With the advent of git-worktree, I claim that --reference without further
> --dissociate is dangerous, such that the combination of these two are
> not the best UX, you could wish for.
>
>> But git submodule only allows for --reference, so there isn't a an
>> option to make a speedy independent submodule clone in one shot:
>> https://git-scm.com/docs/git-submodule
>> I checked the latest online documentation (currently at 2.16.3) and
>> the documentation in the latest sources (almost 2.18):
>> https://github.com/git/git/blob/next/Documentation/git-submodule.txt
>>
>> As far as I am aware this can be worked around with 'git repack -a'
>> and manual removal of the objects/info/alternates file afterward.
>> Though I don't know if this results in a less speedy clone than
>> dissociate would.
>
> That is an interesting workaround!
> I agree we should have the --dissociate option available for submodules.
> Care to implement it?
>
> Thanks,
> Stefan

[PATCH 0/2] Add --progress and --dissociate to git submodule

[Casey Fitzpatrick]

These patches add --progress and --dissociate options to git submodule.

The --progress option existed beforehand, but only for the update command and
it was left undocumented.

Both add and update submodule commands supported --reference, but not its pair
option --dissociate which allows for independent clones rather than depending
on the reference.

I apologize for any errors I may have made in creation of these patches or the
formatting of these emails. This is the first time I have submitted patches to
a mailing list.

Casey Fitzpatrick (2):
  submodule: Add --progress option to add command
  submodule: Add --dissociate option to add/update commands

 Documentation/git-submodule.txt | 17 ++++++++++++++++-
 builtin/submodule--helper.c     | 17 ++++++++++++++---
 git-submodule.sh                | 13 ++++++++++++-
 t/t7400-submodule-basic.sh      | 31 +++++++++++++++++++++++++++++++
 t/t7408-submodule-reference.sh  | 17 +++++++++++++++++
 5 files changed, 90 insertions(+), 5 deletions(-)

-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 1/2] submodule: Add --progress option to add command

[Casey Fitzpatrick]

--progress was missing from add command, was only in update.
Add --progress where it makes sense (both clone helper commands).
Add missing documentation entry.
Add test.

Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
---
 Documentation/git-submodule.txt |  7 +++++++
 git-submodule.sh                |  5 ++++-
 t/t7400-submodule-basic.sh      | 31 +++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 71c5618e8..d1ebe32e8 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -239,6 +239,13 @@ OPTIONS
 --quiet::
 	Only print error messages.
 
+--progress::
+	This option is only valid for add and update commands.
+	Progress status is reported on the standard error stream
+	by default when it is attached to a terminal, unless -q
+	is specified. This flag forces progress status even if the
+	standard error stream is not directed to a terminal.
+
 --all::
 	This option is only valid for the deinit command. Unregister all
 	submodules in the working tree.
diff --git a/git-submodule.sh b/git-submodule.sh
index 24914963c..aa1c0bb67 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -117,6 +117,9 @@ cmd_add()
 		-q|--quiet)
 			GIT_QUIET=1
 			;;
+		--progress)
+			progress="--progress"
+			;;
 		--reference)
 			case "$2" in '') usage ;; esac
 			reference_path=$2
@@ -255,7 +258,7 @@ or you are unsure what this means choose another name with the '--name' option."
 				eval_gettextln "Reactivating local git directory for submodule '\$sm_name'."
 			fi
 		fi
-		git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
+		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
 		(
 			sanitize_submodule_env
 			cd "$sm_path" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a39e69a3e..d7225a74b 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -126,6 +126,37 @@ test_expect_success 'submodule add' '
 	test_cmp empty untracked
 '
 
+test_expect_success 'setup test repo' '
+	mkdir parent &&
+	(cd parent && git init &&
+	 echo one >file && git add file &&
+	 git commit -m one)
+'
+
+test_expect_success 'clone -o' '
+	git clone -o foo parent clone-o &&
+	(cd clone-o && git rev-parse --verify refs/remotes/foo/master)
+'
+
+test_expect_success 'redirected submodule add does not show progress' '
+	(
+		cd addtest &&
+		git submodule add "file://$submodurl/parent" submod-redirected \
+			>out 2>err &&
+		! grep % err &&
+		test_i18ngrep ! "Checking connectivity" err
+	)
+'
+
+test_expect_success 'redirected submodule add --progress does show progress' '
+	(
+		cd addtest &&
+		git submodule add --progress "file://$submodurl/parent" \
+			submod-redirected-progress >out 2>err && \
+		grep % err
+	)
+'
+
 test_expect_success 'submodule add to .gitignored path fails' '
 	(
 		cd addtest-ignore &&
-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 2/2] submodule: Add --dissociate option to add/update commands

[Casey Fitzpatrick]

Add --dissociate option to add and update commands, both clone helper commands
that already have the --reference option --dissociate pairs with.
Add documentation.
Add tests.

Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
---
 Documentation/git-submodule.txt | 10 +++++++++-
 builtin/submodule--helper.c     | 17 ++++++++++++++---
 git-submodule.sh                | 10 +++++++++-
 t/t7408-submodule-reference.sh  | 17 +++++++++++++++++
 4 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d1ebe32e8..a75b95043 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -369,7 +369,15 @@ the submodule itself.
 	this option will be passed to the linkgit:git-clone[1] command.
 +
 *NOTE*: Do *not* use this option unless you have read the note
-for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
+for linkgit:git-clone[1]'s `--reference`, `--shared`, and `--dissociate`
+options carefully.
+
+--dissociate::
+	This option is only valid for add and update commands.  These
+	commands sometimes need to clone a remote repository. In this case,
+	this option will be passed to the linkgit:git-clone[1] command.
++
+*NOTE*: see the NOTE for the `--reference` option.
 
 --recursive::
 	This option is only valid for foreach, update, status and sync commands.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6ba8587b6..655f84f3c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1056,7 +1056,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
 }
 
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
-			   const char *depth, struct string_list *reference,
+			   const char *depth, struct string_list *reference, int dissociate,
 			   int quiet, int progress)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -1075,6 +1075,9 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
 			argv_array_pushl(&cp.args, "--reference",
 					 item->string, NULL);
 	}
+	if (dissociate) {
+		argv_array_push(&cp.args, "--dissociate");
+	}
 	if (gitdir && *gitdir)
 		argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL);
 
@@ -1190,6 +1193,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	char *p, *path = NULL, *sm_gitdir;
 	struct strbuf sb = STRBUF_INIT;
 	struct string_list reference = STRING_LIST_INIT_NODUP;
+	int dissociate = 0;
 	char *sm_alternate = NULL, *error_strategy = NULL;
 
 	struct option module_clone_options[] = {
@@ -1208,6 +1212,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 		OPT_STRING_LIST(0, "reference", &reference,
 			   N_("repo"),
 			   N_("reference repository")),
+		OPT_BOOL(0, "dissociate", &dissociate,
+			   N_("use --reference only while cloning")),
 		OPT_STRING(0, "depth", &depth,
 			   N_("string"),
 			   N_("depth for shallow clones")),
@@ -1247,7 +1253,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 
 		prepare_possible_alternates(name, &reference);
 
-		if (clone_submodule(path, sm_gitdir, url, depth, &reference,
+		if (clone_submodule(path, sm_gitdir, url, depth, &reference, dissociate,
 				    quiet, progress))
 			die(_("clone of '%s' into submodule path '%s' failed"),
 			    url, path);
@@ -1300,6 +1306,7 @@ struct submodule_update_clone {
 	int quiet;
 	int recommend_shallow;
 	struct string_list references;
+	int dissociate;
 	const char *depth;
 	const char *recursive_prefix;
 	const char *prefix;
@@ -1315,7 +1322,7 @@ struct submodule_update_clone {
 	int failed_clones_nr, failed_clones_alloc;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
-	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
+	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \
 	NULL, NULL, NULL, \
 	STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
 
@@ -1442,6 +1449,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		for_each_string_list_item(item, &suc->references)
 			argv_array_pushl(&child->args, "--reference", item->string, NULL);
 	}
+	if (suc->dissociate)
+		argv_array_push(&child->args, "--dissociate");
 	if (suc->depth)
 		argv_array_push(&child->args, suc->depth);
 
@@ -1575,6 +1584,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
 			   N_("rebase, merge, checkout or none")),
 		OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
 			   N_("reference repository")),
+		OPT_BOOL(0, "dissociate", &suc.dissociate,
+			   N_("use --reference only while cloning")),
 		OPT_STRING(0, "depth", &suc.depth, "<depth>",
 			   N_("Create a shallow clone truncated to the "
 			      "specified number of revisions")),
diff --git a/git-submodule.sh b/git-submodule.sh
index aa1c0bb67..4743b4745 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -42,6 +42,7 @@ prefix=
 custom_name=
 depth=
 progress=
+dissociate=
 
 die_if_unmatched ()
 {
@@ -128,6 +129,9 @@ cmd_add()
 		--reference=*)
 			reference_path="${1#--reference=}"
 			;;
+		--dissociate)
+			dissociate="--dissociate"
+			;;
 		--name)
 			case "$2" in '') usage ;; esac
 			custom_name=$2
@@ -258,7 +262,7 @@ or you are unsure what this means choose another name with the '--name' option."
 				eval_gettextln "Reactivating local git directory for submodule '\$sm_name'."
 			fi
 		fi
-		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
+		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"$dissociate"} ${depth:+"$depth"} || exit
 		(
 			sanitize_submodule_env
 			cd "$sm_path" &&
@@ -493,6 +497,9 @@ cmd_update()
 		--reference=*)
 			reference="$1"
 			;;
+		--dissociate)
+			dissociate="--dissociate"
+			;;
 		-m|--merge)
 			update="merge"
 			;;
@@ -550,6 +557,7 @@ cmd_update()
 		${prefix:+--recursive-prefix "$prefix"} \
 		${update:+--update "$update"} \
 		${reference:+"$reference"} \
+		${dissociate:+"$dissociate"} \
 		${depth:+--depth "$depth"} \
 		${recommend_shallow:+"$recommend_shallow"} \
 		${jobs:+$jobs} \
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index e159fc503..ee9772e0a 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -59,6 +59,16 @@ test_expect_success 'submodule add --reference uses alternates' '
 	test_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub
 '
 
+test_expect_success 'submodule add --reference with --dissociate doesnt use alternates' '
+	(
+		cd super &&
+		git submodule add --reference ../B --dissociate "file://$base_dir/A" sub-dissociate &&
+		git commit -m B-super-added &&
+		git repack -ad
+	) &&
+	test_must_fail test_alternate_is_used super/.git/modules/sub-dissociate/objects/info/alternates super/sub-dissociate
+'
+
 test_expect_success 'that reference gets used with add' '
 	(
 		cd super/sub &&
@@ -82,6 +92,13 @@ test_expect_success 'updating superproject keeps alternates' '
 	test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
 '
 
+test_expect_success 'updating superproject with --dissociate doesnt keep alternates' '
+	test_when_finished "rm -rf super-clone" &&
+	git clone super super-clone &&
+	git -C super-clone submodule update --init --reference ../B --dissociate &&
+	test_must_fail test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
+'
+
 test_expect_success 'submodules use alternates when cloning a superproject' '
 	test_when_finished "rm -rf super-clone" &&
 	git clone --reference super --recursive super super-clone &&
-- 
2.17.0.1.ge0414f29c.dirty

Re: [PATCH 0/2] Add --progress and --dissociate to git submodule

[Stefan Beller]

Hi Casey,

thanks for sending those patches and improving submodules!

On Tue, May 1, 2018 at 11:09 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
> These patches add --progress and --dissociate options to git submodule.
>
> The --progress option existed beforehand, but only for the update command and
> it was left undocumented.
>
> Both add and update submodule commands supported --reference, but not its pair
> option --dissociate which allows for independent clones rather than depending
> on the reference.

This makes sense.

>
> I apologize for any errors I may have made in creation of these patches or the
> formatting of these emails. This is the first time I have submitted patches to
> a mailing list.

The formatting seems to be alright, git-format-patch/git-send-email
just works. ;)

Stefan

Re: [PATCH 1/2] submodule: Add --progress option to add command

[Stefan Beller]

On Tue, May 1, 2018 at 11:09 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
> --progress was missing from add command, was only in update.
> Add --progress where it makes sense (both clone helper commands).
> Add missing documentation entry.
> Add test.

Maybe:
  The '--progress' was introduced in 72c5f88311d (clone: pass --progress
  decision to recursive submodules, 2016-09-22) to fix the progress reporting
  of the clone command. Also add the progress option to the 'submodule add'
  command. The update command already support the progress flag, but it
  is not documented.

> @@ -117,6 +117,9 @@ cmd_add()
>                 -q|--quiet)
>                         GIT_QUIET=1
>                         ;;
> +               --progress)
> +                       progress="--progress"
> +                       ;;

The code looks good, though unlike the other commands progress is a
boolean decision.

If we want to support --no-progress as well, we can do so by adding
    --no-progress)
        progress="--no-progress"
        ;;
and then the submodule helper ought to cope with it.


> +test_expect_success 'setup test repo' '
> +       mkdir parent &&
> +       (cd parent && git init &&
> +        echo one >file && git add file &&
> +        git commit -m one)
> +'

Coding style:
    (
        parens open on its own line, and its contents
        are indented by a tab.

Instead of coding this yourself, you could write the
test as:

    test_create_repo parent &&
    test_create_commit -C parent one

> +test_expect_success 'clone -o' '

What are we testing here? I do not quite see the connection to
testing --progress here.

> +       git clone -o foo parent clone-o &&
> +       (cd clone-o && git rev-parse --verify refs/remotes/foo/master)


> +test_expect_success 'redirected submodule add does not show progress' '
> +       (
> +               cd addtest &&



> +               git submodule add "file://$submodurl/parent" submod-redirected \
> +                       >out 2>err &&
> +               ! grep % err &&

We're grepping for percent to see that there is no progress. At first I thought
the percent sign might be a special character, had to research it to know it's
meant literally. TiL!

> +               test_i18ngrep ! "Checking connectivity" err

Makes sense.

> +       )

We could omit the extra shell by using

    git -C addtest submodule add "file://$... \
            >../out 2>../err &&

Why do we need 'out' ?

> +test_expect_success 'redirected submodule add --progress does show progress' '
> +       (
> +               cd addtest &&
> +               git submodule add --progress "file://$submodurl/parent" \
> +                       submod-redirected-progress >out 2>err && \
> +               grep % err
> +       )
> +'

Thanks for writing these tests!
Stefan

Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands

[Stefan Beller]

On Tue, May 1, 2018 at 11:09 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
>
> +test_expect_success 'submodule add --reference with --dissociate doesnt use alternates' '
> +       (
> +               cd super &&
> +               git submodule add --reference ../B --dissociate "file://$base_dir/A" sub-dissociate &&
> +               git commit -m B-super-added &&
> +               git repack -ad
> +       ) &&
> +       test_must_fail test_alternate_is_used super/.git/modules/sub-dissociate/objects/info/alternates super/sub-dissociate

We do already have the unfortunate precedent of using "test_must_fail
test_alternate_is_used"
(and it was partially included by me in 31224cbdc72 (clone: recursive
and reference option
triggers submodule alternates, 2016-08-17), further used in
bf03b790471 (submodule--helper:
set alternateLocation for cloned submodules, 2016-12-08).

I think it is the wrong approach though, as the test_must_fail only ensure that
*something* doesn't return success. Usually we use it with
"test_must_fail git ..."
to ensure we properly error out on user requests, that are impossible
to fulfill in
the current implementation.

For this test case I'd suggest we rather test that the no alternate is setup,

    test_path_is_missing
super/.git/modules/sub-dissociate/objects/info/alternates

as that tests the correctness of the --dissociate option?

Thanks,
Stefan

Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands

[Eric Sunshine]

On Tue, May 1, 2018 at 2:09 PM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
> submodule: Add --dissociate option to add/update commands

s/Add/add/

> Add --dissociate option to add and update commands, both clone helper commands
> that already have the --reference option --dissociate pairs with.
> Add documentation.
> Add tests.
>
> Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> @@ -1075,6 +1075,9 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
> +       if (dissociate) {
> +               argv_array_push(&cp.args, "--dissociate");
> +       }

Style: drop unnecessary braces

> @@ -1208,6 +1212,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
> +               OPT_BOOL(0, "dissociate", &dissociate,
> +                          N_("use --reference only while cloning")),

s/reference/dissociate/

> @@ -1575,6 +1584,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
> +               OPT_BOOL(0, "dissociate", &suc.dissociate,
> +                          N_("use --reference only while cloning")),

s/reference/dissociate/

> diff --git a/git-submodule.sh b/git-submodule.sh
> +               --dissociate)
> +                       dissociate="--dissociate"
> @@ -258,7 +262,7 @@ or you are unsure what this means choose another name with the '--name' option."
> -               git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
> +               git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"$dissociate"} ${depth:+"$depth"} || exit

I realize that you're just following existing practice in this script,
but it's a bit off-putting to see expansions for the new --progress
and --dissociate options being done via unnecessarily complex
${foobar:+"$foobar"} when the simpler $foobar would work just as well.

Just a comment; not necessarily a request for change. (A separate
preparatory cleanup patch which simplifies the existing complex
expansion expressions would be welcome but could also be considered
outside the scope of this patch series.)

> @@ -493,6 +497,9 @@ cmd_update()
> +               --dissociate)
> +                       dissociate="--dissociate"
> +                       ;;
> @@ -550,6 +557,7 @@ cmd_update()
>                 ${reference:+"$reference"} \
> +               ${dissociate:+"$dissociate"} \
>                 ${depth:+--depth "$depth"} \

Re: [PATCH 1/2] submodule: Add --progress option to add command

[Casey Fitzpatrick]

Thanks, I'll clean it up based on your comments. I based the tests
from t5606-clone-options.sh; I'm not sure why now but I thought I
needed that clone -o thing from there, but it appears useless.



On Tue, May 1, 2018 at 2:41 PM, Stefan Beller <sbeller@google.com> wrote:
> On Tue, May 1, 2018 at 11:09 AM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
>> --progress was missing from add command, was only in update.
>> Add --progress where it makes sense (both clone helper commands).
>> Add missing documentation entry.
>> Add test.
>
> Maybe:
>   The '--progress' was introduced in 72c5f88311d (clone: pass --progress
>   decision to recursive submodules, 2016-09-22) to fix the progress reporting
>   of the clone command. Also add the progress option to the 'submodule add'
>   command. The update command already support the progress flag, but it
>   is not documented.
>
>> @@ -117,6 +117,9 @@ cmd_add()
>>                 -q|--quiet)
>>                         GIT_QUIET=1
>>                         ;;
>> +               --progress)
>> +                       progress="--progress"
>> +                       ;;
>
> The code looks good, though unlike the other commands progress is a
> boolean decision.
>
> If we want to support --no-progress as well, we can do so by adding
>     --no-progress)
>         progress="--no-progress"
>         ;;
> and then the submodule helper ought to cope with it.
>
>
>> +test_expect_success 'setup test repo' '
>> +       mkdir parent &&
>> +       (cd parent && git init &&
>> +        echo one >file && git add file &&
>> +        git commit -m one)
>> +'
>
> Coding style:
>     (
>         parens open on its own line, and its contents
>         are indented by a tab.
>
> Instead of coding this yourself, you could write the
> test as:
>
>     test_create_repo parent &&
>     test_create_commit -C parent one
>
>> +test_expect_success 'clone -o' '
>
> What are we testing here? I do not quite see the connection to
> testing --progress here.
>
>> +       git clone -o foo parent clone-o &&
>> +       (cd clone-o && git rev-parse --verify refs/remotes/foo/master)
>
>
>> +test_expect_success 'redirected submodule add does not show progress' '
>> +       (
>> +               cd addtest &&
>
>
>
>> +               git submodule add "file://$submodurl/parent" submod-redirected \
>> +                       >out 2>err &&
>> +               ! grep % err &&
>
> We're grepping for percent to see that there is no progress. At first I thought
> the percent sign might be a special character, had to research it to know it's
> meant literally. TiL!
>
>> +               test_i18ngrep ! "Checking connectivity" err
>
> Makes sense.
>
>> +       )
>
> We could omit the extra shell by using
>
>     git -C addtest submodule add "file://$... \
>             >../out 2>../err &&
>
> Why do we need 'out' ?
>
>> +test_expect_success 'redirected submodule add --progress does show progress' '
>> +       (
>> +               cd addtest &&
>> +               git submodule add --progress "file://$submodurl/parent" \
>> +                       submod-redirected-progress >out 2>err && \
>> +               grep % err
>> +       )
>> +'
>
> Thanks for writing these tests!
> Stefan

Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands

[Casey Fitzpatrick]

Thanks, I will clean up the braces and commit message.

I have to disagree with the 's/reference/dissociate/' comments. It
appears this section of option descriptions mostly copies from the
descriptions given by 'git clone -h', which outputs:
    --reference <repo>    reference repository
    --reference-if-able <repo>
                          reference repository
    --dissociate          use --reference only while cloning
It is perhaps not the best description, but it means to say when
--dissociate is used --reference is only in play for reducing network
transfer, not keeping alternates.

Those expansions *are* certainly off-putting, they make a long line
even more difficult to parse. I just searched that file for ":+" for
those types of expressions and I think a patch to fix them would be
fairly short. I'll look into making that cleanup patch.



On Tue, May 1, 2018 at 4:25 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Tue, May 1, 2018 at 2:09 PM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
>> submodule: Add --dissociate option to add/update commands
>
> s/Add/add/
>
>> Add --dissociate option to add and update commands, both clone helper commands
>> that already have the --reference option --dissociate pairs with.
>> Add documentation.
>> Add tests.
>>
>> Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
>> ---
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> @@ -1075,6 +1075,9 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
>> +       if (dissociate) {
>> +               argv_array_push(&cp.args, "--dissociate");
>> +       }
>
> Style: drop unnecessary braces
>
>> @@ -1208,6 +1212,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>> +               OPT_BOOL(0, "dissociate", &dissociate,
>> +                          N_("use --reference only while cloning")),
>
> s/reference/dissociate/
>
>> @@ -1575,6 +1584,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
>> +               OPT_BOOL(0, "dissociate", &suc.dissociate,
>> +                          N_("use --reference only while cloning")),
>
> s/reference/dissociate/
>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> +               --dissociate)
>> +                       dissociate="--dissociate"
>> @@ -258,7 +262,7 @@ or you are unsure what this means choose another name with the '--name' option."
>> -               git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
>> +               git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"$dissociate"} ${depth:+"$depth"} || exit
>
> I realize that you're just following existing practice in this script,
> but it's a bit off-putting to see expansions for the new --progress
> and --dissociate options being done via unnecessarily complex
> ${foobar:+"$foobar"} when the simpler $foobar would work just as well.
>
> Just a comment; not necessarily a request for change. (A separate
> preparatory cleanup patch which simplifies the existing complex
> expansion expressions would be welcome but could also be considered
> outside the scope of this patch series.)
>
>> @@ -493,6 +497,9 @@ cmd_update()
>> +               --dissociate)
>> +                       dissociate="--dissociate"
>> +                       ;;
>> @@ -550,6 +557,7 @@ cmd_update()
>>                 ${reference:+"$reference"} \
>> +               ${dissociate:+"$dissociate"} \
>>                 ${depth:+--depth "$depth"} \

[PATCH 0/3] Add --progress and --dissociate to git submodule

[Casey Fitzpatrick]

These patches add --progress and --dissociate options to git submodule.

The --progress option existed beforehand, but only for the update command and
it was left undocumented.

Both add and update submodule commands supported --reference, but not its pair
option --dissociate which allows for independent clones rather than depending
on the reference.

This is a resubmission with comments from Stefan Beller and Eric Sunshine
addressed.

Casey Fitzpatrick (3):
  submodule: clean up subsititions in script
  submodule: add --progress option to add command
  submodule: add --dissociate option to add/update commands

 Documentation/git-submodule.txt | 17 ++++++++++++++++-
 builtin/submodule--helper.c     | 16 +++++++++++++---
 git-submodule.sh                | 21 ++++++++++++++++-----
 t/t7400-submodule-basic.sh      | 16 ++++++++++++++++
 t/t7408-submodule-reference.sh  | 17 +++++++++++++++++
 5 files changed, 78 insertions(+), 9 deletions(-)

-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 1/3] submodule: clean up subsititions in script

[Casey Fitzpatrick]

'recommend_shallow' and 'jobs' variables do not need quotes (they never contain
spaces) and do not require any additional prefix, therefore remove the
unnecessary subsitition.

'progress' is a boolean value. Treat it like the other boolean values in the
script by using a substitution.
---
 git-submodule.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 24914963c..262547968 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -465,7 +465,7 @@ cmd_update()
 			GIT_QUIET=1
 			;;
 		--progress)
-			progress="--progress"
+			progress=1
 			;;
 		-i|--init)
 			init=1
@@ -542,14 +542,14 @@ cmd_update()
 
 	{
 	git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
-		${progress:+"$progress"} \
+		${progress:+"--progress"} \
 		${wt_prefix:+--prefix "$wt_prefix"} \
 		${prefix:+--recursive-prefix "$prefix"} \
 		${update:+--update "$update"} \
 		${reference:+"$reference"} \
 		${depth:+--depth "$depth"} \
-		${recommend_shallow:+"$recommend_shallow"} \
-		${jobs:+$jobs} \
+		$recommend_shallow \
+		$jobs \
 		"$@" || echo "#unmatched" $?
 	} | {
 	err=
-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 2/3] submodule: add --progress option to add command

[Casey Fitzpatrick]

The '--progress' was introduced in 72c5f88311d (clone: pass --progress
decision to recursive submodules, 2016-09-22) to fix the progress reporting
of the clone command. Also add the progress option to the 'submodule add'
command. The update command already supports the progress flag, but it
is not documented.

Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
---
 Documentation/git-submodule.txt |  7 +++++++
 git-submodule.sh                |  5 ++++-
 t/t7400-submodule-basic.sh      | 16 ++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 71c5618e8..d1ebe32e8 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -239,6 +239,13 @@ OPTIONS
 --quiet::
 	Only print error messages.
 
+--progress::
+	This option is only valid for add and update commands.
+	Progress status is reported on the standard error stream
+	by default when it is attached to a terminal, unless -q
+	is specified. This flag forces progress status even if the
+	standard error stream is not directed to a terminal.
+
 --all::
 	This option is only valid for the deinit command. Unregister all
 	submodules in the working tree.
diff --git a/git-submodule.sh b/git-submodule.sh
index 262547968..39c241a1d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -117,6 +117,9 @@ cmd_add()
 		-q|--quiet)
 			GIT_QUIET=1
 			;;
+		--progress)
+			progress=1
+			;;
 		--reference)
 			case "$2" in '') usage ;; esac
 			reference_path=$2
@@ -255,7 +258,7 @@ or you are unsure what this means choose another name with the '--name' option."
 				eval_gettextln "Reactivating local git directory for submodule '\$sm_name'."
 			fi
 		fi
-		git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
+		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
 		(
 			sanitize_submodule_env
 			cd "$sm_path" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a39e69a3e..b5b4922ab 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -126,6 +126,22 @@ test_expect_success 'submodule add' '
 	test_cmp empty untracked
 '
 
+test_create_repo parent &&
+test_commit -C parent one
+
+test_expect_success 'redirected submodule add does not show progress' '
+	git -C addtest submodule add "file://$submodurl/parent" submod-redirected \
+		2>err &&
+	! grep % err &&
+	test_i18ngrep ! "Checking connectivity" err
+'
+
+test_expect_success 'redirected submodule add --progress does show progress' '
+	git -C addtest submodule add --progress "file://$submodurl/parent" \
+		submod-redirected-progress 2>err && \
+	grep % err
+'
+
 test_expect_success 'submodule add to .gitignored path fails' '
 	(
 		cd addtest-ignore &&
-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 3/3] submodule: add --dissociate option to add/update commands

[Casey Fitzpatrick]

Add --dissociate option to add and update commands, both clone helper commands
that already have the --reference option --dissociate pairs with.

Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
---
 Documentation/git-submodule.txt | 10 +++++++++-
 builtin/submodule--helper.c     | 16 +++++++++++++---
 git-submodule.sh                | 10 +++++++++-
 t/t7408-submodule-reference.sh  | 17 +++++++++++++++++
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d1ebe32e8..a75b95043 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -369,7 +369,15 @@ the submodule itself.
 	this option will be passed to the linkgit:git-clone[1] command.
 +
 *NOTE*: Do *not* use this option unless you have read the note
-for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
+for linkgit:git-clone[1]'s `--reference`, `--shared`, and `--dissociate`
+options carefully.
+
+--dissociate::
+	This option is only valid for add and update commands.  These
+	commands sometimes need to clone a remote repository. In this case,
+	this option will be passed to the linkgit:git-clone[1] command.
++
+*NOTE*: see the NOTE for the `--reference` option.
 
 --recursive::
 	This option is only valid for foreach, update, status and sync commands.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6ba8587b6..a85b30419 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1056,7 +1056,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
 }
 
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
-			   const char *depth, struct string_list *reference,
+			   const char *depth, struct string_list *reference, int dissociate,
 			   int quiet, int progress)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -1075,6 +1075,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
 			argv_array_pushl(&cp.args, "--reference",
 					 item->string, NULL);
 	}
+	if (dissociate)
+		argv_array_push(&cp.args, "--dissociate");
 	if (gitdir && *gitdir)
 		argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL);
 
@@ -1190,6 +1192,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	char *p, *path = NULL, *sm_gitdir;
 	struct strbuf sb = STRBUF_INIT;
 	struct string_list reference = STRING_LIST_INIT_NODUP;
+	int dissociate = 0;
 	char *sm_alternate = NULL, *error_strategy = NULL;
 
 	struct option module_clone_options[] = {
@@ -1208,6 +1211,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 		OPT_STRING_LIST(0, "reference", &reference,
 			   N_("repo"),
 			   N_("reference repository")),
+		OPT_BOOL(0, "dissociate", &dissociate,
+			   N_("use --reference only while cloning")),
 		OPT_STRING(0, "depth", &depth,
 			   N_("string"),
 			   N_("depth for shallow clones")),
@@ -1247,7 +1252,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 
 		prepare_possible_alternates(name, &reference);
 
-		if (clone_submodule(path, sm_gitdir, url, depth, &reference,
+		if (clone_submodule(path, sm_gitdir, url, depth, &reference, dissociate,
 				    quiet, progress))
 			die(_("clone of '%s' into submodule path '%s' failed"),
 			    url, path);
@@ -1300,6 +1305,7 @@ struct submodule_update_clone {
 	int quiet;
 	int recommend_shallow;
 	struct string_list references;
+	int dissociate;
 	const char *depth;
 	const char *recursive_prefix;
 	const char *prefix;
@@ -1315,7 +1321,7 @@ struct submodule_update_clone {
 	int failed_clones_nr, failed_clones_alloc;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
-	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
+	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \
 	NULL, NULL, NULL, \
 	STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
 
@@ -1442,6 +1448,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		for_each_string_list_item(item, &suc->references)
 			argv_array_pushl(&child->args, "--reference", item->string, NULL);
 	}
+	if (suc->dissociate)
+		argv_array_push(&child->args, "--dissociate");
 	if (suc->depth)
 		argv_array_push(&child->args, suc->depth);
 
@@ -1575,6 +1583,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
 			   N_("rebase, merge, checkout or none")),
 		OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
 			   N_("reference repository")),
+		OPT_BOOL(0, "dissociate", &suc.dissociate,
+			   N_("use --reference only while cloning")),
 		OPT_STRING(0, "depth", &suc.depth, "<depth>",
 			   N_("Create a shallow clone truncated to the "
 			      "specified number of revisions")),
diff --git a/git-submodule.sh b/git-submodule.sh
index 39c241a1d..1fddc098f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -42,6 +42,7 @@ prefix=
 custom_name=
 depth=
 progress=
+dissociate=
 
 die_if_unmatched ()
 {
@@ -128,6 +129,9 @@ cmd_add()
 		--reference=*)
 			reference_path="${1#--reference=}"
 			;;
+		--dissociate)
+			dissociate=1
+			;;
 		--name)
 			case "$2" in '') usage ;; esac
 			custom_name=$2
@@ -258,7 +262,7 @@ or you are unsure what this means choose another name with the '--name' option."
 				eval_gettextln "Reactivating local git directory for submodule '\$sm_name'."
 			fi
 		fi
-		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
+		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"--dissociate"} ${depth:+"$depth"} || exit
 		(
 			sanitize_submodule_env
 			cd "$sm_path" &&
@@ -493,6 +497,9 @@ cmd_update()
 		--reference=*)
 			reference="$1"
 			;;
+		--dissociate)
+			dissociate=1
+			;;
 		-m|--merge)
 			update="merge"
 			;;
@@ -550,6 +557,7 @@ cmd_update()
 		${prefix:+--recursive-prefix "$prefix"} \
 		${update:+--update "$update"} \
 		${reference:+"$reference"} \
+		${dissociate:+"--dissociate"} \
 		${depth:+--depth "$depth"} \
 		$recommend_shallow \
 		$jobs \
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index e159fc503..1b3e7c2f7 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -59,6 +59,16 @@ test_expect_success 'submodule add --reference uses alternates' '
 	test_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub
 '
 
+test_expect_success 'submodule add --reference with --dissociate doesnt use alternates' '
+	(
+		cd super &&
+		git submodule add --reference ../B --dissociate "file://$base_dir/A" sub-dissociate &&
+		git commit -m B-super-added &&
+		git repack -ad
+	) &&
+	test_path_is_missing super/.git/modules/sub-dissociate/objects/info/alternates
+'
+
 test_expect_success 'that reference gets used with add' '
 	(
 		cd super/sub &&
@@ -82,6 +92,13 @@ test_expect_success 'updating superproject keeps alternates' '
 	test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
 '
 
+test_expect_success 'updating superproject with --dissociate doesnt keep alternates' '
+	test_when_finished "rm -rf super-clone" &&
+	git clone super super-clone &&
+	git -C super-clone submodule update --init --reference ../B --dissociate &&
+	test_must_fail test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
+'
+
 test_expect_success 'submodules use alternates when cloning a superproject' '
 	test_when_finished "rm -rf super-clone" &&
 	git clone --reference super --recursive super super-clone &&
-- 
2.17.0.1.ge0414f29c.dirty

Re: [PATCH 3/3] submodule: add --dissociate option to add/update commands

[Casey Fitzpatrick]

Just noticed I missed the other 'test_must_fail'. Resubmitting in a few moments.

On Tue, May 1, 2018 at 8:27 PM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
> Add --dissociate option to add and update commands, both clone helper commands
> that already have the --reference option --dissociate pairs with.
>
> Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
> ---
>  Documentation/git-submodule.txt | 10 +++++++++-
>  builtin/submodule--helper.c     | 16 +++++++++++++---
>  git-submodule.sh                | 10 +++++++++-
>  t/t7408-submodule-reference.sh  | 17 +++++++++++++++++
>  4 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index d1ebe32e8..a75b95043 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -369,7 +369,15 @@ the submodule itself.
>         this option will be passed to the linkgit:git-clone[1] command.
>  +
>  *NOTE*: Do *not* use this option unless you have read the note
> -for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
> +for linkgit:git-clone[1]'s `--reference`, `--shared`, and `--dissociate`
> +options carefully.
> +
> +--dissociate::
> +       This option is only valid for add and update commands.  These
> +       commands sometimes need to clone a remote repository. In this case,
> +       this option will be passed to the linkgit:git-clone[1] command.
> ++
> +*NOTE*: see the NOTE for the `--reference` option.
>
>  --recursive::
>         This option is only valid for foreach, update, status and sync commands.
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 6ba8587b6..a85b30419 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1056,7 +1056,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
>  }
>
>  static int clone_submodule(const char *path, const char *gitdir, const char *url,
> -                          const char *depth, struct string_list *reference,
> +                          const char *depth, struct string_list *reference, int dissociate,
>                            int quiet, int progress)
>  {
>         struct child_process cp = CHILD_PROCESS_INIT;
> @@ -1075,6 +1075,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
>                         argv_array_pushl(&cp.args, "--reference",
>                                          item->string, NULL);
>         }
> +       if (dissociate)
> +               argv_array_push(&cp.args, "--dissociate");
>         if (gitdir && *gitdir)
>                 argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL);
>
> @@ -1190,6 +1192,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>         char *p, *path = NULL, *sm_gitdir;
>         struct strbuf sb = STRBUF_INIT;
>         struct string_list reference = STRING_LIST_INIT_NODUP;
> +       int dissociate = 0;
>         char *sm_alternate = NULL, *error_strategy = NULL;
>
>         struct option module_clone_options[] = {
> @@ -1208,6 +1211,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>                 OPT_STRING_LIST(0, "reference", &reference,
>                            N_("repo"),
>                            N_("reference repository")),
> +               OPT_BOOL(0, "dissociate", &dissociate,
> +                          N_("use --reference only while cloning")),
>                 OPT_STRING(0, "depth", &depth,
>                            N_("string"),
>                            N_("depth for shallow clones")),
> @@ -1247,7 +1252,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
>
>                 prepare_possible_alternates(name, &reference);
>
> -               if (clone_submodule(path, sm_gitdir, url, depth, &reference,
> +               if (clone_submodule(path, sm_gitdir, url, depth, &reference, dissociate,
>                                     quiet, progress))
>                         die(_("clone of '%s' into submodule path '%s' failed"),
>                             url, path);
> @@ -1300,6 +1305,7 @@ struct submodule_update_clone {
>         int quiet;
>         int recommend_shallow;
>         struct string_list references;
> +       int dissociate;
>         const char *depth;
>         const char *recursive_prefix;
>         const char *prefix;
> @@ -1315,7 +1321,7 @@ struct submodule_update_clone {
>         int failed_clones_nr, failed_clones_alloc;
>  };
>  #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
> -       SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
> +       SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \
>         NULL, NULL, NULL, \
>         STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
>
> @@ -1442,6 +1448,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>                 for_each_string_list_item(item, &suc->references)
>                         argv_array_pushl(&child->args, "--reference", item->string, NULL);
>         }
> +       if (suc->dissociate)
> +               argv_array_push(&child->args, "--dissociate");
>         if (suc->depth)
>                 argv_array_push(&child->args, suc->depth);
>
> @@ -1575,6 +1583,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
>                            N_("rebase, merge, checkout or none")),
>                 OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
>                            N_("reference repository")),
> +               OPT_BOOL(0, "dissociate", &suc.dissociate,
> +                          N_("use --reference only while cloning")),
>                 OPT_STRING(0, "depth", &suc.depth, "<depth>",
>                            N_("Create a shallow clone truncated to the "
>                               "specified number of revisions")),
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 39c241a1d..1fddc098f 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -42,6 +42,7 @@ prefix=
>  custom_name=
>  depth=
>  progress=
> +dissociate=
>
>  die_if_unmatched ()
>  {
> @@ -128,6 +129,9 @@ cmd_add()
>                 --reference=*)
>                         reference_path="${1#--reference=}"
>                         ;;
> +               --dissociate)
> +                       dissociate=1
> +                       ;;
>                 --name)
>                         case "$2" in '') usage ;; esac
>                         custom_name=$2
> @@ -258,7 +262,7 @@ or you are unsure what this means choose another name with the '--name' option."
>                                 eval_gettextln "Reactivating local git directory for submodule '\$sm_name'."
>                         fi
>                 fi
> -               git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
> +               git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"--dissociate"} ${depth:+"$depth"} || exit
>                 (
>                         sanitize_submodule_env
>                         cd "$sm_path" &&
> @@ -493,6 +497,9 @@ cmd_update()
>                 --reference=*)
>                         reference="$1"
>                         ;;
> +               --dissociate)
> +                       dissociate=1
> +                       ;;
>                 -m|--merge)
>                         update="merge"
>                         ;;
> @@ -550,6 +557,7 @@ cmd_update()
>                 ${prefix:+--recursive-prefix "$prefix"} \
>                 ${update:+--update "$update"} \
>                 ${reference:+"$reference"} \
> +               ${dissociate:+"--dissociate"} \
>                 ${depth:+--depth "$depth"} \
>                 $recommend_shallow \
>                 $jobs \
> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
> index e159fc503..1b3e7c2f7 100755
> --- a/t/t7408-submodule-reference.sh
> +++ b/t/t7408-submodule-reference.sh
> @@ -59,6 +59,16 @@ test_expect_success 'submodule add --reference uses alternates' '
>         test_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub
>  '
>
> +test_expect_success 'submodule add --reference with --dissociate doesnt use alternates' '
> +       (
> +               cd super &&
> +               git submodule add --reference ../B --dissociate "file://$base_dir/A" sub-dissociate &&
> +               git commit -m B-super-added &&
> +               git repack -ad
> +       ) &&
> +       test_path_is_missing super/.git/modules/sub-dissociate/objects/info/alternates
> +'
> +
>  test_expect_success 'that reference gets used with add' '
>         (
>                 cd super/sub &&
> @@ -82,6 +92,13 @@ test_expect_success 'updating superproject keeps alternates' '
>         test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
>  '
>
> +test_expect_success 'updating superproject with --dissociate doesnt keep alternates' '
> +       test_when_finished "rm -rf super-clone" &&
> +       git clone super super-clone &&
> +       git -C super-clone submodule update --init --reference ../B --dissociate &&
> +       test_must_fail test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
> +'
> +
>  test_expect_success 'submodules use alternates when cloning a superproject' '
>         test_when_finished "rm -rf super-clone" &&
>         git clone --reference super --recursive super super-clone &&
> --
> 2.17.0.1.ge0414f29c.dirty
>

[PATCH 0/3] Add --progress and --dissociate to git submodule

[Casey Fitzpatrick]

These patches add --progress and --dissociate options to git submodule.

The --progress option existed beforehand, but only for the update command and
it was left undocumented.

Both add and update submodule commands supported --reference, but not its pair
option --dissociate which allows for independent clones rather than depending
on the reference.

This is a resubmission with comments from Stefan Beller and Eric Sunshine
addressed.

Casey Fitzpatrick (3):
  submodule: clean up subsititions in script
  submodule: add --progress option to add command
  submodule: add --dissociate option to add/update commands

 Documentation/git-submodule.txt | 17 ++++++++++++++++-
 builtin/submodule--helper.c     | 16 +++++++++++++---
 git-submodule.sh                | 21 ++++++++++++++++-----
 t/t7400-submodule-basic.sh      | 16 ++++++++++++++++
 t/t7408-submodule-reference.sh  | 17 +++++++++++++++++
 5 files changed, 78 insertions(+), 9 deletions(-)

-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 1/3] submodule: clean up subsititions in script

[Casey Fitzpatrick]

'recommend_shallow' and 'jobs' variables do not need quotes (they never contain
spaces) and do not require any additional prefix, therefore remove the
unnecessary subsitition.

'progress' is a boolean value. Treat it like the other boolean values in the
script by using a substitution.

Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
---
 git-submodule.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 24914963c..262547968 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -465,7 +465,7 @@ cmd_update()
 			GIT_QUIET=1
 			;;
 		--progress)
-			progress="--progress"
+			progress=1
 			;;
 		-i|--init)
 			init=1
@@ -542,14 +542,14 @@ cmd_update()
 
 	{
 	git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
-		${progress:+"$progress"} \
+		${progress:+"--progress"} \
 		${wt_prefix:+--prefix "$wt_prefix"} \
 		${prefix:+--recursive-prefix "$prefix"} \
 		${update:+--update "$update"} \
 		${reference:+"$reference"} \
 		${depth:+--depth "$depth"} \
-		${recommend_shallow:+"$recommend_shallow"} \
-		${jobs:+$jobs} \
+		$recommend_shallow \
+		$jobs \
 		"$@" || echo "#unmatched" $?
 	} | {
 	err=
-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 2/3] submodule: add --progress option to add command

[Casey Fitzpatrick]

The '--progress' was introduced in 72c5f88311d (clone: pass --progress
decision to recursive submodules, 2016-09-22) to fix the progress reporting
of the clone command. Also add the progress option to the 'submodule add'
command. The update command already supports the progress flag, but it
is not documented.

Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
---
 Documentation/git-submodule.txt |  7 +++++++
 git-submodule.sh                |  5 ++++-
 t/t7400-submodule-basic.sh      | 16 ++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 71c5618e8..d1ebe32e8 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -239,6 +239,13 @@ OPTIONS
 --quiet::
 	Only print error messages.
 
+--progress::
+	This option is only valid for add and update commands.
+	Progress status is reported on the standard error stream
+	by default when it is attached to a terminal, unless -q
+	is specified. This flag forces progress status even if the
+	standard error stream is not directed to a terminal.
+
 --all::
 	This option is only valid for the deinit command. Unregister all
 	submodules in the working tree.
diff --git a/git-submodule.sh b/git-submodule.sh
index 262547968..39c241a1d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -117,6 +117,9 @@ cmd_add()
 		-q|--quiet)
 			GIT_QUIET=1
 			;;
+		--progress)
+			progress=1
+			;;
 		--reference)
 			case "$2" in '') usage ;; esac
 			reference_path=$2
@@ -255,7 +258,7 @@ or you are unsure what this means choose another name with the '--name' option."
 				eval_gettextln "Reactivating local git directory for submodule '\$sm_name'."
 			fi
 		fi
-		git submodule--helper clone ${GIT_QUIET:+--quiet} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
+		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
 		(
 			sanitize_submodule_env
 			cd "$sm_path" &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a39e69a3e..b5b4922ab 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -126,6 +126,22 @@ test_expect_success 'submodule add' '
 	test_cmp empty untracked
 '
 
+test_create_repo parent &&
+test_commit -C parent one
+
+test_expect_success 'redirected submodule add does not show progress' '
+	git -C addtest submodule add "file://$submodurl/parent" submod-redirected \
+		2>err &&
+	! grep % err &&
+	test_i18ngrep ! "Checking connectivity" err
+'
+
+test_expect_success 'redirected submodule add --progress does show progress' '
+	git -C addtest submodule add --progress "file://$submodurl/parent" \
+		submod-redirected-progress 2>err && \
+	grep % err
+'
+
 test_expect_success 'submodule add to .gitignored path fails' '
 	(
 		cd addtest-ignore &&
-- 
2.17.0.1.ge0414f29c.dirty

[PATCH 3/3] submodule: add --dissociate option to add/update commands

[Casey Fitzpatrick]

Add --dissociate option to add and update commands, both clone helper commands
that already have the --reference option --dissociate pairs with.

Signed-off-by: Casey Fitzpatrick <kcghost@gmail.com>
---
 Documentation/git-submodule.txt | 10 +++++++++-
 builtin/submodule--helper.c     | 16 +++++++++++++---
 git-submodule.sh                | 10 +++++++++-
 t/t7408-submodule-reference.sh  | 17 +++++++++++++++++
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d1ebe32e8..a75b95043 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -369,7 +369,15 @@ the submodule itself.
 	this option will be passed to the linkgit:git-clone[1] command.
 +
 *NOTE*: Do *not* use this option unless you have read the note
-for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.
+for linkgit:git-clone[1]'s `--reference`, `--shared`, and `--dissociate`
+options carefully.
+
+--dissociate::
+	This option is only valid for add and update commands.  These
+	commands sometimes need to clone a remote repository. In this case,
+	this option will be passed to the linkgit:git-clone[1] command.
++
+*NOTE*: see the NOTE for the `--reference` option.
 
 --recursive::
 	This option is only valid for foreach, update, status and sync commands.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6ba8587b6..a85b30419 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1056,7 +1056,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
 }
 
 static int clone_submodule(const char *path, const char *gitdir, const char *url,
-			   const char *depth, struct string_list *reference,
+			   const char *depth, struct string_list *reference, int dissociate,
 			   int quiet, int progress)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -1075,6 +1075,8 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
 			argv_array_pushl(&cp.args, "--reference",
 					 item->string, NULL);
 	}
+	if (dissociate)
+		argv_array_push(&cp.args, "--dissociate");
 	if (gitdir && *gitdir)
 		argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL);
 
@@ -1190,6 +1192,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 	char *p, *path = NULL, *sm_gitdir;
 	struct strbuf sb = STRBUF_INIT;
 	struct string_list reference = STRING_LIST_INIT_NODUP;
+	int dissociate = 0;
 	char *sm_alternate = NULL, *error_strategy = NULL;
 
 	struct option module_clone_options[] = {
@@ -1208,6 +1211,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 		OPT_STRING_LIST(0, "reference", &reference,
 			   N_("repo"),
 			   N_("reference repository")),
+		OPT_BOOL(0, "dissociate", &dissociate,
+			   N_("use --reference only while cloning")),
 		OPT_STRING(0, "depth", &depth,
 			   N_("string"),
 			   N_("depth for shallow clones")),
@@ -1247,7 +1252,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
 
 		prepare_possible_alternates(name, &reference);
 
-		if (clone_submodule(path, sm_gitdir, url, depth, &reference,
+		if (clone_submodule(path, sm_gitdir, url, depth, &reference, dissociate,
 				    quiet, progress))
 			die(_("clone of '%s' into submodule path '%s' failed"),
 			    url, path);
@@ -1300,6 +1305,7 @@ struct submodule_update_clone {
 	int quiet;
 	int recommend_shallow;
 	struct string_list references;
+	int dissociate;
 	const char *depth;
 	const char *recursive_prefix;
 	const char *prefix;
@@ -1315,7 +1321,7 @@ struct submodule_update_clone {
 	int failed_clones_nr, failed_clones_alloc;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
-	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, \
+	SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \
 	NULL, NULL, NULL, \
 	STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
 
@@ -1442,6 +1448,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		for_each_string_list_item(item, &suc->references)
 			argv_array_pushl(&child->args, "--reference", item->string, NULL);
 	}
+	if (suc->dissociate)
+		argv_array_push(&child->args, "--dissociate");
 	if (suc->depth)
 		argv_array_push(&child->args, suc->depth);
 
@@ -1575,6 +1583,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
 			   N_("rebase, merge, checkout or none")),
 		OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
 			   N_("reference repository")),
+		OPT_BOOL(0, "dissociate", &suc.dissociate,
+			   N_("use --reference only while cloning")),
 		OPT_STRING(0, "depth", &suc.depth, "<depth>",
 			   N_("Create a shallow clone truncated to the "
 			      "specified number of revisions")),
diff --git a/git-submodule.sh b/git-submodule.sh
index 39c241a1d..1fddc098f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -42,6 +42,7 @@ prefix=
 custom_name=
 depth=
 progress=
+dissociate=
 
 die_if_unmatched ()
 {
@@ -128,6 +129,9 @@ cmd_add()
 		--reference=*)
 			reference_path="${1#--reference=}"
 			;;
+		--dissociate)
+			dissociate=1
+			;;
 		--name)
 			case "$2" in '') usage ;; esac
 			custom_name=$2
@@ -258,7 +262,7 @@ or you are unsure what this means choose another name with the '--name' option."
 				eval_gettextln "Reactivating local git directory for submodule '\$sm_name'."
 			fi
 		fi
-		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
+		git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"--progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"--dissociate"} ${depth:+"$depth"} || exit
 		(
 			sanitize_submodule_env
 			cd "$sm_path" &&
@@ -493,6 +497,9 @@ cmd_update()
 		--reference=*)
 			reference="$1"
 			;;
+		--dissociate)
+			dissociate=1
+			;;
 		-m|--merge)
 			update="merge"
 			;;
@@ -550,6 +557,7 @@ cmd_update()
 		${prefix:+--recursive-prefix "$prefix"} \
 		${update:+--update "$update"} \
 		${reference:+"$reference"} \
+		${dissociate:+"--dissociate"} \
 		${depth:+--depth "$depth"} \
 		$recommend_shallow \
 		$jobs \
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index e159fc503..82656747f 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -59,6 +59,16 @@ test_expect_success 'submodule add --reference uses alternates' '
 	test_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub
 '
 
+test_expect_success 'submodule add --reference with --dissociate doesnt use alternates' '
+	(
+		cd super &&
+		git submodule add --reference ../B --dissociate "file://$base_dir/A" sub-dissociate &&
+		git commit -m B-super-added &&
+		git repack -ad
+	) &&
+	test_path_is_missing super/.git/modules/sub-dissociate/objects/info/alternates
+'
+
 test_expect_success 'that reference gets used with add' '
 	(
 		cd super/sub &&
@@ -82,6 +92,13 @@ test_expect_success 'updating superproject keeps alternates' '
 	test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
 '
 
+test_expect_success 'updating superproject with --dissociate doesnt keep alternates' '
+	test_when_finished "rm -rf super-clone" &&
+	git clone super super-clone &&
+	git -C super-clone submodule update --init --reference ../B --dissociate &&
+	test_path_is_missing super-clone/.git/modules/sub/objects/info/alternates
+'
+
 test_expect_success 'submodules use alternates when cloning a superproject' '
 	test_when_finished "rm -rf super-clone" &&
 	git clone --reference super --recursive super super-clone &&
-- 
2.17.0.1.ge0414f29c.dirty

Re: git-submodule is missing --dissociate option

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Mon, Apr 30, 2018 at 1:30 PM, Casey Fitzpatrick <kcghost@gmail.com> wrote:
>> It also seems to be missing "--progress", and I imagine others.
>> Perhaps submodule add/update should be reworked to automatically
>> accept all the options that clone would?
>
> --progress is not missing, but I see that it isn't documented. It was
> added in 72c5f88311 ("clone: pass --progress decision to recursive
> submodules", 2016-09-22). What you're suggesting makes sense, but as
> shown in that commit it's not easy for it to happen automatically,
> there's a lot of boilerplate involved.
>
> But since you're interested you can see how to add new options with
> that patch, it should be easy for anyone not experienced with the
> codebase, it's all just boilerplate + adding a test.

I think it is going in the right direction overall, but a few corner
cases may need special attention.  "add" may be adding a new module
that locally originates, in which case "clone" is not relevant.
Similarly, for "update"options for "clone" are not relevant unless
it is the very initial one.

Re: git-submodule is missing --dissociate option

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

>> As far as I am aware this can be worked around with 'git repack -a'
>> and manual removal of the objects/info/alternates file afterward.
>> Though I don't know if this results in a less speedy clone than
>> dissociate would.
>
> That is an interesting workaround!

It's not just "workaround", but actually is an implementation of
that exact option, no?

Re: [PATCH 0/3] Add --progress and --dissociate to git submodule

[Junio C Hamano]

Casey Fitzpatrick <kcghost@gmail.com> writes:

> These patches add --progress and --dissociate options to git submodule.
>
> The --progress option existed beforehand, but only for the update command and
> it was left undocumented.
>
> Both add and update submodule commands supported --reference, but not its pair
> option --dissociate which allows for independent clones rather than depending
> on the reference.
>
> This is a resubmission with comments from Stefan Beller and Eric Sunshine
> addressed.

Readers would really appreciate it if these are prepared with
format-patch with -v$N option.  Unless they read faster than you
post patches, they will find a few messages identically titled, all
unread in their mailbox, and it is not always easy to tell which
ones are the latest.

Thanks.

Re: [PATCH 1/3] submodule: clean up subsititions in script

[Junio C Hamano]

Casey Fitzpatrick <kcghost@gmail.com> writes:

> 'recommend_shallow' and 'jobs' variables do not need quotes (they never contain
> spaces) and do not require any additional prefix, therefore remove the
> unnecessary subsitition.

The resulting patch is good, but "they never contain spaces" is not
a very good rationale.  The real reason is that (1) we use them only
to hold a single token value (or leave them empty) in the current
code, and (2) if the feature they represent is enhanced in the
future to make them multi-token options (e.g. we may allow $jobs to
contain, in addition to "--jobs=2", "--jobs 2" for whatever reason
later), it is likely that we would want these multi-tokens split at
$IFS (e.g. "--jobs" and "2" get passed as separate option, not a
single "--jobs 2" string).

> 'progress' is a boolean value. Treat it like the other boolean values in the
> script by using a substitution.

This is OK.

>  	git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
> -		${progress:+"$progress"} \
> +		${progress:+"--progress"} \
>  		${wt_prefix:+--prefix "$wt_prefix"} \
>  		${prefix:+--recursive-prefix "$prefix"} \
>  		${update:+--update "$update"} \
>  		${reference:+"$reference"} \
>  		${depth:+--depth "$depth"} \
> -		${recommend_shallow:+"$recommend_shallow"} \
> -		${jobs:+$jobs} \
> +		$recommend_shallow \
> +		$jobs \

Re: [PATCH 0/3] Add --progress and --dissociate to git submodule

[Casey Fitzpatrick]

Thanks I was not aware of that option.

On Wed, May 2, 2018 at 12:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Casey Fitzpatrick <kcghost@gmail.com> writes:
>
>> These patches add --progress and --dissociate options to git submodule.
>>
>> The --progress option existed beforehand, but only for the update command and
>> it was left undocumented.
>>
>> Both add and update submodule commands supported --reference, but not its pair
>> option --dissociate which allows for independent clones rather than depending
>> on the reference.
>>
>> This is a resubmission with comments from Stefan Beller and Eric Sunshine
>> addressed.
>
> Readers would really appreciate it if these are prepared with
> format-patch with -v$N option.  Unless they read faster than you
> post patches, they will find a few messages identically titled, all
> unread in their mailbox, and it is not always easy to tell which
> ones are the latest.
>
> Thanks.

Re: [PATCH 1/3] submodule: clean up subsititions in script

[Casey Fitzpatrick]

Thanks I will amend and re-submit (this time with  -v$N, I apologize
for creating a confusing mess in everyone's email clients :))

On Wed, May 2, 2018 at 1:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Casey Fitzpatrick <kcghost@gmail.com> writes:
>
>> 'recommend_shallow' and 'jobs' variables do not need quotes (they never contain
>> spaces) and do not require any additional prefix, therefore remove the
>> unnecessary subsitition.
>
> The resulting patch is good, but "they never contain spaces" is not
> a very good rationale.  The real reason is that (1) we use them only
> to hold a single token value (or leave them empty) in the current
> code, and (2) if the feature they represent is enhanced in the
> future to make them multi-token options (e.g. we may allow $jobs to
> contain, in addition to "--jobs=2", "--jobs 2" for whatever reason
> later), it is likely that we would want these multi-tokens split at
> $IFS (e.g. "--jobs" and "2" get passed as separate option, not a
> single "--jobs 2" string).
>
>> 'progress' is a boolean value. Treat it like the other boolean values in the
>> script by using a substitution.
>
> This is OK.
>
>>       git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
>> -             ${progress:+"$progress"} \
>> +             ${progress:+"--progress"} \
>>               ${wt_prefix:+--prefix "$wt_prefix"} \
>>               ${prefix:+--recursive-prefix "$prefix"} \
>>               ${update:+--update "$update"} \
>>               ${reference:+"$reference"} \
>>               ${depth:+--depth "$depth"} \
>> -             ${recommend_shallow:+"$recommend_shallow"} \
>> -             ${jobs:+$jobs} \
>> +             $recommend_shallow \
>> +             $jobs \
>