git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Peter Oberndorfer <kumbayo84@arcor.de>
To: Stefan Beller <sbeller@google.com>
Cc: git <git@vger.kernel.org>, Prathamesh Chavan <pc44800@gmail.com>
Subject: Re: git submodule deinit resulting in BUG: builtin/submodule--helper.c:1045: module_list_compute should not choke on empty pathspec
Date: Wed, 28 Mar 2018 21:37:19 +0200	[thread overview]
Message-ID: <9ead5ee1-9d4a-38f6-0fa3-ca4c982b33f5@arcor.de> (raw)
In-Reply-To: <CAGZ79kYGY5bjh0WPQh7xkXQxLkB9EQ-OcJhVuGE8YUnwmvk2Fg@mail.gmail.com>

On 2018-03-28 00:56, Stefan Beller wrote:
> On Tue, Mar 27, 2018 at 12:55 PM Peter Oberndorfer <kumbayo84@arcor.de>
> wrote:

Hi,

as expected your patch fixed the BUG output.
Thanks!

>> 2) Should "git submodule deinit" work on submodules that were removed by
> upstream already?
> 
> To answer the question "Is this a submodule that upstream removed
> (recently)?"
> we'd have to put in some effort, essentially checking if that was ever a
> submodule
> (and not a directory or file).
> 

Hmm, yeah looks a bit more complicated than I initially imagined
since submodules can have a name that's different from their path.
And after the rebase, the name <-> path mapping via .gitmodules is not available anymore.

Naively I think it could work the following way:
* Either iterate over all submodules in .git/modules/ and check their config
  has a worktree = "../../path" that resolves to the submodule path we want to remove.
* Or check the "gitlink:" path in submodule/.git if it points to our .git/modules/
Then if .git/config contains a [submodule "name"] entry
we should have a pretty good idea if this folder contains a stale submodule.

> When using "git pull --recurse-submodules" the submodule ought to be removed
> automatically.
> 
> When doing a fetch && merge manually, we may want to teach merge to remove
> a submodule that we have locally upon merge, too.
> 

Yeah that would be nice :-)
In my case I updated the repository via a rebase, so that would also have to be covered.

> I view the git-submodule command as a bare bones plumbing helper, that we'd
> want
> to deprecate eventually as all other higher level commands will know how to
> deal
> with submodules.
> 
> So I think we do not want to teach "git submodule deinit" to remove dormant
> repositories, that were submodules removed by upstream already.
> 

My gut feeling makes me expect the following:
* It would be nice if such stale submodules showed up in "git submodule status" or "git status"
  Now "git submodule" shows nothing related to this stale submodule
  Now "git status" shows  Untracked files: src/rt which is a bit confusing as the actual submodule is in src/rt/hoedown
  Now "Git gui" shows src/rt/hoedown as untracked git repository
* There should be an easy(and safe) way for the user to deinit such a submodule
  if if the automatic submodule updating during a merge/rebase was not enabled or somehow failed.
(Minus the problem of somebody having to actually do the work...)

>> ~/src/rust/rust$ git submodule status
> ...
>>   b87873eaceb75cf9342d5273f01ba2c020f61ca8 src/tools/lld ((null))
> 
>> -> strangely I get (null) for the current branch/commit in some
> submodules?
> 
> This sounds like (3). Looking into that.

Sorry, what do you mean by (3)?

Thanks,
Greetings Peter

  parent reply	other threads:[~2018-03-28 19:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-27 19:48 git submodule deinit resulting in BUG: builtin/submodule--helper.c:1045: module_list_compute should not choke on empty pathspec Peter Oberndorfer
2018-03-27 22:56 ` Stefan Beller
2018-03-27 23:28   ` [PATCH] submodule deinit: handle non existing pathspecs gracefully Stefan Beller
2018-03-28  4:09     ` Martin Ågren
2018-03-28  5:06     ` Junio C Hamano
2018-03-28 19:37   ` Peter Oberndorfer [this message]
2018-03-28 21:19     ` git submodule deinit resulting in BUG: builtin/submodule--helper.c:1045: module_list_compute should not choke on empty pathspec Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9ead5ee1-9d4a-38f6-0fa3-ca4c982b33f5@arcor.de \
    --to=kumbayo84@arcor.de \
    --cc=git@vger.kernel.org \
    --cc=pc44800@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).