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: git@vger.kernel.org
Subject: Re: What's cooking in git.git (Oct 2011, #05; Fri, 14)
Date: Sun, 16 Oct 2011 13:27:06 -0400	[thread overview]
Message-ID: <20111016172706.GA17348@sigill.intra.peff.net> (raw)
In-Reply-To: <7vvcrorh49.fsf@alter.siamese.dyndns.org>

On Sun, Oct 16, 2011 at 10:17:10AM -0700, Junio C Hamano wrote:

> >> * jk/pull-rebase-with-work-tree (2011-10-13) 1 commit
> >>  - pull,rebase: handle GIT_WORK_TREE better
> >> 
> >> Looked reasonable.
> >> Will merge to 'next'.
> >
> > I'm not so sure. Didn't you demonstrate that cd_to_toplevel as-is will
> > not actually go to the toplevel if we're outside of the work tree?
> >
> > And changing it is non-trivial, because there may be weird cases that
> > rely on staying there? See my final note in the thread:
> >
> >   http://article.gmane.org/gmane.comp.version-control.git/183519
> 
> Hmm, I might be mistaken, but my impression was that sane people do not do
> so, that the discussion that originated this proposed patch was not such a
> use case, and most importantly that fixing unsane ones is just the matter
> for them to set GIT_WORKING_TREE correctly. So if anything, wouldn't
> getting this in as early as possible to 'master' or at least 'next' help
> catching a flaw in the above logic and possible downside in the real
> world?

Hmm. I thought there were two separate problems:

  1. my analysis was wrong, and "git rev-parse --show-toplevel" did not
     actually show the root of the work tree when we were outside it
     (and therefore cd_to_toplevel did not actually go anywhere)

  2. some people might be outside of the work tree, set GIT_DIR
     explicitly, but not bother setting GIT_WORK_TREE

But I was wrong on (1). If GIT_WORK_TREE is set, we _will_ actually go
to its work tree, which is what we want. If it's not set, then we go
nowhere, and assume the cwd is the work tree. Which is compatible with
current behavior, and makes (2) still work.

So I was thinking there was more problem then there is. I agree we
should let it go to 'next' to shake out any bugs.

Sorry for the noise.

-Peff

  reply	other threads:[~2011-10-16 17:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-14 23:23 What's cooking in git.git (Oct 2011, #05; Fri, 14) Junio C Hamano
2011-10-16 16:53 ` Jeff King
2011-10-16 17:17   ` Junio C Hamano
2011-10-16 17:27     ` Jeff King [this message]
2011-10-17  3:37     ` Jeff King
2011-10-17  4:20       ` Junio C Hamano

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=20111016172706.GA17348@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).