git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] Revert "setup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR"
Date: Tue, 22 Dec 2015 13:50:12 -0800	[thread overview]
Message-ID: <xmqqk2o6cfvv.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CACsJy8DP7ugh1UAU=iGzzHTSwUm5ZPkQhS0fTvkOpQV6vP1Jxg@mail.gmail.com> (Duy Nguyen's message of "Tue, 22 Dec 2015 08:06:20 +0700")

Duy Nguyen <pclouds@gmail.com> writes:

> On Tue, Dec 22, 2015 at 1:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>>
>>>> This commit has caused three regression reports so far. All of them are
>>>> about spawning git subprocesses, where the new presence of GIT_WORK_TREE
>>>> either changes command behaviour (git-init or git-clone), or how
>>>> repo/worktree is detected (from aliases), with or without $GIT_DIR.
>>>> The original bug will be re-fixed another way.
>>>>
>>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>>> ---
>>>>  On Thu, Dec 3, 2015 at 12:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>  > OK, when/if you decide that our first step should be a revert of
>>>>  > d95138e please send in a patch to do so with a brief write-up of a
>>>>  > follow-up plan.
>>>>
>>>>  Three reports to me are enough. And I obviously could not push the
>>>>  fix out fast enough. So if you want to revert it, here's the patch on
>>>>  maint.
>>
>> Also, can you reference these three reports for future reference?
>
> http://article.gmane.org/gmane.comp.version-control.git/281608
> http://article.gmane.org/gmane.comp.version-control.git/281979
> http://article.gmane.org/gmane.comp.version-control.git/282691
>
> The last one is not confirmed by the reporter yet. But I'm pretty sure
> i'll trigger the special case "when GIT_WORK_TREE is set but GIT_DIR
> is not" in setup code

Thanks, I'll leave these breadcrumbs in the log message for future
reference.

I think the last sentence of the original commit is telling how this
bug came about.  "It does not harm if $GIT_WORK_TREE is set while
$GIT_DIR is not." forgets to consider the possibility that scripts
may be relying on the "Go to the top of the working tree and setting
GIT_DIR would give you a reasonable environment".  That is true if
GIT_WORK_TREE is not set, and these scripts weren't getting the
environment exported [*1*].  These scripts now have to unset
GIT_WORK_TREE themselves (or set it to their $cwd if they are indeed
at the top), just in case the process that calls them exports it
X-<.

Thanks.


[Footnote]

*1* If the end user has GIT_WORK_TREE in the environment, even if
    Git stops exporting it by reverting d95138e, such a script may
    break.  So in that sense, d95138e did not quite change the rule
    of the game for these scripts, but made it more obvious when
    these scripts were written sloppily.

      reply	other threads:[~2015-12-22 21:50 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CA+dzEB=2LJXiLSTqyLw8AeHNwdQicwvEiMg=hVEX0-_s1bySpA@mail.gmail.com>
2015-11-24  2:22 ` Fwd: Git clone fails during pre-commit hook due to GIT_WORK_TREE=. (regression 2.5 -> 2.6) Anthony Sottile
2015-11-24 17:57   ` Stefan Beller
2015-11-25 20:13     ` Duy Nguyen
2015-11-30 19:01       ` Duy Nguyen
2015-11-30 20:16         ` Junio C Hamano
2015-12-01 17:59           ` Duy Nguyen
2015-12-02 17:09             ` Junio C Hamano
2015-12-03 18:17               ` [PATCH 1/2] git.c: make it clear save_env() is for alias handling only Nguyễn Thái Ngọc Duy
2015-12-03 18:17                 ` [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when Nguyễn Thái Ngọc Duy
2015-12-04 20:35                   ` Junio C Hamano
2015-12-05  5:48                     ` Duy Nguyen
2015-12-05 15:32                   ` [PATCH 3/2] git.c: make sure we do not leak GIT_* to alias scripts Nguyễn Thái Ngọc Duy
2015-12-07 18:54                     ` Junio C Hamano
2015-12-08 16:55                       ` Duy Nguyen
2015-12-08 17:20                         ` Jeff King
2015-12-08 23:55                           ` Junio C Hamano
2015-12-05 19:12                   ` [PATCH 2/2] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when Duy Nguyen
2015-12-07 18:33                     ` Junio C Hamano
2015-12-20  7:50                 ` [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias Nguyễn Thái Ngọc Duy
2015-12-20  7:50                   ` [PATCH v2 1/3] git.c: make it clear save_env() is for alias handling only Nguyễn Thái Ngọc Duy
2015-12-20  7:50                   ` [PATCH v2 2/3] setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when Nguyễn Thái Ngọc Duy
2015-12-20  7:50                   ` [PATCH v2 3/3] git.c: make sure we do not leak GIT_* to alias scripts Nguyễn Thái Ngọc Duy
2015-12-21 21:18                   ` [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias Junio C Hamano
2015-12-22 10:57                     ` Duy Nguyen
2015-12-22 11:53                       ` Duy Nguyen
2015-12-22 18:13                         ` Junio C Hamano
2015-12-23  9:37                           ` Jeff King
2015-12-23 10:20                             ` Duy Nguyen
2015-12-23 16:17                             ` Eric Sunshine
2015-12-23 20:37                             ` Johannes Sixt
2015-12-23 21:31                               ` Jeff King
2015-12-24  9:35                                 ` Duy Nguyen
2015-12-29  8:12                                   ` Jeff King
2015-12-29 21:34                                     ` Junio C Hamano
2015-12-21 10:22               ` [PATCH] Revert "setup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR" Nguyễn Thái Ngọc Duy
2015-12-21 17:28                 ` Junio C Hamano
2015-12-21 18:31                   ` Junio C Hamano
2015-12-22  1:06                     ` Duy Nguyen
2015-12-22 21:50                       ` Junio C Hamano [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqk2o6cfvv.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).