git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j.sixt@viscovery.net>, git@vger.kernel.org
Subject: Re: [PATCH 1/4] config: add include directive
Date: Thu, 26 Jan 2012 17:25:32 -0500	[thread overview]
Message-ID: <20120126222532.GA12855@sigill.intra.peff.net> (raw)
In-Reply-To: <7vmx9a2o23.fsf@alter.siamese.dyndns.org>

On Thu, Jan 26, 2012 at 12:42:28PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > And no, I didn't do any cycle detection. We could either do:
> >
> >   1. Record some canonical name for each source we look at (probably
> >      realpath() for files, and the sha1 for refs), and don't descend
> >      into already-seen sources.
> >
> >   2. Simply provide a maximum depth, and don't include beyond it.
> >
> > The latter is much simpler to implement, but I think the former is a
> > little nicer for the user.
> 
> Another thing I wondered after reading this patch was that it will be a
> rather common "mistake" to include the same file twice, one in ~/.gitconfig
> and then another in project specific .git/config, or more likely, people
> start using useful ones in ~their/.gitconfig, and then the site administrator
> by popular demand adding the same include in /etc/gitconfig to retroactively
> cause the same file included twice for them.
> 
> Your first alternative solution should solve this case nicely as well, I
> would think.

Agreed. I'll implement that, then.

There's a slight complication with when to clear the "I have seen this"
mark for each source. If you are interested only in breaking cycles,
then obviously you can just clear the marks as you pop the stack. But if
you want to stop repeated inclusion down different branches of the
include tree, you need to keep the marks until you're done (e.g., the
same file referenced by .git/config and ~/.gitconfig). But you do still
need to clear them, because we repeatedly call git_config with different
callback functions, and we need to re-parse each time.

I think it should be sufficient to make the marks live through
git_config(), and get cleared after that (so all of .git/config,
~/.gitconfig, and /etc/gitconfig will only load a given include once,
but another call to git_config will load it again).

-Peff

  reply	other threads:[~2012-01-26 22:25 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-26  7:35 [RFC/PATCH 0/4] config include directives Jeff King
2012-01-26  7:37 ` [PATCH 1/4] config: add include directive Jeff King
2012-01-26  9:16   ` Johannes Sixt
2012-01-26 16:54     ` Jeff King
2012-01-26 20:42       ` Junio C Hamano
2012-01-26 22:25         ` Jeff King [this message]
2012-01-26 22:43           ` Jeff King
2012-01-26 20:58   ` Junio C Hamano
2012-01-26 22:51     ` Jeff King
2012-01-27  5:23       ` Junio C Hamano
2012-01-27  5:55         ` Jeff King
2012-01-27 17:03       ` Jens Lehmann
2012-01-27  0:02   ` Ævar Arnfjörð Bjarmason
2012-01-27  0:32     ` Jeff King
2012-01-27  9:33       ` Ævar Arnfjörð Bjarmason
2012-01-27  5:07   ` Michael Haggerty
2012-01-27  5:54     ` Jeff King
2012-01-26  7:38 ` [PATCH 2/4] config: factor out config file stack management Jeff King
2012-01-26  7:40 ` [PATCH 3/4] config: support parsing config data from buffers Jeff King
2012-01-26  7:42 ` [PATCH 4/4] config: allow including config from repository blobs Jeff King
2012-01-26  9:25   ` Johannes Sixt
2012-01-26 17:22     ` Jeff King
2012-01-27  3:47     ` Nguyen Thai Ngoc Duy
2012-01-27  5:57       ` Jeff King
2012-01-26 21:14   ` Junio C Hamano
2012-01-26 23:00     ` Jeff King
2012-01-27  0:35       ` Junio C Hamano
2012-01-27  0:49         ` Jeff King
2012-01-27  5:30           ` Junio C Hamano
2012-01-27  5:42             ` Jeff King
2012-01-27  7:27               ` Johannes Sixt
2012-01-27 23:10                 ` Junio C Hamano
2012-01-27  4:01   ` Nguyen Thai Ngoc Duy
2012-01-27  5:59     ` Jeff King
2012-01-27  9:51 ` [RFC/PATCH 0/4] config include directives Ævar Arnfjörð Bjarmason
2012-01-27 17:34   ` Jeff King

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=20120126222532.GA12855@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    /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).