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

[Stefan Beller]

On Tue, May 3, 2016 at 3:52 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> So eventually (i.e. after the submodule groups lands)
>> "submodule deinit ." will start acting weird again?
>
> It would be nice if it never acts in a weird way, but that is all
> future development, not related to this fix, no?

Yes, just this patch is fine. Though lacking documentation.
I'll send an update.
--
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] submodule deinit: require '--all' instead of '.' for all submodules

[Junio C Hamano]

Stefan Beller  writes:

> So eventually (i.e. after the submodule groups lands)
> "submodule deinit ." will start acting weird again?

It would be nice if it never acts in a weird way, but that is all
future development, not related to this fix, no?
--
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] submodule deinit: require '--all' instead of '.' for all submodules

[Stefan Beller]

On Tue, May 3, 2016 at 3:28 PM, Junio C Hamano  wrote:
> Stefan Beller  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?

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?
--
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] submodule deinit: require '--all' instead of '.' for all submodules

[Junio C Hamano]

Stefan Beller  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?
--
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