public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: Kirill Likhodedov <kirill.likhodedov@jetbrains•com>, git@vger•kernel.org
Subject: Re: [Bug] git pull doesn't recognize --work-tree parameter
Date: Thu, 13 Oct 2011 11:01:13 -0700	[thread overview]
Message-ID: <7vbotk6aae.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20111013155923.GA13134@sigill.intra.peff.net> (Jeff King's message of "Thu, 13 Oct 2011 11:59:24 -0400")

Jeff King <peff@peff•net> writes:

> Changing these scripts to use require_work_tree_exists is
> easy to verify. We immediately call cd_to_toplevel, anyway.
> Therefore no matter which function we use, the state
> afterwards is one of:
>
>   1. We have a work tree, and we are at the top level.
>
>   2. We don't have a work tree, and we have died.
>
> The only catch is that we must also make sure no code that
> ran before the cd_to_toplevel assumed that we were already
> in the working tree.
>
> In this case, we will only have included shell libraries and
> called set_reflog_action, neither of which care about the
> current working directory at all.
>
> Signed-off-by: Jeff King <peff@peff•net>
> ---
> This is the low-hanging, obviously correct fruit.

I am not absolutely sure about "obviously correct", given that you assume
that cd_to_toplevel does what its name makes you think it does.  I've been
wondering if we want to do give a bit more sanity to "cd_to_toplevel" and
"rev-parse --show-toplevel".

    $ pwd
    /srv/project/git/git.git
    $ (cd Documentation/howto && git rev-parse --show-toplevel); echo $?
    /srv/project/git/git.git
    0

So far so good, however:

    $ (cd .git/refs/heads && git rev-parse --show-toplevel); echo $?
    0

I do not think this is quite right.

We would probably want to add "rev-parse --show-work-tree", but we would
need to audit the users of cd_to_toplevel before starting to use it.  I
wouldn't be surprised if there is a script that creates a temporary work
tree in .git/some/where and runs the scripted Porcelains without setting
GIT_WORK_TREE, relying on the historical behaviour of cd_to_toplevel that
does not really go to the top level.

next prev parent reply	other threads:[~2011-10-13 18:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-13  8:38 [Bug] git pull doesn't recognize --work-tree parameter Kirill Likhodedov
2011-10-13 10:55 ` Nguyen Thai Ngoc Duy
2011-10-13 15:59 ` Jeff King
2011-10-13 18:01   ` Junio C Hamano [this message]
2011-10-13 18:37     ` Jeff King
2011-10-13 19:06       ` Junio C Hamano
2011-10-13 19:14         ` Jeff King
2011-10-13 19:44           ` Jeff King
2011-10-18  6:34             ` Junio C Hamano
2011-10-13 20:48           ` 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=7vbotk6aae.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=kirill.likhodedov@jetbrains$(echo .)com \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox