git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Stefan Beller <sbeller@google.com>
To: sammck@gmail.com
Cc: git <git@vger.kernel.org>, smckelvie@xevo.com
Subject: Re: [PATCH] submodule.c: Make get_superproject_working_tree() work when supermodule has unmerged changes of the submodule reference
Date: Mon, 24 Sep 2018 18:24:07 -0700
Message-ID: <CAGZ79kZa-d3kPXp=q-YezffF68BUZzXv_tUaOKCi8=9SHy6jrA@mail.gmail.com> (raw)
In-Reply-To: <20180924212352.41909-1-smckelvie@xevo.com>

On Mon, Sep 24, 2018 at 2:25 PM Sam McKelvie <sammck@gmail.com> wrote:

Thanks for writing a patch!
I wonder if we can shorten the subject and make it a bit more concise,
how about

    submodule: Alllow staged changes for get_superproject_working_tree

or

    rev-parse: allow staged submodule in --show-superproject-working-tree


>     Previously, "fatal: BUG: returned path string doesn't match cwd?" is displayed and
>     git-rev-parse aborted.

Usually it is hard to read, continuing from the commit title to the body
of the commit message, as the title may be displayed differently or
somewhere else, so it would be good to restate it or rather state the
command that is broken, which we are trying to fix.

    Invoking 'git rev-parse --show-superproject-working-tree' exits with

        fatal: BUG ...

    instead of displaying the superproject working tree when ....

>     The problem is due to the fact that when a merge of the submodule reference is in progress,
>     "git --stage —full-name <submodule-relative-path>” returns three seperate entries for the

"ls-files" is missing in that git invocation?

>     submodule (one for each stage) rather than a single entry; e.g.,
>
>     $ git ls-files --stage --full-name submodule-child-test
>     160000 dbbd2766fa330fa741ea59bb38689fcc2d283ac5 1       submodule-child-test
>     160000 f174d1dbfe863a59692c3bdae730a36f2a788c51 2       submodule-child-test
>     160000 e6178f3a58b958543952e12824aa2106d560f21d 3       submodule-child-test
>
>     The code in get_superproject_working_tree() expected exactly one entry to be returned;
>     this patch makes it use the first entry if multiple entries are returned.

The code looks good.

I wonder if we want to add a test for it, see t/t1500-rev-parse.sh
(at the very end 'showing the superproject correctly').

>
>     Signed-off-by: Sam McKelvie <smckelvie@xevo.com>

Side note: the commit message seems to be indented,
but the patch applies fine, which makes me curious:
How did you produce&send the patch?
(Usually a patch would not apply as white spaces are
mangled in the code for example in some email setups)
(If I had to guess I could imagine that it is the output of
git-show as that indents the commit message usually,
see git-format-patch for a better tool)

Thanks for writing the patch!
Stefan

      reply index

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-24 21:23 Sam McKelvie
2018-09-25  1:24 ` Stefan Beller [this message]

Reply instructions:

You may reply publically 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='CAGZ79kZa-d3kPXp=q-YezffF68BUZzXv_tUaOKCi8=9SHy6jrA@mail.gmail.com' \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=sammck@gmail.com \
    --cc=smckelvie@xevo.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

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox