git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Git submodule remove
@ 2021-10-21 17:25 Kalyan Sriram
  2021-10-21 21:36 ` brian m. carlson
  0 siblings, 1 reply; 15+ messages in thread
From: Kalyan Sriram @ 2021-10-21 17:25 UTC (permalink / raw)
  To: git

Hello,

I was curious why git-submodule does not have an `rm` command. Currently
I have to manually delete it from .gitmodules, .git/config,
.git/modules/, etc. See [0].

I'd like to contribute my first patch to this project by adding this
feature, but wanted to check first with the community if there's any
particular reason it was chosen not to not be implemented, or if it's
simply that nobody has gotten around to it - it seems to be a relatively
common feature someone might want.

Anyway, please let me know if this is something that would be accepted,
or if anyone has any comments or suggestions.

Thanks!
Kalyan

[0] https://git.wiki.kernel.org/index.php/GitSubmoduleTutorial#Removal

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-21 17:25 Git submodule remove Kalyan Sriram
@ 2021-10-21 21:36 ` brian m. carlson
  2021-10-21 22:47   ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: brian m. carlson @ 2021-10-21 21:36 UTC (permalink / raw)
  To: Kalyan Sriram; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
> Hello,
> 
> I was curious why git-submodule does not have an `rm` command. Currently
> I have to manually delete it from .gitmodules, .git/config,
> .git/modules/, etc. See [0].
> 
> I'd like to contribute my first patch to this project by adding this
> feature, but wanted to check first with the community if there's any
> particular reason it was chosen not to not be implemented, or if it's
> simply that nobody has gotten around to it - it seems to be a relatively
> common feature someone might want.
> 
> Anyway, please let me know if this is something that would be accepted,
> or if anyone has any comments or suggestions.

I think the reason it hasn't been implemented is that nobody's gotten
around to it yet.  I certainly would find this useful and have wanted
the same thing myself, so I can't see a reason why the right series
wouldn't be accepted.

If someone else knows of a good reason why we haven't done this, I'm all
ears.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-21 21:36 ` brian m. carlson
@ 2021-10-21 22:47   ` Junio C Hamano
  2021-10-21 23:25     ` Matheus Tavares
  2021-10-22 13:47     ` Randall S. Becker
  0 siblings, 2 replies; 15+ messages in thread
From: Junio C Hamano @ 2021-10-21 22:47 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Kalyan Sriram, git

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
>> Hello,
>> 
>> I was curious why git-submodule does not have an `rm` command. Currently
>> I have to manually delete it from .gitmodules, .git/config,
>> .git/modules/, etc. See [0].
>> 
>> I'd like to contribute my first patch to this project by adding this
>> feature, but wanted to check first with the community if there's any
>> particular reason it was chosen not to not be implemented, or if it's
>> simply that nobody has gotten around to it - it seems to be a relatively
>> common feature someone might want.
>> 
>> Anyway, please let me know if this is something that would be accepted,
>> or if anyone has any comments or suggestions.
>
> I think the reason it hasn't been implemented is that nobody's gotten
> around to it yet.  I certainly would find this useful and have wanted
> the same thing myself, so I can't see a reason why the right series
> wouldn't be accepted.

I tend to agree that nobody felt the need strongly enough.  Code
tends to accumulate without ever getting removed, and removal of a
file, removal of a directory, or removal of a submodule is a much
rarer event compared to other changes people would need to make.
Adding such a feature would have been much more work for those who
faced such a rare occasion to want to use it than just doing it by
hand and committing the result.

I'd imagine that the happy-case implementation should be fairly
straight-forward.  You would:

 - ensure that the submodule is "absorbed" already;

 - run "git rm -f" the submodule to remove the gitlink from the index
   and remove the directory from the working tree; and

 - remove the .gitmodules entry for the submodule.

and you'd leave the final "record the state of the index as a
commit" to the user, simply because the user would want to have
other changes related to the removal of the submodule in the same
commit (like changes to files in the superproject that refer to the
submodule contents or removal of other submodules).

The hard part is unhappy-cases.  There are too many things that can
go wrong and you need to handle all the error cases correctly so
that you do not leave the user's repository in an uncontrollably
messy state.

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-21 22:47   ` Junio C Hamano
@ 2021-10-21 23:25     ` Matheus Tavares
  2021-10-21 23:35       ` Junio C Hamano
  2021-10-22 13:47     ` Randall S. Becker
  1 sibling, 1 reply; 15+ messages in thread
