* git-submodule is missing --dissociate option @ 2018-04-30 8:29 Casey Fitzpatrick 2018-04-30 11:30 ` Casey Fitzpatrick 2018-04-30 18:19 ` Stefan Beller 0 siblings, 2 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-04-30 8:29 UTC (permalink / raw) To: git 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: git-submodule is missing --dissociate option 2018-04-30 8:29 git-submodule is missing --dissociate option Casey Fitzpatrick @ 2018-04-30 11:30 ` Casey Fitzpatrick 2018-04-30 13:16 ` Ævar Arnfjörð Bjarmason 2018-04-30 18:19 ` Stefan Beller 1 sibling, 1 reply; 29+ messages in thread From: Casey Fitzpatrick @ 2018-04-30 11:30 UTC (permalink / raw) To: git 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: git-submodule is missing --dissociate option 2018-04-30 11:30 ` Casey Fitzpatrick @ 2018-04-30 13:16 ` Ævar Arnfjörð Bjarmason 2018-05-02 4:32 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-30 13:16 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: Git Mailing List 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: git-submodule is missing --dissociate option 2018-04-30 13:16 ` Ævar Arnfjörð Bjarmason @ 2018-05-02 4:32 ` Junio C Hamano 0 siblings, 0 replies; 29+ messages in thread From: Junio C Hamano @ 2018-05-02 4:32 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Casey Fitzpatrick, Git Mailing List Æ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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: git-submodule is missing --dissociate option 2018-04-30 8:29 git-submodule is missing --dissociate option Casey Fitzpatrick 2018-04-30 11:30 ` Casey Fitzpatrick @ 2018-04-30 18:19 ` Stefan Beller 2018-04-30 21:39 ` Casey Fitzpatrick 2018-05-02 4:34 ` git-submodule is missing --dissociate option Junio C Hamano 1 sibling, 2 replies; 29+ messages in thread From: Stefan Beller @ 2018-04-30 18:19 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: git-submodule is missing --dissociate option 2018-04-30 18:19 ` Stefan Beller @ 2018-04-30 21:39 ` Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Casey Fitzpatrick ` (2 more replies) 2018-05-02 4:34 ` git-submodule is missing --dissociate option Junio C Hamano 1 sibling, 3 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-04-30 21:39 UTC (permalink / raw) To: Stefan Beller, avarab; +Cc: git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/2] Add --progress and --dissociate to git submodule 2018-04-30 21:39 ` Casey Fitzpatrick @ 2018-05-01 18:09 ` Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 1/2] submodule: Add --progress option to add command Casey Fitzpatrick ` (2 more replies) 2018-05-02 0:27 ` [PATCH 0/3] " Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2 siblings, 3 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-01 18:09 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/2] submodule: Add --progress option to add command 2018-05-01 18:09 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Casey Fitzpatrick @ 2018-05-01 18:09 ` Casey Fitzpatrick 2018-05-01 18:41 ` Stefan Beller 2018-05-01 18:09 ` [PATCH 2/2] submodule: Add --dissociate option to add/update commands Casey Fitzpatrick 2018-05-01 18:20 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Stefan Beller 2 siblings, 1 reply; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-01 18:09 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] submodule: Add --progress option to add command 2018-05-01 18:09 ` [PATCH 1/2] submodule: Add --progress option to add command Casey Fitzpatrick @ 2018-05-01 18:41 ` Stefan Beller 2018-05-01 20:48 ` Casey Fitzpatrick 0 siblings, 1 reply; 29+ messages in thread From: Stefan Beller @ 2018-05-01 18:41 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] submodule: Add --progress option to add command 2018-05-01 18:41 ` Stefan Beller @ 2018-05-01 20:48 ` Casey Fitzpatrick 0 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-01 20:48 UTC (permalink / raw) To: Stefan Beller; +Cc: git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/2] submodule: Add --dissociate option to add/update commands 2018-05-01 18:09 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 1/2] submodule: Add --progress option to add command Casey Fitzpatrick @ 2018-05-01 18:09 ` Casey Fitzpatrick 2018-05-01 20:23 ` Stefan Beller 2018-05-01 20:25 ` Eric Sunshine 2018-05-01 18:20 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Stefan Beller 2 siblings, 2 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-01 18:09 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands 2018-05-01 18:09 ` [PATCH 2/2] submodule: Add --dissociate option to add/update commands Casey Fitzpatrick @ 2018-05-01 20:23 ` Stefan Beller 2018-05-01 20:25 ` Eric Sunshine 1 sibling, 0 replies; 29+ messages in thread From: Stefan Beller @ 2018-05-01 20:23 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands 2018-05-01 18:09 ` [PATCH 2/2] submodule: Add --dissociate option to add/update commands Casey Fitzpatrick 2018-05-01 20:23 ` Stefan Beller @ 2018-05-01 20:25 ` Eric Sunshine 2018-05-01 21:21 ` Casey Fitzpatrick 1 sibling, 1 reply; 29+ messages in thread From: Eric Sunshine @ 2018-05-01 20:25 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: Git List 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"} \ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands 2018-05-01 20:25 ` Eric Sunshine @ 2018-05-01 21:21 ` Casey Fitzpatrick 0 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-01 21:21 UTC (permalink / raw) To: Eric Sunshine; +Cc: Git List 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"} \ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/2] Add --progress and --dissociate to git submodule 2018-05-01 18:09 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 1/2] submodule: Add --progress option to add command Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 2/2] submodule: Add --dissociate option to add/update commands Casey Fitzpatrick @ 2018-05-01 18:20 ` Stefan Beller 2 siblings, 0 replies; 29+ messages in thread From: Stefan Beller @ 2018-05-01 18:20 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: git 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/3] Add --progress and --dissociate to git submodule 2018-04-30 21:39 ` Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Casey Fitzpatrick @ 2018-05-02 0:27 ` Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick ` (2 more replies) 2018-05-02 0:55 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2 siblings, 3 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:27 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] submodule: clean up subsititions in script 2018-05-02 0:27 ` [PATCH 0/3] " Casey Fitzpatrick @ 2018-05-02 0:27 ` Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 2/3] submodule: add --progress option to add command Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 3/3] submodule: add --dissociate option to add/update commands Casey Fitzpatrick 2 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:27 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 2/3] submodule: add --progress option to add command 2018-05-02 0:27 ` [PATCH 0/3] " Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick @ 2018-05-02 0:27 ` Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 3/3] submodule: add --dissociate option to add/update commands Casey Fitzpatrick 2 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:27 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/3] submodule: add --dissociate option to add/update commands 2018-05-02 0:27 ` [PATCH 0/3] " Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 2/3] submodule: add --progress option to add command Casey Fitzpatrick @ 2018-05-02 0:27 ` Casey Fitzpatrick 2018-05-02 0:40 ` Casey Fitzpatrick 2 siblings, 1 reply; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:27 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 3/3] submodule: add --dissociate option to add/update commands 2018-05-02 0:27 ` [PATCH 3/3] submodule: add --dissociate option to add/update commands Casey Fitzpatrick @ 2018-05-02 0:40 ` Casey Fitzpatrick 0 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:40 UTC (permalink / raw) To: git, Stefan Beller, Eric Sunshine; +Cc: 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 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/3] Add --progress and --dissociate to git submodule 2018-04-30 21:39 ` Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 0/3] " Casey Fitzpatrick @ 2018-05-02 0:55 ` Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick ` (3 more replies) 2 siblings, 4 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:55 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] submodule: clean up subsititions in script 2018-05-02 0:55 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Casey Fitzpatrick @ 2018-05-02 0:55 ` Casey Fitzpatrick 2018-05-02 5:59 ` Junio C Hamano 2018-05-02 0:55 ` [PATCH 2/3] submodule: add --progress option to add command Casey Fitzpatrick ` (2 subsequent siblings) 3 siblings, 1 reply; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:55 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/3] submodule: clean up subsititions in script 2018-05-02 0:55 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick @ 2018-05-02 5:59 ` Junio C Hamano 2018-05-03 10:46 ` Casey Fitzpatrick 0 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2018-05-02 5:59 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: git, sbeller, sunshine 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 \ ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/3] submodule: clean up subsititions in script 2018-05-02 5:59 ` Junio C Hamano @ 2018-05-03 10:46 ` Casey Fitzpatrick 0 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-03 10:46 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Stefan Beller, Eric Sunshine 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 \ > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/3] submodule: add --progress option to add command 2018-05-02 0:55 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick @ 2018-05-02 0:55 ` Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 3/3] submodule: add --dissociate option to add/update commands Casey Fitzpatrick 2018-05-02 4:37 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Junio C Hamano 3 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:55 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 3/3] submodule: add --dissociate option to add/update commands 2018-05-02 0:55 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 2/3] submodule: add --progress option to add command Casey Fitzpatrick @ 2018-05-02 0:55 ` Casey Fitzpatrick 2018-05-02 4:37 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Junio C Hamano 3 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 0:55 UTC (permalink / raw) To: git, sbeller, sunshine; +Cc: 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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] Add --progress and --dissociate to git submodule 2018-05-02 0:55 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Casey Fitzpatrick ` (2 preceding siblings ...) 2018-05-02 0:55 ` [PATCH 3/3] submodule: add --dissociate option to add/update commands Casey Fitzpatrick @ 2018-05-02 4:37 ` Junio C Hamano 2018-05-02 8:54 ` Casey Fitzpatrick 3 siblings, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2018-05-02 4:37 UTC (permalink / raw) To: Casey Fitzpatrick; +Cc: git, sbeller, sunshine 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] Add --progress and --dissociate to git submodule 2018-05-02 4:37 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Junio C Hamano @ 2018-05-02 8:54 ` Casey Fitzpatrick 0 siblings, 0 replies; 29+ messages in thread From: Casey Fitzpatrick @ 2018-05-02 8:54 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Stefan Beller, Eric Sunshine 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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: git-submodule is missing --dissociate option 2018-04-30 18:19 ` Stefan Beller 2018-04-30 21:39 ` Casey Fitzpatrick @ 2018-05-02 4:34 ` Junio C Hamano 1 sibling, 0 replies; 29+ messages in thread From: Junio C Hamano @ 2018-05-02 4:34 UTC (permalink / raw) To: Stefan Beller; +Cc: Casey Fitzpatrick, git 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? ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2018-05-03 10:46 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-04-30 8:29 git-submodule is missing --dissociate option Casey Fitzpatrick 2018-04-30 11:30 ` Casey Fitzpatrick 2018-04-30 13:16 ` Ævar Arnfjörð Bjarmason 2018-05-02 4:32 ` Junio C Hamano 2018-04-30 18:19 ` Stefan Beller 2018-04-30 21:39 ` Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 1/2] submodule: Add --progress option to add command Casey Fitzpatrick 2018-05-01 18:41 ` Stefan Beller 2018-05-01 20:48 ` Casey Fitzpatrick 2018-05-01 18:09 ` [PATCH 2/2] submodule: Add --dissociate option to add/update commands Casey Fitzpatrick 2018-05-01 20:23 ` Stefan Beller 2018-05-01 20:25 ` Eric Sunshine 2018-05-01 21:21 ` Casey Fitzpatrick 2018-05-01 18:20 ` [PATCH 0/2] Add --progress and --dissociate to git submodule Stefan Beller 2018-05-02 0:27 ` [PATCH 0/3] " Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 2/3] submodule: add --progress option to add command Casey Fitzpatrick 2018-05-02 0:27 ` [PATCH 3/3] submodule: add --dissociate option to add/update commands Casey Fitzpatrick 2018-05-02 0:40 ` Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 1/3] submodule: clean up subsititions in script Casey Fitzpatrick 2018-05-02 5:59 ` Junio C Hamano 2018-05-03 10:46 ` Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 2/3] submodule: add --progress option to add command Casey Fitzpatrick 2018-05-02 0:55 ` [PATCH 3/3] submodule: add --dissociate option to add/update commands Casey Fitzpatrick 2018-05-02 4:37 ` [PATCH 0/3] Add --progress and --dissociate to git submodule Junio C Hamano 2018-05-02 8:54 ` Casey Fitzpatrick 2018-05-02 4:34 ` git-submodule is missing --dissociate option Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).