Re: [PATCH v2 8/9] read_early_config(): really discover .git/

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Peff,

On Fri, 3 Mar 2017, Jeff King wrote:

> I think the "dirty hack..." comment in read_early_config() can be
> condensed (or go away) now.

Yes, I made that change in response to a comment you made about an earlier
patch in this series.

> I think we _could_ do away with read_early_config() entirely, and just
> have the regular config code do this lookup when we're not already in a
> repo. But then we'd really need to depend on the "creating_repository"
> flag being managed correctly.

Well, that would be a major design change. I'm not really all that
comfortable with that...

> I think I prefer the idea that a few "early" spots like pager and alias
> config need to use this special function to access the config. That's
> less likely to cause surprises when some config option is picked up
> before we have run setup_git_directory().

Exactly. There is semantic meaning in calling read_early_config() vs
git_config().

> There is one surprising case that I think we need to deal with even now,
> though. If I do:
> 
>   git init repo
>   git -C repo config pager.upload-pack 'echo whoops'
>   git upload-pack repo
>   cd repo && git upload-pack .
> 
> the first one is OK, but the second reads and executes the pager config
> from the repo, even though we usually consider upload-pack to be OK to
> run in an untrusted repo. This _isn't_ a new thing in your patch, but
> just something I noticed while we are here.
> 
> And maybe it is a non-issue. The early-config bits all happen via the
> git wrapper, and normally we use the direct dashed "git-upload-pack" to
> access the other repositories. Certainly I have been known to use "git
> -c ... upload-pack" while debugging various things.
> 
> So I dunno. You really have to jump through some hoops for it to matter,
> but I just wonder if the git wrapper should be special-casing
> upload-pack, too.

While I agree that it may sound surprising, I would like to point out that
there *is* a semantic difference between `git upload-pack repo` and `git
-C repo upload-pack .`: the working directory is different. And given that
so much in Git's operations depend on the working directory, it makes
sense that those two invocations have slightly different semantics, too.

I also agree, obviously, that this is something that would need a separate
patch series to tackle ;-)

Ciao,
Dscho



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]