From: Matheus Tavares @ 2021-10-21 23:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, Kalyan Sriram, git

On Thu, Oct 21, 2021 at 7:47 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
> >> Hello,
> >>
> >> I was curious why git-submodule does not have an `rm` command. Currently
> >> I have to manually delete it from .gitmodules, .git/config,
> >> .git/modules/, etc. See [0].
> >>
[...]
> I'd imagine that the happy-case implementation should be fairly
> straight-forward.  You would:
>
>  - ensure that the submodule is "absorbed" already;
>
>  - run "git rm -f" the submodule to remove the gitlink from the index
>    and remove the directory from the working tree; and
>
>  - remove the .gitmodules entry for the submodule.

I think "git rm <submodule>" already does these three steps, doesn't it?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-21 23:25     ` Matheus Tavares
@ 2021-10-21 23:35       ` Junio C Hamano
  2021-10-22  3:32         ` Kalyan Sriram
  2021-10-22 12:12         ` Ævar Arnfjörð Bjarmason
  0 siblings, 2 replies; 15+ messages in thread
From: Junio C Hamano @ 2021-10-21 23:35 UTC (permalink / raw)
  To: Matheus Tavares; +Cc: brian m. carlson, Kalyan Sriram, git

Matheus Tavares <matheus.bernardino@usp.br> writes:

> On Thu, Oct 21, 2021 at 7:47 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>>
>> > On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
>> >> Hello,
>> >>
>> >> I was curious why git-submodule does not have an `rm` command. Currently
>> >> I have to manually delete it from .gitmodules, .git/config,
>> >> .git/modules/, etc. See [0].
>> >>
> [...]
>> I'd imagine that the happy-case implementation should be fairly
>> straight-forward.  You would:
>>
>>  - ensure that the submodule is "absorbed" already;
>>
>>  - run "git rm -f" the submodule to remove the gitlink from the index
>>    and remove the directory from the working tree; and
>>
>>  - remove the .gitmodules entry for the submodule.
>
> I think "git rm <submodule>" already does these three steps, doesn't it?

Wow, that is a unnerving layering violation, but I suspect it is too
late to fix it. So perhaps "git submodule rm" would just become a
synonym for "git rm"?

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-21 23:35       ` Junio C Hamano
@ 2021-10-22  3:32         ` Kalyan Sriram
  2021-10-22 22:02           ` Junio C Hamano
  2021-10-22 12:12         ` Ævar Arnfjörð Bjarmason
  1 sibling, 1 reply; 15+ messages in thread
From: Kalyan Sriram @ 2021-10-22  3:32 UTC (permalink / raw)
  To: Junio C Hamano, Matheus Tavares; +Cc: brian m. carlson, git

On Thu Oct 21, 2021 at 4:35 PM PDT, Junio C Hamano wrote:
> Matheus Tavares <matheus.bernardino@usp.br> writes:
>
> > On Thu, Oct 21, 2021 at 7:47 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> >>
> >> > On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
> >> >> Hello,
> >> >>
> >> >> I was curious why git-submodule does not have an `rm` command. Currently
> >> >> I have to manually delete it from .gitmodules, .git/config,
> >> >> .git/modules/, etc. See [0].
> >> >>
> > [...]
> >> I'd imagine that the happy-case implementation should be fairly
> >> straight-forward.  You would:
> >>
> >>  - ensure that the submodule is "absorbed" already;
> >>
> >>  - run "git rm -f" the submodule to remove the gitlink from the index
> >>    and remove the directory from the working tree; and
> >>
> >>  - remove the .gitmodules entry for the submodule.
> >
> > I think "git rm <submodule>" already does these three steps, doesn't it?

Wow, this is a surprise. Looks like this behavior is mentioned in the
man page but is not very well known.
> So perhaps "git submodule rm" would just become a synonym for "git rm"?
>
> Thanks.
Almost, yeah, but not quite. I ran a couple quick tests to clarify my own
understanding of how submodules work, and found that:

"git rm" deletes the submodule directory completely and modifies
.gitmodules, effectively removing the submodule. However, it leaves the
entry in .git/config dangling, which is annoying.

"git submodule deinit" (which I didn't know existed until I just read the man
page) deletes all contents of submodule directory, but leaves the 
(empty) submodule directory itself intact. It DOES delete the entry in
.git/config, but leaves a dangling entry in .gitmodules, so the next
"git submodule update --init --recursive" registers and populates the
submodule again.

>
> Wow, that is a unnerving layering violation, but I suspect it is too
> late to fix it. 
I agree, it's probably too late to change the behavior of "git rm"
regarding how it treats submodules. That said, I think the cleanest
solution would be to update "git rm" to also remove the dangling
.git/config entry. Then, I could make "git submodule rm" a synonym for
"git rm" for clarity. This would probably also involve updating
documentation in multiple locations to make sure everything is
consistent, and verifying "git rm" doesn't leave anything else about the
submodule dangling. Please let me know if anyone has a better idea for
anything I listed.

Thanks for your input so far, Junio, Matheus, and Brian! If no one has
any objections, I'll figure out how to make these updates and send a
patch along.

Thanks!
Kalyan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-21 23:35       ` Junio C Hamano
  2021-10-22  3:32         ` Kalyan Sriram
@ 2021-10-22 12:12         ` Ævar Arnfjörð Bjarmason
  2021-10-22 21:32           ` Junio C Hamano
  1 sibling, 1 reply; 15+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-22 12:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Matheus Tavares, brian m. carlson, Kalyan Sriram, git,
	Emily Shaffer


On Thu, Oct 21 2021, Junio C Hamano wrote:

> Matheus Tavares <matheus.bernardino@usp.br> writes:
>
>> On Thu, Oct 21, 2021 at 7:47 PM Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>>>
>>> > On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
>>> >> Hello,
>>> >>
>>> >> I was curious why git-submodule does not have an `rm` command. Currently
>>> >> I have to manually delete it from .gitmodules, .git/config,
>>> >> .git/modules/, etc. See [0].
>>> >>
>> [...]
>>> I'd imagine that the happy-case implementation should be fairly
>>> straight-forward.  You would:
>>>
>>>  - ensure that the submodule is "absorbed" already;
>>>
>>>  - run "git rm -f" the submodule to remove the gitlink from the index
>>>    and remove the directory from the working tree; and
>>>
>>>  - remove the .gitmodules entry for the submodule.
>>
>> I think "git rm <submodule>" already does these three steps, doesn't it?
>
> Wow, that is a unnerving layering violation, but I suspect it is too
> late to fix it. So perhaps "git submodule rm" would just become a
> synonym for "git rm"?
>
> Thanks.

Why would it be a good thing to change it even if we could? We added
that section with "git submodule add", it makes sense to have it removed
on "git init". The user could have added it manually, but the same goes
for manually added config that "git remote remove" would remove.

If anything I think it would make sense to extend this behavior. E.g. if
we "git rm" a <path> and notice that the <path> was the only thing that
matched a given entry in .gitignore we should remove that entry.

Or e.g. warn if we notice that the non-ext name of the removed file is
the only thing matching non-ext-versions of .gitignore, i.e. to catch
the common case of removing a "foo.c", but leaving a "foo.o" gitignore
entry behind.

See 95c16418f03 (rm: delete .gitmodules entry of submodules removed from
the work tree, 2013-08-06) for the commit that gave "git rm" these
smarts.

I used to think that "git rm" was a bad layering violation, i.e. what's
the point of it over:

    rm foo
    git add foo

And in the early days having it at all is IMO one of the things that
confused a lot of users about git's data model, i.e. they'd re-do their
moves or removals with the "git mv" or "git rm", thinking the SCM cared
about being informed of things like that.

But those commands are worthwhile, because they interact nicely with
other native git features, pathspecs, .gitmodules etc.

FWIW I think one thing that might make the "git submodule" interface
less confusing is to go in the opposite direction, not to add a "git
submodule rm" or whatever, but think about whether we need something
like "git submodule add" at all, i.e. shouldn't that ideally just be
some special-case of "git add" or "git clone"?

I.e. Either you "git clone" to the working tree and it Just Works (adds
.gitmodules and all), or "git add" on a directory with a .git dir in it
will (perhps after asking) shimmy up the relevant submodule scaffolding.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: Git submodule remove
  2021-10-21 22:47   ` Junio C Hamano
  2021-10-21 23:25     ` Matheus Tavares
@ 2021-10-22 13:47     ` Randall S. Becker
  2021-10-22 17:52       ` Kalyan Sriram
  1 sibling, 1 reply; 15+ messages in thread
From: Randall S. Becker @ 2021-10-22 13:47 UTC (permalink / raw)
  To: 'Junio C Hamano', 'brian m. carlson'
  Cc: 'Kalyan Sriram', git

On October 21, 2021 6:47 PM, Junio C Hamano wrote:
>To: brian m. carlson <sandals@crustytoothpaste.net>
>Cc: Kalyan Sriram <kalyan@coderkalyan.com>; git@vger.kernel.org
>Subject: Re: Git submodule remove
>
>"brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
>>> Hello,
>>>
>>> I was curious why git-submodule does not have an `rm` command.
>>> Currently I have to manually delete it from .gitmodules, .git/config,
>>> .git/modules/, etc. See [0].
>>>
>>> I'd like to contribute my first patch to this project by adding this
>>> feature, but wanted to check first with the community if there's any
>>> particular reason it was chosen not to not be implemented, or if it's
>>> simply that nobody has gotten around to it - it seems to be a
>>> relatively common feature someone might want.
>>>
>>> Anyway, please let me know if this is something that would be
>>> accepted, or if anyone has any comments or suggestions.
>>
>> I think the reason it hasn't been implemented is that nobody's gotten
>> around to it yet.  I certainly would find this useful and have wanted
>> the same thing myself, so I can't see a reason why the right series
>> wouldn't be accepted.
>
>I tend to agree that nobody felt the need strongly enough.  Code tends to accumulate without ever getting removed, and removal of a
file,
>removal of a directory, or removal of a submodule is a much rarer event compared to other changes people would need to make.
>Adding such a feature would have been much more work for those who faced such a rare occasion to want to use it than just doing it
by
>hand and committing the result.
>
>I'd imagine that the happy-case implementation should be fairly straight-forward.  You would:
>
> - ensure that the submodule is "absorbed" already;
>
> - run "git rm -f" the submodule to remove the gitlink from the index
>   and remove the directory from the working tree; and
>
> - remove the .gitmodules entry for the submodule.
>
>and you'd leave the final "record the state of the index as a commit" to the user, simply because the user would want to have other
>changes related to the removal of the submodule in the same commit (like changes to files in the superproject that refer to the
>submodule contents or removal of other submodules).
>
>The hard part is unhappy-cases.  There are too many things that can go wrong and you need to handle all the error cases correctly
so that
>you do not leave the user's repository in an uncontrollably messy state.

Just my rambling:

The really unhappy place is when a user deletes the upstream submodule repo itself after not seeing it in main any longer during
some cleanup adventure, then someone else tries to check out an older commit that references the submodule. This particular unhappy
place seems a whole lot like 'git branch -d' vs '-D', where it might be good not to allow the submodule rm if it is referenced in a
commit (insert acceptable criteria for not forcing it here, which probably doesn't exist). A prune-like operation, as with workspace
prune might be a little safer - but not much.

-Randall


^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: Git submodule remove
  2021-10-22 13:47     ` Randall S. Becker
@ 2021-10-22 17:52       ` Kalyan Sriram
  2021-10-22 20:45         ` Randall S. Becker
  2021-10-22 21:17         ` Junio C Hamano
  0 siblings, 2 replies; 15+ messages in thread
From: Kalyan Sriram @ 2021-10-22 17:52 UTC (permalink / raw)
  To: Randall S. Becker, 'Junio C Hamano',
	'brian m. carlson'; +Cc: git

On Fri Oct 22, 2021 at 6:47 AM PDT, Randall S. Becker wrote:
> On October 21, 2021 6:47 PM, Junio C Hamano wrote:
> >To: brian m. carlson <sandals@crustytoothpaste.net>
> >Cc: Kalyan Sriram <kalyan@coderkalyan.com>; git@vger.kernel.org
> >Subject: Re: Git submodule remove
> >
> >"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> >
> >> On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
> >>> Hello,
> >>>
> >>> I was curious why git-submodule does not have an `rm` command.
> >>> Currently I have to manually delete it from .gitmodules, .git/config,
> >>> .git/modules/, etc. See [0].
> >>>
> >>> I'd like to contribute my first patch to this project by adding this
> >>> feature, but wanted to check first with the community if there's any
> >>> particular reason it was chosen not to not be implemented, or if it's
> >>> simply that nobody has gotten around to it - it seems to be a
> >>> relatively common feature someone might want.
> >>>
> >>> Anyway, please let me know if this is something that would be
> >>> accepted, or if anyone has any comments or suggestions.
> >>
> >> I think the reason it hasn't been implemented is that nobody's gotten
> >> around to it yet.  I certainly would find this useful and have wanted
> >> the same thing myself, so I can't see a reason why the right series
> >> wouldn't be accepted.
> >
> >I tend to agree that nobody felt the need strongly enough.  Code tends to accumulate without ever getting removed, and removal of a
> file,
> >removal of a directory, or removal of a submodule is a much rarer event compared to other changes people would need to make.
> >Adding such a feature would have been much more work for those who faced such a rare occasion to want to use it than just doing it
> by
> >hand and committing the result.
> >
> >I'd imagine that the happy-case implementation should be fairly straight-forward.  You would:
> >
> > - ensure that the submodule is "absorbed" already;
> >
> > - run "git rm -f" the submodule to remove the gitlink from the index
> >   and remove the directory from the working tree; and
> >
> > - remove the .gitmodules entry for the submodule.
> >
> >and you'd leave the final "record the state of the index as a commit" to the user, simply because the user would want to have other
> >changes related to the removal of the submodule in the same commit (like changes to files in the superproject that refer to the
> >submodule contents or removal of other submodules).
> >
> >The hard part is unhappy-cases.  There are too many things that can go wrong and you need to handle all the error cases correctly
> so that
> >you do not leave the user's repository in an uncontrollably messy state.
>
> Just my rambling:
>
> The really unhappy place is when a user deletes the upstream submodule
> repo itself after not seeing it in main any longer during
> some cleanup adventure, then someone else tries to check out an older
> commit that references the submodule.
IMO this seems like a pretty unlikely situation to be in, which doesn't
warrant *not* adding this feature. I get the idea, but how commonly do
people checkout old commits and play around with them? In any case, this
seems to be the project maintainer's problem, not git's.
> This particular unhappy
> place seems a whole lot like 'git branch -d' vs '-D', where it might be
> good not to allow the submodule rm if it is referenced in a
> commit (insert acceptable criteria for not forcing it here, which
> probably doesn't exist).
But wouldn't a submodule always be referenced in a commit? The first
thing anyone does after adding a submodule, after all, is to commit it
into the repository. Later on, you may decide you don't need that
submodule dependency anymore...
> A prune-like operation, as with workspace
> prune might be a little safer - but not much.
Sorry, I'm having a hard time understanding. What would this look like, and how
it would be safer?

Thanks!
Kalyan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* RE: Git submodule remove
  2021-10-22 17:52       ` Kalyan Sriram
