On Tue, May 3, 2016 at 10:21 AM, Junio C Hamano <gits...@pobox.com> wrote: > Stefan Beller <sbel...@google.com> writes: > >> The discussion in [1] realized that '.' is a faulty suggestion as >> there is a corner case where it fails: >> >>> "submodule deinit ." may have "worked" in the sense that you would >>> have at least one path in your tree and avoided this "nothing >>> matches" most of the time. It would have still failed with the >>> exactly same error if run in an empty repository, i.e. >>> >>> $ E=/var/tmp/x/empty && rm -fr "$E" && mkdir -p "$E" && cd "$E" >>> $ git init >>> $ rungit v2.6.6 submodule deinit . >>> error: pathspec '.' did not match any file(s) known to git. >>> Did you forget to 'git add'? >>> $ >file && git add file >>> $ rungit v2.6.6 submodule deinit . >>> $ echo $? >>> 0 >> >> Allow no argument for `submodule deinit` to mean all submodules >> and add a test to check for the corner case of an empty repository. >> >> There is no need to update the documentation as it did not describe the >> special case '.' to remove all submodules. > > OK, and the reason why there is no need to update the actual code, > other than the "do not allow" gate, is because "submodule--helper > list" aka module_list already knows to list everything if no > pathspec is given. Am I reading the code (not in the patch) > correctly?
You are correct. > >> [1] http://news.gmane.org/gmane.comp.version-control.git/289535 > > The old discussion thread raises a good point. The refusal to > accept no-pathspec form for a potentially destructive "deinit" may > have been a safety measure, even though the suggested way to tell > the command "Yes, I know I want to deinit everything" was not a > good one (i.e. it resulted in an error if your project did not have > any files tracked yet). > > So possible ways forward may be > > - to remove the safety altogether; or > - keep the safety, but give a better suggestion to say "Yes, deinit > everything". > > And this patch decides to take the former approach? Yes. > > I am wondering if this can be solved in a cleaner way to teach > "deinit" take a new "--all" option instead, e.g. something like... > > diff --git a/git-submodule.sh b/git-submodule.sh > index 82e95a9..4b84116 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -405,6 +405,7 @@ cmd_init() > cmd_deinit() > { > # parse $args after "submodule ... deinit". > + deinit_all= > while test $# -ne 0 > do > case "$1" in > @@ -414,6 +415,9 @@ cmd_deinit() > -q|--quiet) > GIT_QUIET=1 > ;; > + -a|--all) > + deinit_all=t > + ;; > --) > shift > break > @@ -428,9 +432,9 @@ cmd_deinit() > shift > done > > - if test $# = 0 > + if test $# = 0 && test -z "$deinit_all" > then > - die "$(eval_gettext "Use '.' if you really want to > deinitialize all submodules")" > + die "$(eval_gettext "Use '--all' if you really want to > deinitialize all submodules")" > fi > > git submodule--helper list --prefix "$wt_prefix" "$@" | > > > That would work even in the pathological "empty directory that has > nothing to match even '.'" case without losing the safety, no? It would work for the case git submodule deinit --all # as you would expect. git submodule deinit --all COPYIN* # would break git submodule deinit --all . # may break git submodule deinit --all path/to/some/submodules/ # would be unclear to me So maybe we want to add a check that no pathspec arguments are given when --all is given? -- 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
