From: Junio C Hamano <gitster@pobox.com>
To: Paul Mackerras <paulus@ozlabs.org>,
Eric Sunshine <sunshine@sunshineco.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>, ch <cr@onlinehome.de>,
"Martin von Zweigbergk" <martinvonz@gmail.com>,
"Git List" <git@vger.kernel.org>, "Jeff King" <peff@peff.net>
Subject: [PATCH] gitk: to run in a bare repository (was: gitk can't be run from non-worktree folders)
Date: Thu, 23 Jan 2020 11:20:36 -0800 [thread overview]
Message-ID: <xmqqk15i3rp7.fsf_-_@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <xmqqpnfa3sj1.fsf@gitster-ct.c.googlers.com> (Junio C. Hamano's message of "Thu, 23 Jan 2020 11:02:42 -0800")
[This message is designed to be readable by "git am -c".]
Junio C Hamano <gitster@pobox.com> writes:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> It's a new regression introduced by 2d92ab32fd (rev-parse: make
>> --show-toplevel without a worktree an error, 2019-11-19), as far as I
>> can tell. I have many times used gitk on bare repositories as an
>> interactive replacement for git-log, so this is a unfortunate bit of
>> fallout from that change. That's not to say that 2d92ab32fd should be
>> reverted, though... perhaps gitk itself needs a bit of a fix.
>
> Curious.
>
> There is a "proc gitworktree" that does use --show-toplevel but it
> is fairly conservative and assumes these calls can fail.
>
> ...
> if {[catch {set _gitworktree [exec git rev-parse --show-toplevel]}]} {
> if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
> catch {set _gitworktree [exec git config --get core.worktree]}
> if {$_gitworktree eq ""} {
> set _gitworktree [file normalize ./[exec git rev-parse --show-cdup]]
> }
>
> However, there is this call
>
> set worktree [exec git rev-parse --show-toplevel]
>
> at the top-level of the code. I wonder if the obvious and minimum
> fix would be sufficient, i.e.
>
> -set worktree [exec git rev-parse --show-toplevel]
> +set worktree [gitworktree]
>
> around l.12546
I've read 784b7e2f ("gitk: Fix "External diff" with separate work
tree", 2011-04-04) again and am reasonably convinced that the above
single liner would be "sufficient". The external_diff_get_one_file
proc is written, with or without the "fix external diff" patch, to
work ONLY in a non-bare repository (it used to find and use the
parent directory of ".git" as the top-level of the working tree
before 784b7e2f, which changed it to use $worktree global obtained
by calling "rev-parse --show-toplevel") and does not work in a bare
repository anyway, if I am reading the code correctly.
-- >8 --
Subject: [PATCH] gitk: be prepared to be run in a bare repository
784b7e2f ("gitk: Fix "External diff" with separate work tree",
2011-04-04) added an unconditional call to "git rev-parse
--show-toplevel" to set up a global variable quite early in the
course of the program, so that the location of the working tree can
later be known if/when the user chooses to run the external diff via
the external_diff_get_one_file proc. Before that change, the
external diff code used to assume that the parent directory of ".git"
directory is the top-level of the working tree.
Recent versions of git however notices that "rev-parse --show-toplevel"
executed in a bare repository is an error, which makes gitk stop,
even before the user could attempt to run external diff.
Use the gitworktree helper introduced in 65bb0bda ("gitk: Fix the
display of files when filtered by path", 2011-12-13), which is
prepared to see failures from "rev-parse --show-toplevel" and other
means it tries to find the top-level of the working tree instead to
work around this issue. The resulting value in $worktree global,
when run in a bare repository, is bogus, but the code is not
prepared to run external diff correctly without a working tree
anyway ;-)
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
gitk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk b/gitk
index abe4805ade..1483bf5ed5 100755
--- a/gitk
+++ b/gitk
@@ -12599,7 +12599,7 @@ set cdup {}
if {[expr {[exec git rev-parse --is-inside-work-tree] == "true"}]} {
set cdup [exec git rev-parse --show-cdup]
}
-set worktree [exec git rev-parse --show-toplevel]
+set worktree [gitworktree]
setcoords
makewindow
catch {
next prev parent reply other threads:[~2020-01-23 19:20 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-23 16:04 [REGRESSION] gitk can't be run from non-worktree folders ch
2020-01-23 16:31 ` SZEDER Gábor
2020-01-23 16:36 ` Eric Sunshine
2020-01-23 17:04 ` SZEDER Gábor
2020-01-23 17:46 ` SZEDER Gábor
2020-01-23 19:02 ` Junio C Hamano
2020-01-23 19:20 ` Junio C Hamano [this message]
2020-01-23 19:26 ` [PATCH] gitk: to run in a bare repository (was: gitk can't be run from non-worktree folders) Eric Sunshine
2020-01-23 19:27 ` Jeff King
2020-03-30 15:20 ` ch
2020-10-11 5:08 ` Eric Sunshine
2020-10-13 14:25 ` [PATCH] gitk: to run in a bare repository ch
2020-10-13 15:31 ` Pratyush Yadav
2020-04-02 13:40 ` [REGRESSION] gitk can't be run from non-worktree folders ch
2020-04-02 17:47 ` 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=xmqqk15i3rp7.fsf_-_@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=cr@onlinehome.de \
--cc=git@vger.kernel.org \
--cc=martinvonz@gmail.com \
--cc=paulus@ozlabs.org \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
--cc=szeder.dev@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.
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).