@ 2021-10-22 20:45         ` Randall S. Becker
  2021-10-22 21:17         ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Randall S. Becker @ 2021-10-22 20:45 UTC (permalink / raw)
  To: 'Kalyan Sriram', 'Junio C Hamano',
	'brian m. carlson'
  Cc: git

On October 22, 2021 1:52 PM, Kalyan Sriram wrote:
>To: Randall S. Becker <rsbecker@nexbridge.com>; 'Junio C Hamano' <gitster@pobox.com>; 'brian m. carlson'
><sandals@crustytoothpaste.net>
>Cc: git@vger.kernel.org
>Subject: RE: Git submodule remove
>
>On Fri Oct 22, 2021 at 6:47 AM PDT, Randall S. Becker wrote:
>> On October 21, 2021 6:47 PM, Junio C Hamano wrote:
>> >To: brian m. carlson <sandals@crustytoothpaste.net>
>> >Cc: Kalyan Sriram <kalyan@coderkalyan.com>; git@vger.kernel.org
>> >Subject: Re: Git submodule remove
>> >
>> >"brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> >
>> >> On 2021-10-21 at 17:25:38, Kalyan Sriram wrote:
>> >>> Hello,
>> >>>
>> >>> I was curious why git-submodule does not have an `rm` command.
>> >>> Currently I have to manually delete it from .gitmodules,
>> >>> .git/config, .git/modules/, etc. See [0].
>> >>>
>> >>> I'd like to contribute my first patch to this project by adding
>> >>> this feature, but wanted to check first with the community if
>> >>> there's any particular reason it was chosen not to not be
>> >>> implemented, or if it's simply that nobody has gotten around to it
>> >>> - it seems to be a relatively common feature someone might want.
>> >>>
>> >>> Anyway, please let me know if this is something that would be
>> >>> accepted, or if anyone has any comments or suggestions.
>> >>
>> >> I think the reason it hasn't been implemented is that nobody's
>> >> gotten around to it yet.  I certainly would find this useful and
>> >> have wanted the same thing myself, so I can't see a reason why the
>> >> right series wouldn't be accepted.
>> >
>> >I tend to agree that nobody felt the need strongly enough.  Code
>> >tends to accumulate without ever getting removed, and removal of a
>> file,
>> >removal of a directory, or removal of a submodule is a much rarer event compared to other changes people would need to make.
>> >Adding such a feature would have been much more work for those who
>> >faced such a rare occasion to want to use it than just doing it
>> by
>> >hand and committing the result.
>> >
>> >I'd imagine that the happy-case implementation should be fairly straight-forward.  You would:
>> >
>> > - ensure that the submodule is "absorbed" already;
>> >
>> > - run "git rm -f" the submodule to remove the gitlink from the index
>> >   and remove the directory from the working tree; and
>> >
>> > - remove the .gitmodules entry for the submodule.
>> >
>> >and you'd leave the final "record the state of the index as a commit"
>> >to the user, simply because the user would want to have other changes
>> >related to the removal of the submodule in the same commit (like changes to files in the superproject that refer to the submodule
>contents or removal of other submodules).
>> >
>> >The hard part is unhappy-cases.  There are too many things that can
>> >go wrong and you need to handle all the error cases correctly
>> so that
>> >you do not leave the user's repository in an uncontrollably messy state.
>>
>> Just my rambling:
>>
>> The really unhappy place is when a user deletes the upstream submodule
>> repo itself after not seeing it in main any longer during some cleanup
>> adventure, then someone else tries to check out an older commit that
>> references the submodule.
>IMO this seems like a pretty unlikely situation to be in, which doesn't warrant *not* adding this feature. I get the idea, but how commonly
>do people checkout old commits and play around with them? In any case, this seems to be the project maintainer's problem, not git's.
>> This particular unhappy
>> place seems a whole lot like 'git branch -d' vs '-D', where it might
>> be good not to allow the submodule rm if it is referenced in a commit
>> (insert acceptable criteria for not forcing it here, which probably
>> doesn't exist).
>But wouldn't a submodule always be referenced in a commit? The first thing anyone does after adding a submodule, after all, is to
>commit it into the repository. Later on, you may decide you don't need that submodule dependency anymore...
>> A prune-like operation, as with workspace prune might be a little
>> safer - but not much.
>Sorry, I'm having a hard time understanding. What would this look like, and how it would be safer?

Given that git is distributed, probably not safer at all (see other responses in the thread). I really don't like the submodule rm concept in the slightest although I appreciate the necessity, but you really really have to want it. A prune operation might be better if a criteria could be specified to define when removing the submodule might be acceptable, at least locally. Suppose:

git submodule prune --branch=main --after=commitish

which could allow the prune if the submodule was not referenced on the main branches before a certain point in history. I don't like it, but it might be an idea worth thinking about.

-Randall


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-22 17:52       ` Kalyan Sriram
  2021-10-22 20:45         ` Randall S. Becker
@ 2021-10-22 21:17         ` Junio C Hamano
  1 sibling, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2021-10-22 21:17 UTC (permalink / raw)
  To: Kalyan Sriram; +Cc: Randall S. Becker, 'brian m. carlson', git

