git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Rast <tr@thomasrast.ch>
Cc: "Duy Nguyen" <pclouds@gmail.com>,
	"Øystein Walle" <oystwa@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: git stash doesn't honor --work-tree or GIT_WORK_TREE
Date: Mon, 02 Dec 2013 10:01:33 -0800	[thread overview]
Message-ID: <xmqqfvqbrv2a.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <87zjokpo47.fsf@linux-1gf2.Speedport_W723_V_Typ_A_1_00_098> (Thomas Rast's message of "Sun, 01 Dec 2013 16:50:00 +0100")

Thomas Rast <tr@thomasrast.ch> writes:

> The problem here is that shell scripts that want to do something with a
> worktree tend to call require_work_tree in git-sh-setup:
>
> require_work_tree () {
> 	test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
> 	die "fatal: $0 cannot be used without a working tree."
> }
>
> However, when an explicit GIT_WORK_TREE is in effect, that seems a bit
> silly.  The _intent_ of that command is "I need a worktree to work
> with".  But what it currently checks is something completely different,
> namely "am I _inside_ the worktree".

Correct.

I have a few issues with the proposed "solution", though.

 - require_work_tree has always meant that "This command has to be
   run inside a working tree".  That automatically implies that you
   cannot be working with a bare repository but it is much stronger
   than that.  You actually have to be inside one.

 - $GIT_WORK_TREE (and core.worktree) never meant "I do not bother
   to chdir there myself".  More specifically, "GIT_WORK_TREE=$there
   git foo" is not "cd $there && git foo".  It only means "Because
   the working tree may not have .git directory embedded in it, even
   though you may be able to know where the .git repository is
   (perhaps because I am telling you with $GIT_DIR), you may not be
   able to tell where the top level of the working tree is---hence I
   am telling you where it is".

 - "I do not bother to chdir there myself" has long been treated as
   a non-issue desire, but recently we added "git -C $there". We
   should not conflate GIT_WORK_TREE, which is a discovery mechanism
   for where the working tree is, with that.

 - Some command "git foo" may want to affect and/or look at only a
   part of the working tree, and the $cwd is one way of specifying
   that, e.g. "cd doc && git grep foo".  "If a command that needs to
   be run from somewhere in a working tree was run outside, error
   out." has been the general design principle for commands that
   interact with files in the working tree.

   It may be OK to propose changing it to "Instead of erroring out,
   pretend as if the command were run from the top-level of the
   working tree", but there is an issue of what to do with the
   command line arguments that are path-like.  For example,
   should this command sequence:

	cd /tmp
	GIT_WORK_TREE=/var/tmp/work git foo -f inputFile

   read from /tmp/inputFile, or /var/tmp/work/inputFile?  There may
   be other "unintuitive interactions with silent chdir"; with "git
   -C $there", we are very clear that chdir is the first thing that
   happens and everything path-like are relative to $there, which is
   similar to what "make" does, but doing the same for core.worktree
   or $GIT_WORK_TREE may be surprising, given that environment is
   "set and forget" (the examples in this message explicitly spell
   it out with one-shot export syntax, but that is only for
   illustration) and not visible during usual use of the commands.

> But it would give require_work_tree somewhat interesting and unintuitive
> side effects.

Exactly. My knee-jerk reaction is that I do not think we would want
to go there, but I haven't thought things through.

  reply	other threads:[~2013-12-02 18:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-30 19:04 git stash doesn't honor --work-tree or GIT_WORK_TREE Aaron Brooks
2013-11-30 21:22 ` Øystein Walle
2013-12-01 11:12   ` Thomas Rast
2013-12-01 11:50     ` Duy Nguyen
2013-12-01 15:50       ` Thomas Rast
2013-12-02 18:01         ` Junio C Hamano [this message]
2013-12-01 19:12       ` Øystein Walle

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=xmqqfvqbrv2a.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=oystwa@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=tr@thomasrast.ch \
    /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).