Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()

From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j6t@kdbg.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
Date: Wed, 7 Nov 2018 20:18:14 -0500	[thread overview]
Message-ID: <20181108011813.GA10148@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqlg649zs8.fsf@gitster-ct.c.googlers.com>

On Thu, Nov 08, 2018 at 09:30:15AM +0900, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Wed, Nov 07, 2018 at 10:36:52PM +0100, Johannes Sixt wrote:
> >
> > All that said, if we're just interested in allowing this for config,
> > then we already have such a wrapper function: git_config_pathname().
> >
> > So I don't think it's a big deal to implement it in any of these ways.
> > It's much more important to get the syntax right, because that's
> > user-facing and will be with us forever.
> 
> All of us are on the same page after seeing the clarification by
> Dscho, it seems.  I came to pretty much the same conclusion this
> morning before reading this subthread.  Outside config values, the
> callers of expand_user_path() only feed "~/.git$constant", and they
> are all about "personal customization" that do not want to be shared
> with other users of the same installation, so "relative to runtime
> prefix" feature would not be wanted.  But we do not know about new
> caller's needs.  For now I am OK to have it in expand_user_path(),
> possibly renaming the function if people feel it is needed (I don't).

I think we would want to carefully think about the call in enter_repo().
We do not want git-daemon to accidentally expose repositories in
$RUNTIME_PREFIX.

Looking over the code, I think this is OK. The expansion happens in
enter_repo(), and then we take the path that was found and do our
ok_paths checks on it (which makes sense -- even now you'd ask to export
"/home/" and it would need to look at "~peff/repo.git" and expand that
to "/home/peff/repo.git" before doing a simple string check.

> Between ~<reserved name> and $VARIABLE_LOOKING_THINGS, I do not have
> a strong preference either way, but I am getting an impression that
> the latter is more generally favoured in the discussion?

I certainly prefer the latter, but I thought I was the only one to have
expressed support so far. ;)

-Peff