git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Antonio Ospite <ao2@ao2.it>
To: Stefan Beller <sbeller@google.com>
Cc: git <git@vger.kernel.org>,
	Richard Hartmann <richih.mailinglist@gmail.com>
Subject: Re: [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command
Date: Mon, 16 Apr 2018 18:18:41 +0200
Message-ID: <20180416181841.b486524b8b9b0e68e3a31bfa@ao2.it> (raw)
In-Reply-To: <CAGZ79kbnc17PZ9_=8QLkZgUZ0DHJKfWnxrekmgkLGFBU_0ieug@mail.gmail.com>

On Thu, 12 Apr 2018 16:55:15 -0700
Stefan Beller <sbeller@google.com> wrote:

> Hi Antonio,
> 
> the subject line could also be
>   fetch: fix custom submodule location
> as I was not expecting this patch from the subject line.
>

OK.

> On Thu, Apr 12, 2018 at 3:20 PM, Antonio Ospite <ao2@ao2.it> wrote:
> > Import the default git configuration before accessing the gitmodules
> > file.
> >
> > This is important when a value different from the default one is set via
> > the 'core.submodulesfile' config.
> >
> > Signed-off-by: Antonio Ospite <ao2@ao2.it>
> > ---
> >  builtin/fetch.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index dcdfc66f0..d56636f74 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -1428,8 +1428,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> >         for (i = 1; i < argc; i++)
> >                 strbuf_addf(&default_rla, " %s", argv[i]);
> >
> > -       config_from_gitmodules(gitmodules_fetch_config, NULL);
> >         git_config(git_fetch_config, NULL);
> > +       config_from_gitmodules(gitmodules_fetch_config, NULL);
> 
> Wouldn't this break the overwriting behavior?
> e.g. you configure a submodule URL in .gitmodules and then override it
> in .git/config,
> then we'd currently first load config from the modules file and then override it
> in git_config(git_fetch_config,..), whereas swapping them might have
> unintended consequences? Do the tests pass with this patch?

The patch changes the current behavior indeed, but it does not break
any of the existing tests.

Anyway, to be on the safe side, I could load only the
core.submodulesFile option from the global configuration, maybe even in
config_from_gitmodules() itself, before accessing the gitmodules file,
but I still don't know how to do that.

Is there an API to just load one config setting?

Ciao,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

  reply index

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-12 22:20 [RFC 00/10] Make .the gitmodules file path configurable Antonio Ospite
2018-04-12 22:20 ` [RFC 01/10] submodule: add 'core.submodulesFile' to override the '.gitmodules' path Antonio Ospite
2018-04-12 23:50   ` Stefan Beller
2018-04-16 16:37     ` Antonio Ospite
2018-04-16 21:22       ` Stefan Beller
2018-04-18 11:43         ` Antonio Ospite
2018-04-18 18:44           ` Stefan Beller
2018-04-12 22:20 ` [RFC 02/10] submodule: fix getting custom gitmodule file in fetch command Antonio Ospite
2018-04-12 23:55   ` Stefan Beller
2018-04-16 16:18     ` Antonio Ospite [this message]
2018-04-16 19:23       ` Stefan Beller
2018-04-16 20:46         ` Antonio Ospite
2018-04-12 22:20 ` [RFC 03/10] submodule: use the 'submodules_file' variable in output messages Antonio Ospite
2018-04-12 22:20 ` [RFC 04/10] submodule: document 'core.submodulesFile' and fix references to '.gitmodules' Antonio Ospite
2018-04-12 22:20 ` [RFC 05/10] submodule: adjust references to '.gitmodules' in comments Antonio Ospite
2018-04-12 22:20 ` [RFC 06/10] completion: add 'core.submodulesfile' to the git-completion.bash file Antonio Ospite
2018-04-12 23:36 ` [RFC 00/10] Make .the gitmodules file path configurable Stefan Beller
2018-04-16 11:33   ` Antonio Ospite
2018-04-16 19:22     ` Stefan Beller
2018-04-13  8:07 ` Antonio Ospite
2018-04-13  8:07 ` [RFC 07/10] FIXME: wrap-for-bin.sh: set 'core.submodulesFile' for each git invocation Antonio Ospite
2018-04-13  8:07   ` [RFC 08/10] FIXME: submodule: fix t1300-repo-config.sh to take into account the new config Antonio Ospite
2018-04-13  8:07   ` [RFC 09/10] FIXME: submodule: pass custom gitmodules file to 'test-tool submodule-config' Antonio Ospite
2018-04-13  8:07   ` [RFC 10/10] FIXME: add a hacky script to test the changes with a patched test suite Antonio Ospite
2018-04-13 20:05   ` [RFC 07/10] FIXME: wrap-for-bin.sh: set 'core.submodulesFile' for each git invocation Stefan Beller
2018-04-16 11:36     ` Antonio Ospite
2018-04-23 17:47 ` [RFC 00/10] Make .the gitmodules file path configurable Jonathan Nieder
2018-04-30 12:51   ` Antonio Ospite

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=20180416181841.b486524b8b9b0e68e3a31bfa@ao2.it \
    --to=ao2@ao2.it \
    --cc=git@vger.kernel.org \
    --cc=richih.mailinglist@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

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