Re: [PATCH v4] submodule: add 'deinit' command
Am 12.03.2013 17:22, schrieb Junio C Hamano: Phil Hord phil.h...@gmail.com writes: I think this would be clearer if 'git deinit' said rm 'submodule/*' or maybe Removed workdir for 'submodule' Is it just me? The latter may probably be better. Hmm, it doesn't really remove the directory but only empties it (it recreates it a few lines after removing it together with its contents). So what about Cleared directory 'submodule' The attached interdiff suppresses the rm 'submodule' message and issues the Cleared ... message after it successfully removed the work tree. (But please note that it also prints this message even if the submodule work tree is already empty, e.g. when you deinit a submodule the second time) 8- diff --git a/git-submodule.sh b/git-submodule.sh index 204bc78..d003e8a 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -601,10 +601,12 @@ cmd_deinit() if test -z $force then - git rm -n $sm_path || + git rm -qn $sm_path || die $(eval_gettext Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them) fi - rm -rf $sm_path || say $(eval_gettext Could not remove submodule work tree '\$sm_path') + rm -rf $sm_path + say $(eval_gettext Cleared directory '\$sm_path') || + say $(eval_gettext Could not remove submodule work tree '\$sm_path') fi mkdir $sm_path || say $(eval_gettext Could not create empty submodule directory '\$sm_path') -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] submodule: add 'deinit' command
Jens Lehmann jens.lehm...@web.de writes: Am 12.03.2013 17:22, schrieb Junio C Hamano: Phil Hord phil.h...@gmail.com writes: I think this would be clearer if 'git deinit' said rm 'submodule/*' or maybe Removed workdir for 'submodule' Is it just me? The latter may probably be better. Hmm, it doesn't really remove the directory but only empties it (it recreates it a few lines after removing it together with its contents). So what about Cleared directory 'submodule' Sounds the cleanest among the suggested phrasing so far. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] submodule: add 'deinit' command
On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann jens.lehm...@web.de wrote: With git submodule init the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to git submodule update. But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the submodule.$name.url setting from .git/config together with the work tree himself). Help those users by providing a 'deinit' command. This removes the whole submodule.name section from .git/config either for the given submodule(s) or for all those which have been initialized if '.' is given. Fail if the current work tree contains modifications unless forced. Complain when for a submodule given on the command line the url setting can't be found in .git/config, but nonetheless don't fail. Add tests and link the man pages of git submodule deinit and git rm to assist the user in deciding whether removing or unregistering the submodule is the right thing to do for him. Signed-off-by: Jens Lehmann jens.lehm...@web.de Probably because I was new to this command, I was confused by this output. $ git submodule deinit submodule rm 'submodule' Submodule 'submodule' (gerrit:foo/submodule) unregistered for path 'submodule' $ git rm submodule rm 'submodule' This line is confusing to me in the deinit command: rm 'submodule' It doesn't mean what git usually means when it says this to me. See how the 'git rm' command says the same thing but means something different in the next command. In the deinit case, git removes the workdir contents of 'submodule' and it reports rm 'submodule'. In the rm case, git removes the submodule link from the tree and rmdirs the empty 'submodule' directory. I think this would be clearer if 'git deinit' said rm 'submodule/*' or maybe Removed workdir for 'submodule' Is it just me? Phil -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] submodule: add 'deinit' command
Phil Hord phil.h...@gmail.com writes: I think this would be clearer if 'git deinit' said rm 'submodule/*' or maybe Removed workdir for 'submodule' Is it just me? The latter may probably be better. After cloning the superproject, you show interest in individual submodules by saying git submodule init that submodule and until then your .git/config in the superproject does not indicate that you are interested in that submodule, and you won't have a submodule checkout. deinit is a way to revert back to that original state; recording that you are no longer interested in the submodule is the primary effect, and removal of its checkout is a mere side effect of it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] submodule: add 'deinit' command
Am 12.02.2013 18:11, schrieb Phil Hord: On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann jens.lehm...@web.de wrote: + die_if_unmatched $mode + name=$(module_name $sm_path) || exit + url=$(git config submodule.$name.url) + if test -z $url + then + say $(eval_gettext No url found for submodule path '\$sm_path' in .git/config) Is it safe to shelter the user a little bit more from the git internals here and say instead: Submodule '\$sm_path' is not initialized. Yeah, that makes much more sense. But I'd rather use the name too: Submodule '\$name' is not initialized for path '\$sm_path' Also, I think this code will show this message for each submodule on 'git submodule deinit .' But I think I would prefer to suppress it in that case. If I have not explicitly stated which submodules to deinit, then I do not think git should complain that some of them are not initialized. Yes, that message gets printed for each uninitialized submodule. We could easily suppress that for '.', but it would be really hard to get that right for other wildcards like 'foo*'. (And e.g. running a submodule update also lists all submodules it skips because of an update=none setting, so I'm not sure if it's that important here) But if people really want that, I'd suppress that for the '.' case. Further opinions? + continue + fi + + # Remove the submodule work tree (unless the user already did it) + if test -d $sm_path + then + # Protect submodules containing a .git directory + if test -d $sm_path/.git + then + echo 2 $(eval_gettext Submodule work tree $sm_path contains a .git directory) + die $(eval_gettext (use 'rm -rf' if you really want to remove it including all of its history)) I expect this is the right thing to do for now. But I wonder if we can also move $sm_path/.git to $GIT_DIR/modules/$sm_path in this case (though I think I am not spelling this path correctly). Would that be ok? What extra work is needed to relocate the .git dir like this? Hmm, I'm a bit torn about automagically moving the repo somewhere else. While I think it is a sane solution for most users I suspect some users might hate us for doing that without asking (and with no option to turn that off). What about adding a separate git submodule to-gitfile command which does that and hinting that here? However I do have the feeling that this should be done in another commit. + die $(eval_gettext Submodule work tree $sm_path contains local modifications, use '-f' to discard them) Nit, grammar: use a semicolon instead of a comma. Ok. And while looking at it I noticed that $sm_path should be '\$sm_path' here, same a few lines above ... sigh +test_expect_success 'set up a second submodule' ' + git submodule add ./init2 example2 + git commit -m submodle example2 added Nit: submodule is misspelled Thanks. Junio, this looks like a we have v5 as soon as we decide what to do with the not initialized messages when '.' is used, right? My current diff to v4 looks like this: -8- diff --git a/git-submodule.sh b/git-submodule.sh index f1b552f..27f8e12 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -591,7 +591,7 @@ cmd_deinit() url=$(git config submodule.$name.url) if test -z $url then - say $(eval_gettext No url found for submodule path '\$sm_path' in .git/config) + say $(eval_gettext Submodule '\$name' is not initialized for path '\$sm_path') continue fi @@ -601,14 +601,14 @@ cmd_deinit() # Protect submodules containing a .git directory if test -d $sm_path/.git then - echo 2 $(eval_gettext Submodule work tree $sm_path contains a .git directory) + echo 2 $(eval_gettext Submodule work tree '\$sm_path' contains a .git directory) die $(eval_gettext (use 'rm -rf' if you really want to remove it including all of its history)) fi if test -z $force then git rm -n $sm_path || - die $(eval_gettext Submodule work tree $sm_path contains local modifications, use '-f' to discard them) + die $(eval_gettext Submodule work tree '\$sm_path' contains local modifications; use '-f' to discard them) fi rm -rf $sm_path || say $(eval_gettext Could not remove
Re: [PATCH v4] submodule: add 'deinit' command
Jens Lehmann jens.lehm...@web.de writes: Junio, this looks like a we have v5 as soon as we decide what to do with the not initialized messages when '.' is used, right? OK. I myself do not deeply care if we end up special casing . or not; I'll leave it up to you and other submodule folks. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] submodule: add 'deinit' command
I haven't tried it yet, but I have some comments. On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann jens.lehm...@web.de wrote: With git submodule init the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to git submodule update. But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the submodule.$name.url setting from .git/config together with the work tree himself). Help those users by providing a 'deinit' command. This removes the whole submodule.name section from .git/config either for the given submodule(s) or for all those which have been initialized if '.' is given. Fail if the current work tree contains modifications unless forced. Complain when for a submodule given on the command line the url setting can't be found in .git/config, but nonetheless don't fail. Add tests and link the man pages of git submodule deinit and git rm to assist the user in deciding whether removing or unregistering the submodule is the right thing to do for him. Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Changes since v3: - Add deinit to the --force documentation of git submodule - Never remove submodules containing a .git dir, even when forced - Diagnostic output when rm -rf or mkdir fails - More test cases Documentation/git-rm.txt| 4 ++ Documentation/git-submodule.txt | 18 +++- git-submodule.sh| 78 ++- t/t7400-submodule-basic.sh | 100 4 files changed, 198 insertions(+), 2 deletions(-) diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt index 92bac27..1d876c2 100644 --- a/Documentation/git-rm.txt +++ b/Documentation/git-rm.txt @@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree. Ignored files are deemed expendable and won't stop a submodule's work tree from being removed. +If you only want to remove the local checkout of a submodule from your +work tree without committing the removal, +use linkgit:git-submodule[1] `deinit` instead. + EXAMPLES `git rm Documentation/\*.txt`:: diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index a0c9df8..bc06159 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -13,6 +13,7 @@ SYNOPSIS [--reference repository] [--] repository [path] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...] 'git submodule' [--quiet] init [--] [path...] +'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [--rebase] [--reference repository] [--merge] [--recursive] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] @@ -134,6 +135,19 @@ init:: the explicit 'init' step if you do not intend to customize any submodule locations. +deinit:: + Unregister the given submodules, i.e. remove the whole + `submodule.$name` section from .git/config together with their work + tree. Further calls to `git submodule update`, `git submodule foreach` + and `git submodule sync` will skip any unregistered submodules until + they are initialized again, so use this command if you don't want to + have a local checkout of the submodule in your work tree anymore. If + you really want to remove a submodule from the repository and commit + that use linkgit:git-rm[1] instead. ++ +If `--force` is specified, the submodule's work tree will be removed even if +it contains local modifications. + update:: Update the registered submodules, i.e. clone missing submodules and checkout the commit specified in the index of the containing repository. @@ -213,8 +227,10 @@ OPTIONS -f:: --force:: - This option is only valid for add and update commands. + This option is only valid for add, deinit and update commands. When running add, allow adding an otherwise ignored submodule path. + When running deinit the submodule work trees will be removed even if + they contain local changes. When running update, throw away local changes in submodules when switching to a different commit; and always run a checkout operation in the submodule, even if the commit listed in the index of the diff --git a/git-submodule.sh b/git-submodule.sh index 004c034..f1b552f 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -8,6 +8,7 @@ dashless=$(basename $0 | sed -e 's/-/ /') USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference repository] [--] repository [path] or: $dashless [--quiet] status
Re: [PATCH v4] submodule: add 'deinit' command
Phil Hord phil.h...@gmail.com writes: + if test $# = 0 + then + die $(eval_gettext Use '.' if you really want to deinitialize all submodules) + fi + + module_list $@ | + while read mode sha1 stage sm_path + do + die_if_unmatched $mode + name=$(module_name $sm_path) || exit + url=$(git config submodule.$name.url) + if test -z $url + then + say $(eval_gettext No url found for submodule path '\$sm_path' in .git/config) Is it safe to shelter the user a little bit more from the git internals here and say instead: Submodule '\$sm_path' is not initialized. Sounds like a sensible suggestion. Also, I think this code will show this message for each submodule on 'git submodule deinit .' But I think I would prefer to suppress it in that case. If I have not explicitly stated which submodules to deinit,... But isn't it the way to explicitly say everything under the sun? After all, what does the message say to git submodule deinit? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] submodule: add 'deinit' command
With git submodule init the user is able to tell git he cares about one or more submodules and wants to have it populated on the next call to git submodule update. But currently there is no easy way he could tell git he does not care about a submodule anymore and wants to get rid of his local work tree (except he knows a lot about submodule internals and removes the submodule.$name.url setting from .git/config together with the work tree himself). Help those users by providing a 'deinit' command. This removes the whole submodule.name section from .git/config either for the given submodule(s) or for all those which have been initialized if '.' is given. Fail if the current work tree contains modifications unless forced. Complain when for a submodule given on the command line the url setting can't be found in .git/config, but nonetheless don't fail. Add tests and link the man pages of git submodule deinit and git rm to assist the user in deciding whether removing or unregistering the submodule is the right thing to do for him. Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Changes since v3: - Add deinit to the --force documentation of git submodule - Never remove submodules containing a .git dir, even when forced - Diagnostic output when rm -rf or mkdir fails - More test cases Documentation/git-rm.txt| 4 ++ Documentation/git-submodule.txt | 18 +++- git-submodule.sh| 78 ++- t/t7400-submodule-basic.sh | 100 4 files changed, 198 insertions(+), 2 deletions(-) diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt index 92bac27..1d876c2 100644 --- a/Documentation/git-rm.txt +++ b/Documentation/git-rm.txt @@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules work tree. Ignored files are deemed expendable and won't stop a submodule's work tree from being removed. +If you only want to remove the local checkout of a submodule from your +work tree without committing the removal, +use linkgit:git-submodule[1] `deinit` instead. + EXAMPLES `git rm Documentation/\*.txt`:: diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index a0c9df8..bc06159 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -13,6 +13,7 @@ SYNOPSIS [--reference repository] [--] repository [path] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...] 'git submodule' [--quiet] init [--] [path...] +'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [--rebase] [--reference repository] [--merge] [--recursive] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] @@ -134,6 +135,19 @@ init:: the explicit 'init' step if you do not intend to customize any submodule locations. +deinit:: + Unregister the given submodules, i.e. remove the whole + `submodule.$name` section from .git/config together with their work + tree. Further calls to `git submodule update`, `git submodule foreach` + and `git submodule sync` will skip any unregistered submodules until + they are initialized again, so use this command if you don't want to + have a local checkout of the submodule in your work tree anymore. If + you really want to remove a submodule from the repository and commit + that use linkgit:git-rm[1] instead. ++ +If `--force` is specified, the submodule's work tree will be removed even if +it contains local modifications. + update:: Update the registered submodules, i.e. clone missing submodules and checkout the commit specified in the index of the containing repository. @@ -213,8 +227,10 @@ OPTIONS -f:: --force:: - This option is only valid for add and update commands. + This option is only valid for add, deinit and update commands. When running add, allow adding an otherwise ignored submodule path. + When running deinit the submodule work trees will be removed even if + they contain local changes. When running update, throw away local changes in submodules when switching to a different commit; and always run a checkout operation in the submodule, even if the commit listed in the index of the diff --git a/git-submodule.sh b/git-submodule.sh index 004c034..f1b552f 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -8,6 +8,7 @@ dashless=$(basename $0 | sed -e 's/-/ /') USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference repository] [--] repository [path] or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...] or: $dashless [--quiet] init [--] [path...] + or: $dashless [--quiet] deinit [-f|--force] [--] path... or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch]
