git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Richard Hansen <hansenr@google.com>, git@vger.kernel.org
Subject: Re: [PATCH 1/2] diff: document behavior of relative diff.orderFile
Date: Wed, 11 Jan 2017 09:41:59 -0500	[thread overview]
Message-ID: <20170111144158.ef6kle3vw3ejgmut@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqtw96pno0.fsf@gitster.mtv.corp.google.com>

On Tue, Jan 10, 2017 at 02:15:11PM -0800, Junio C Hamano wrote:

> Richard Hansen <hansenr@google.com> writes:
> 
> >> A related tangent.
> >>
> >> I wonder if anything that uses git_config_pathname() should be
> >> relative to GIT_DIR when it is not absolute.
> >
> > I think so.  (For bare repositories anyway; non-bare should be
> > relative to GIT_WORK_TREE.)  Perhaps git_config_pathname() itself
> > should convert relative paths to absolute so that every pathname
> > setting automatically works without changing any calling code.
> 
> Yes, that was what I was alluding to.  We might have to wait until
> major version boundary to do so, but I think that it is the sensible
> way forward in the longer term to convert relative to absolute in
> git_config_pathname().

Yeah, I'd agree.

I'm undecided on whether it would need to happen at a major version
bump. The existing semantics are fairly insane, and would cause a lot of
confusing breakages. We can imagine use of relative paths in a bare
repository falls into one of a few categories:

  1. The user generally runs by "cd /path/to/bare.git && git ...". This
     would be unaffected, as relative and $GIT_DIR are the same.

  2. The user runs via "cd /path/to/bare.git/some-subdir". This would be
     broken, but I have trouble imagining that they really wanted to
     read something like "objects/orderfile".

  3. The user runs via "GIT_DIR=/path/to/bare.git" from various
     directories. This case is probably horribly broken, as things like
     diff.orderFile will complain if they ever run from a directory that
     doesn't have the order file.

  4. They run GIT_DIR=/path/to/bare.git from a consistent origin
     directory. This _does_ work, and we'd be breaking it. Though I kind
     of question why the config in $GIT_DIR is meant to apply to a file
     in a totally unrelated directory.

     I suppose somebody could be relying on the behavior where setting
     GIT_DIR uses the current directory as the working tree (i.e., if
     core.bare is "true" in bare.git). But then, we'd consider their
     working directory as the working tree and read from that anyway. So
     the behavior would stay the same.

So I dunno. I do hate to break even corner cases, but I'm having trouble
imagining the scenario where somebody is actually using the current
behavior in a useful way.

-Peff

  reply	other threads:[~2017-01-11 14:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-10  0:40 [PATCH 0/2] minor diff orderfile documentation improvements Richard Hansen
2017-01-10  0:40 ` [PATCH 1/2] diff: document behavior of relative diff.orderFile Richard Hansen
2017-01-10  6:58   ` Jeff King
2017-01-10 17:27     ` Richard Hansen
2017-01-10 20:19       ` Junio C Hamano
2017-01-10 20:23         ` Junio C Hamano
2017-01-10 22:01           ` Richard Hansen
2017-01-10 22:15             ` Junio C Hamano
2017-01-11 14:41               ` Jeff King [this message]
2017-01-11 20:53                 ` Junio C Hamano
2017-01-10  0:40 ` [PATCH 2/2] diff: document the pattern format for diff.orderFile Richard Hansen
2017-01-10 20:14   ` Junio C Hamano
2017-01-11  1:14     ` Richard Hansen
2017-01-11  2:46       ` Junio C Hamano
2017-01-11 17:24         ` Richard Hansen
2017-01-11 18:15           ` Junio C Hamano
2017-01-11 18:36             ` Richard Hansen
2017-01-11 21:06               ` Junio C Hamano
2017-01-11  1:57 ` [PATCH v2 0/2] diff orderfile documentation improvements Richard Hansen
2017-01-11  1:57   ` [PATCH v2 1/2] diff: document behavior of relative diff.orderFile Richard Hansen
2017-01-11  1:57   ` [PATCH v2 2/2] diff: document the format of the -O (diff.orderFile) file Richard Hansen
2017-01-15 22:16   ` [PATCH v3 0/2] diff orderfile documentation improvements Richard Hansen
2017-01-15 22:16     ` [PATCH v3 1/2] diff: document behavior of relative diff.orderFile Richard Hansen
2017-01-15 22:16     ` [PATCH v3 2/2] diff: document the format of the -O (diff.orderFile) file Richard Hansen

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170111144158.ef6kle3vw3ejgmut@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hansenr@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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 NNTP newsgroup(s).