"Kalyan Sriram" <kalyan@coderkalyan.com> writes:

>> The really unhappy place is when a user deletes the upstream submodule
>> repo itself after not seeing it in main any longer during
>> some cleanup adventure, then someone else tries to check out an older
>> commit that references the submodule.
>
> IMO this seems like a pretty unlikely situation to be in, which doesn't
> warrant *not* adding this feature. I get the idea, but how commonly do
> people checkout old commits and play around with them?

You MUST be able to go back to old versions.  Otherwise, why are we
working on a version control system?  You may find that in the old
days in v1.0 something used to work and in today's code it does not.
You would want to bisect and need to be able to go back to v1.0 that
still had the submodule you recently removed.  And you check where
in the history since v1.0 the actual breakage happend---it is called
"bisection" and people do that all the time.  IOW, it is VERY common.

And that is why I said "submodule rm" should *not* just "rm -fr
submodule-dir/", but ensure that submodule-dir/.git is absorbed (in
.git/modules/submodule-dir, most likely) before doing so, and then
the entry for the submodule should be removed from .gitmodules, but
I deliberatly did not mention anything about .git/config.  The entry
should in general _stay_ there, because having an entry for a
submodule in .git/config means you are interested in it (e.g. your
build may require having it, for a commit in the superproject that
still had the submodule there).  When you are on a commit in the
superproject after the submodule was removed, you may not have
anything from the submodule materialized in the working tree, but
when you go back in the history, you do still need to be able to
materialize it.

> In any case, this
> seems to be the project maintainer's problem, not git's.

You are correct.  The remote side removing some repository and
making it unavailable is just as bad as the site going down.  

On your side, you would want to be able to continue working even
when that happens, and not simply removing submodule-dir/.git
together with submodule-dir/ before making sure the submodule is
absorbed makes sure you'd have a local copy of the submodule
repository available just like you have local copy of the
superproject repository available.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-22 12:12         ` Ævar Arnfjörð Bjarmason
@ 2021-10-22 21:32           ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2021-10-22 21:32 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: Matheus Tavares, brian m. carlson, Kalyan Sriram, git,
	Emily Shaffer

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Why would it be a good thing to change it even if we could? We added
> that section with "git submodule add", it makes sense to have it removed
> on "git init".

... on "git submodule rm", yes.  Not "git rm". I am not sure what
your "git init" is your typo for.

That is primarily because I do not believe the "git submodule"'s
world view is the _only_ valid porcelain UI for a feature that lets
you use another project as if it is one of your subdirectories.  So
whatever opinionated policy the current "git submodule" implements,
I'd prefer to keep it in "git submodule" (and ".gitmodules", which is
where the data to support "git submodule"-style submodule handling
lives in), not in the lower level "git add" and "git rm".

> If anything I think it would make sense to extend this behavior. E.g. if
> we "git rm" a <path> and notice that the <path> was the only thing that
> matched a given entry in .gitignore we should remove that entry.

I think that is a wrong analogy.

It does not make sense to me to remove "*.o" from my .gitignore,
when I do "git rm vendor-binary.o". I have "*.o" in .gitignore
because I do not want to add .o files produced by compiling my
source files, and the "git rm" does not change that fact at all.

I unfortunately had to keep track of an opaque vendor-supplied
binary, which got replaced recently with a new version, and that is
why I am running "git rm" on it.  "git add -f vendor-binary-v2.o"
is what I was planning to do next.

I do not want you to mess with my .gitignore; you are assuming too
much on my workflow.

> See 95c16418f03 (rm: delete .gitmodules entry of submodules removed from
> the work tree, 2013-08-06) for the commit that gave "git rm" these
> smarts.

I know.  That is exactly the layering violation I am unhappy about
but we came with it too far to remove.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-22  3:32         ` Kalyan Sriram
@ 2021-10-22 22:02           ` Junio C Hamano
  2021-10-24 20:33             ` Kalyan Sriram
  0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2021-10-22 22:02 UTC (permalink / raw)
  To: Kalyan Sriram; +Cc: Matheus Tavares, brian m. carlson, git

"Kalyan Sriram" <kalyan@coderkalyan.com> writes:

> "git rm" deletes the submodule directory completely and modifies
> .gitmodules, effectively removing the submodule. However, it leaves the
> entry in .git/config dangling, which is annoying.

The entry is not dangling.  It is there to be used when you go back
in history.

> "git submodule deinit" (which I didn't know existed until I just read the man
> page) deletes all contents of submodule directory, but leaves the 
> (empty) submodule directory itself intact. It DOES delete the entry in
> .git/config, but leaves a dangling entry in .gitmodules, so the next
> "git submodule update --init --recursive" registers and populates the
> submodule again.

"deinit" is *not* about remove a submodule.  A project can be
checked out and used with or without its submodules instantiated,
and "git submodule init" is a way to instantiate it.  "deinit" is
its opposite.  As far as the history (which has already been
recorded in the repository, and the history that will be recorded
in the repository starting from that state) is concerned, the
submodule is there---it's just that you are not interested in it and
chose not to check it out.

So "git rm" seems to be doing exactly what "git submodule rm" should
be doing, nothing more, nothing less.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-22 22:02           ` Junio C Hamano
@ 2021-10-24 20:33             ` Kalyan Sriram
  2021-10-24 20:46               ` Junio C Hamano
  0 siblings, 1 reply; 15+ messages in thread
From: Kalyan Sriram @ 2021-10-24 20:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matheus Tavares, brian m. carlson, git

On Fri Oct 22, 2021 at 3:02 PM PDT, Junio C Hamano wrote:
> "Kalyan Sriram" <kalyan@coderkalyan.com> writes:
>
> > "git rm" deletes the submodule directory completely and modifies
> > .gitmodules, effectively removing the submodule. However, it leaves the
> > entry in .git/config dangling, which is annoying.
>
> The entry is not dangling. It is there to be used when you go back
> in history.
>
> > "git submodule deinit" (which I didn't know existed until I just read the man
> > page) deletes all contents of submodule directory, but leaves the 
> > (empty) submodule directory itself intact. It DOES delete the entry in
> > .git/config, but leaves a dangling entry in .gitmodules, so the next
> > "git submodule update --init --recursive" registers and populates the
> > submodule again.
>
> "deinit" is *not* about remove a submodule. A project can be
> checked out and used with or without its submodules instantiated,
> and "git submodule init" is a way to instantiate it. "deinit" is
> its opposite. As far as the history (which has already been
> recorded in the repository, and the history that will be recorded
> in the repository starting from that state) is concerned, the
> submodule is there---it's just that you are not interested in it and
> chose not to check it out.
>
> So "git rm" seems to be doing exactly what "git submodule rm" should
> be doing, nothing more, nothing less.

That makes sense, thanks for the clarification. So it looks like there
isn't any work to be done here after all?

What are your thoughts about aliasing git submodule rm to git rm? 

Thanks,
Kalyan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: Git submodule remove
  2021-10-24 20:33             ` Kalyan Sriram
