Re: [PATCH] submodule deinit: require '--all' instead of '.' for all submodules

2 messages, 2 authors, 2016-06-16 · open the first message on its own page

Re: [PATCH] submodule deinit: require '--all' instead of '.' for all submodules

From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:09

Stefan Beller [off-list ref] writes:
I developed it on top of 
    "submodule deinit test: fix broken && chain in subshell"
    on top of 2a4c8c36a7f6, 2016-03-24, Merge branch
    'sb/submodule-module-list-pathspec-fix'
but I think this would rather go in as a new feature, not on top
of a bugfix topic, so I think this could go on origin/master ?
I do not particularly view it as a new feature.  The way the old
message suggested did not work in a pathological corner case, but we
wanted to keep the "force user to be explicit when doing mass
destruction", and a fix we happened to chose requires addition of a
new option--that would still look like a fix to me.

It is not like we are forbidding the use of "submodule deinit ."
that used to work in a tree with at least one tracked path.  With
the change, a script that has such a command will continue to work,
no?

Re: [PATCH] submodule deinit: require '--all' instead of '.' for all submodules

From: Stefan Beller <hidden>
Date: 2016-06-16 02:19:09

On Tue, May 3, 2016 at 3:28 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
I developed it on top of
    "submodule deinit test: fix broken && chain in subshell"
    on top of 2a4c8c36a7f6, 2016-03-24, Merge branch
    'sb/submodule-module-list-pathspec-fix'
but I think this would rather go in as a new feature, not on top
of a bugfix topic, so I think this could go on origin/master ?
I do not particularly view it as a new feature.  The way the old
message suggested did not work in a pathological corner case, but we
wanted to keep the "force user to be explicit when doing mass
destruction", and a fix we happened to chose requires addition of a
new option--that would still look like a fix to me.

It is not like we are forbidding the use of "submodule deinit ."
that used to work in a tree with at least one tracked path.  With
the change, a script that has such a command will continue to work,
no?
Maybe.

With just this patch, yes.

I'd like to revert submodule-module-list-pathspec-fix partially
when redoing the groups support. That would break the '.' script
case. So eventually scripts want to use

    git submodule deinit -f --all

instead of

    git submodule deinit -f .

When implementing the groups support, I'd change module_list
in a way that you can give names, paths, or labels to it. In case of
a user gives 'COPYIN*' we'd want to know if that is a path (or a name,
label) or bogus, so I think we'd tighten the checks there just for the
functionality not just performance as originally anticipated for the
order of S_ISGITLINK and match_pathspec.

So eventually (i.e. after the submodule groups lands)
"submodule deinit ." will start acting weird again?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help