@ 2021-10-24 20:46               ` Junio C Hamano
  0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2021-10-24 20:46 UTC (permalink / raw)
  To: Kalyan Sriram; +Cc: Matheus Tavares, brian m. carlson, git

"Kalyan Sriram" <kalyan@coderkalyan.com> writes:

>> So "git rm" seems to be doing exactly what "git submodule rm" should
>> be doing, nothing more, nothing less.
>
> That makes sense, thanks for the clarification. So it looks like there
> isn't any work to be done here after all?
>
> What are your thoughts about aliasing git submodule rm to git rm? 

It is unfortunate that "git rm" learned these "extra" things that
are integral part of "submodule removal" if you believe that the way
"submodules" are handled in the current "git submodule"-style design
is the only thing all Git users must follow.

Unless we plan to eventually make "git rm" unlearn these hardcoded
'git submodule'-way actions it does (like editing the .gitmodules
file) and move them to "git submodule rm", I do not think it is
worth the engineering effort to add an alias so that "git submodule
rm" would be a synonym to the current "git rm".

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-10-24 20:46 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 17:25 Git submodule remove Kalyan Sriram
2021-10-21 21:36 ` brian m. carlson
2021-10-21 22:47   ` Junio C Hamano
2021-10-21 23:25     ` Matheus Tavares
2021-10-21 23:35       ` Junio C Hamano
2021-10-22  3:32         ` Kalyan Sriram
2021-10-22 22:02           ` Junio C Hamano
2021-10-24 20:33             ` Kalyan Sriram
2021-10-24 20:46               ` Junio C Hamano
2021-10-22 12:12         ` Ævar Arnfjörð Bjarmason
2021-10-22 21:32           ` Junio C Hamano
2021-10-22 13:47     ` Randall S. Becker
2021-10-22 17:52       ` Kalyan Sriram
2021-10-22 20:45         ` Randall S. Becker
2021-10-22 21:17         ` 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).