| Message ID | 2f55d6fb2a158c5b26b93ddb9c144ce1af5d9c32.1570534405.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-gui: respect core.hooksPath, falling back to .git/hooks | expand |
Hi Johannes, Thanks for the re-roll. Some comments below... On 08/10/19 04:33AM, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin <Johannes.Schindelin@gmx.de> > > Since v2.9.0, Git knows about the config variable core.hookspath that > allows overriding the path to the directory containing the Git hooks. > > Since v2.10.0, the `--git-path` option respects that config variable, > too, so we may just as well use that command. > > Other paths inside `.git` are equally subject to differ from > `<gitdir>/<path>`, i.e. inside worktrees, where _some_ paths live in the > "gitdir" and some live in the "commondir" (i.e. the "gitdir" of the main > worktree). > > For Git versions older than v2.5.0 (which was the first version to > support the `--git-path` option for the `rev-parse` command), we simply > fall back to the previous code. > > An original patch handled only the hooksPath setting, however, during > the code submission it was deemed better to fix all call to the `gitdir` > function. > > To avoid spawning a gazillion `git rev-parse --git-path` instances, we > cache the returned paths, priming the cache upon startup in a single > `git rev-parse invocation` with some paths (that have been > determined via a typical startup and via grepping the source code for > calls to the `gitdir` function). > > This fixes https://github.com/git-for-windows/git/issues/1755 > > Initial-patch-by: Philipp Gortan <philipp@gortan.org> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > git-gui.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 58 insertions(+), 4 deletions(-) > > diff --git a/git-gui.sh b/git-gui.sh > index fd476b6999..c684dc7ae1 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -158,6 +158,7 @@ if {[tk windowingsystem] eq "aqua"} { > > set _appname {Git Gui} > set _gitdir {} > +array set _gitdir_cache {} > set _gitworktree {} > set _isbare {} > set _gitexec {} > @@ -197,12 +198,59 @@ proc appname {} { > return $_appname > } > > +proc prime_gitdir_cache {} { > + global _gitdir _gitdir_cache > + > + set gitdir_cmd [list git rev-parse --git-dir] > + > + # `--git-path` is only supported since Git v2.5.0 > + if {[package vcompare $::_git_version 2.5.0] >= 0} { > + # This list was generated from a typical startup as well as from > + # grepping through Git GUI's source code. > + set gitdir_keys [list \ > + CHERRY_PICK_HEAD FETCH_HEAD GITGUI_BCK GITGUI_EDITMSG \ > + GITGUI_MSG HEAD hooks hooks/prepare-commit-msg \ > + index.lock info info/exclude logs MERGE_HEAD MERGE_MSG \ > + MERGE_RR objects "objects/4\[0-1\]/*" \ > + "objects/4\[0-3\]/*" objects/info \ > + objects/info/alternates objects/pack packed-refs \ > + PREPARE_COMMIT_MSG rebase-merge/head-name remotes \ > + rr-cache rr-cache/MERGE_RR SQUASH_MSG \ > + ] > + > + foreach key $gitdir_keys { > + lappend gitdir_cmd --git-path $key > + } > + } > + > + set i -1 > + foreach path [split [eval $gitdir_cmd] "\n"] { > + if {$i eq -1} { > + set _gitdir $path > + } else { > + set _gitdir_cache([lindex $gitdir_keys $i]) $path > + } > + incr i > + } > +} > + > proc gitdir {args} { > - global _gitdir > + global _gitdir _gitdir_cache > + > if {$args eq {}} { > return $_gitdir > } > - return [eval [list file join $_gitdir] $args] > + > + set args [eval [list file join] $args] > + if {![info exists _gitdir_cache($args)]} { > + if {[package vcompare $::_git_version 2.5.0] >= 0} { > + set _gitdir_cache($args) [git rev-parse --git-path $args] > + } else { > + set _gitdir_cache($args) [file join $_gitdir $args] > + } > + } > + > + return $_gitdir_cache($args) > } > > proc gitexec {args} { > @@ -1242,7 +1290,7 @@ if {[catch { > && [catch { > # beware that from the .git dir this sets _gitdir to . > # and _prefix to the empty string > - set _gitdir [git rev-parse --git-dir] > + prime_gitdir_cache > set _prefix [git rev-parse --show-prefix] > } err]} { > load_config 1 Looks good till here. > @@ -1453,10 +1501,16 @@ proc rescan {after {honor_trustmtime 1}} { > global HEAD PARENT MERGE_HEAD commit_type > global ui_index ui_workdir ui_comm > global rescan_active file_states > - global repo_config > + global repo_config _gitdir_cache > > if {$rescan_active > 0 || ![lock_index read]} return > > + # Only re-prime gitdir cache on a full rescan > + if {$after ne "ui_ready"} { What do you mean by a "full rescan"? I assume you use it as the differentiator between `ui_do_rescan` (called when you hit F5 or choose rescan from the menu) and `do_rescan` (called when you revert a line or hunk), and a "full rescan" refers to `ui_do_rescan`. Well in that case, this check is incorrect. `do_rescan` passes only "ui_ready" and `ui_do_rescan` passes "force_first_diff ui_ready". But either way, I'm not a big fan of this. This check makes assumptions about the behaviour of its callers based on what they pass to $after. The way I see it, $after should be a black box to `rescan`, and it should make absolutely no assumptions about it. Doing it this way is really brittle, and would break as soon as someone changes the behaviour of `ui_do_rescan`. If someone in the future passes a different value in $after, this would stop working as intended and would not refresh the cached list on a rescan. So, I think a better place for this if statement would be in `ui_do_rescan`. This would mean adding a new function that does this. But if we unset _gitdir_cache in prime_gitdir_cache (I see no reason not to), we can get away with just something like: proc ui_do_rescan {} { rescan {prime_gitdir_cache; ui_ready} } Though since `prime_gitdir_cache` does not really depend on the rescan being finished, something like this would also work fine: proc ui_do_rescan {} { rescan ui_ready prime_gitdir_cache } This would allow us to do these two things in parallel since `rescan` is asynchronous. But that would also mean it is possible that the status bar would show "Ready" while `prime_gitdir_cache` is still executing. I can't really make up my mind on what is better. I'm inclining on using the latter way, effectively trading a bit of UI inconsistency for performance (at least in theory). Thoughts? > + array unset _gitdir_cache > + prime_gitdir_cache > + } > + > repository_state newType newHEAD newMERGE_HEAD > if {[string match amend* $commit_type] > && $newType eq {normal}
Hi Pratyush, On Sat, 12 Oct 2019, Pratyush Yadav wrote: > On 08/10/19 04:33AM, Johannes Schindelin via GitGitGadget wrote: > > > @@ -1453,10 +1501,16 @@ proc rescan {after {honor_trustmtime 1}} { > > global HEAD PARENT MERGE_HEAD commit_type > > global ui_index ui_workdir ui_comm > > global rescan_active file_states > > - global repo_config > > + global repo_config _gitdir_cache > > > > if {$rescan_active > 0 || ![lock_index read]} return > > > > + # Only re-prime gitdir cache on a full rescan > > + if {$after ne "ui_ready"} { > > What do you mean by a "full rescan"? I assume you use it as the > differentiator between `ui_do_rescan` (called when you hit F5 or choose > rescan from the menu) and `do_rescan` (called when you revert a line or > hunk), and a "full rescan" refers to `ui_do_rescan`. > > Well in that case, this check is incorrect. `do_rescan` passes only > "ui_ready" and `ui_do_rescan` passes "force_first_diff ui_ready". > > But either way, I'm not a big fan of this. This check makes assumptions > about the behaviour of its callers based on what they pass to $after. > The way I see it, $after should be a black box to `rescan`, and it > should make absolutely no assumptions about it. > > Doing it this way is really brittle, and would break as soon as someone > changes the behaviour of `ui_do_rescan`. If someone in the future passes > a different value in $after, this would stop working as intended and > would not refresh the cached list on a rescan. > > So, I think a better place for this if statement would be in > `ui_do_rescan`. This would mean adding a new function that does this. > But if we unset _gitdir_cache in prime_gitdir_cache (I see no reason not > to), we can get away with just something like: > > proc ui_do_rescan {} { > rescan {prime_gitdir_cache; ui_ready} > } > > Though since `prime_gitdir_cache` does not really depend on the rescan > being finished, something like this would also work fine: > > proc ui_do_rescan {} { > rescan ui_ready > prime_gitdir_cache > } That was my first attempt. However, there is a very important piece of code that is even still quoted above: that `if {$rescan_active > 0 || ![lock_index read]} return` part. I do _not_ want to interfere with an actively-going-on rescan. If there is an active one, I don't want to re-prime the `_gitdir` cache. That was the reason why I put the additional code into `rescan` rather than into `ui_do_rescan()`. Ciao, Johannes > > This would allow us to do these two things in parallel since `rescan` is > asynchronous. But that would also mean it is possible that the status > bar would show "Ready" while `prime_gitdir_cache` is still executing. > > I can't really make up my mind on what is better. I'm inclining on using > the latter way, effectively trading a bit of UI inconsistency for > performance (at least in theory). > > Thoughts? > > > + array unset _gitdir_cache > > + prime_gitdir_cache > > + } > > + > > repository_state newType newHEAD newMERGE_HEAD > > if {[string match amend* $commit_type] > > && $newType eq {normal} > > -- > Regards, > Pratyush Yadav >
On 12/10/19 11:24PM, Johannes Schindelin wrote: > Hi Pratyush, > > On Sat, 12 Oct 2019, Pratyush Yadav wrote: > > > On 08/10/19 04:33AM, Johannes Schindelin via GitGitGadget wrote: > > > > > @@ -1453,10 +1501,16 @@ proc rescan {after {honor_trustmtime 1}} { > > > global HEAD PARENT MERGE_HEAD commit_type > > > global ui_index ui_workdir ui_comm > > > global rescan_active file_states > > > - global repo_config > > > + global repo_config _gitdir_cache > > > > > > if {$rescan_active > 0 || ![lock_index read]} return > > > > > > + # Only re-prime gitdir cache on a full rescan > > > + if {$after ne "ui_ready"} { > > > > What do you mean by a "full rescan"? I assume you use it as the > > differentiator between `ui_do_rescan` (called when you hit F5 or choose > > rescan from the menu) and `do_rescan` (called when you revert a line or > > hunk), and a "full rescan" refers to `ui_do_rescan`. > > > > Well in that case, this check is incorrect. `do_rescan` passes only > > "ui_ready" and `ui_do_rescan` passes "force_first_diff ui_ready". > > > > But either way, I'm not a big fan of this. This check makes assumptions > > about the behaviour of its callers based on what they pass to $after. > > The way I see it, $after should be a black box to `rescan`, and it > > should make absolutely no assumptions about it. > > > > Doing it this way is really brittle, and would break as soon as someone > > changes the behaviour of `ui_do_rescan`. If someone in the future passes > > a different value in $after, this would stop working as intended and > > would not refresh the cached list on a rescan. > > > > So, I think a better place for this if statement would be in > > `ui_do_rescan`. This would mean adding a new function that does this. > > But if we unset _gitdir_cache in prime_gitdir_cache (I see no reason not > > to), we can get away with just something like: > > > > proc ui_do_rescan {} { > > rescan {prime_gitdir_cache; ui_ready} > > } > > > > Though since `prime_gitdir_cache` does not really depend on the rescan > > being finished, something like this would also work fine: > > > > proc ui_do_rescan {} { > > rescan ui_ready > > prime_gitdir_cache > > } > > That was my first attempt. However, there is a very important piece of > code that is even still quoted above: that `if {$rescan_active > 0 || > ![lock_index read]} return` part. > > I do _not_ want to interfere with an actively-going-on rescan. If there > is an active one, I don't want to re-prime the `_gitdir` cache. Good catch! In that case I suppose refreshing the cache in $after would be the way to go (IOW, the former of my two suggestions). Anything $after won't get executed if we return early from that check. > That was the reason why I put the additional code into `rescan` rather > than into `ui_do_rescan()`. > > Ciao, > Johannes > > > > > This would allow us to do these two things in parallel since `rescan` is > > asynchronous. But that would also mean it is possible that the status > > bar would show "Ready" while `prime_gitdir_cache` is still executing. > > > > I can't really make up my mind on what is better. I'm inclining on using > > the latter way, effectively trading a bit of UI inconsistency for > > performance (at least in theory). > > > > Thoughts? > > > > > + array unset _gitdir_cache > > > + prime_gitdir_cache > > > + } > > > + > > > repository_state newType newHEAD newMERGE_HEAD > > > if {[string match amend* $commit_type] > > > && $newType eq {normal}
Hi Pratyush, On Mon, 14 Oct 2019, Pratyush Yadav wrote: > On 12/10/19 11:24PM, Johannes Schindelin wrote: > > Hi Pratyush, > > > > On Sat, 12 Oct 2019, Pratyush Yadav wrote: > > > > > On 08/10/19 04:33AM, Johannes Schindelin via GitGitGadget wrote: > > > > > > > @@ -1453,10 +1501,16 @@ proc rescan {after {honor_trustmtime 1}} { > > > > global HEAD PARENT MERGE_HEAD commit_type > > > > global ui_index ui_workdir ui_comm > > > > global rescan_active file_states > > > > - global repo_config > > > > + global repo_config _gitdir_cache > > > > > > > > if {$rescan_active > 0 || ![lock_index read]} return > > > > > > > > + # Only re-prime gitdir cache on a full rescan > > > > + if {$after ne "ui_ready"} { > > > > > > What do you mean by a "full rescan"? I assume you use it as the > > > differentiator between `ui_do_rescan` (called when you hit F5 or choose > > > rescan from the menu) and `do_rescan` (called when you revert a line or > > > hunk), and a "full rescan" refers to `ui_do_rescan`. > > > > > > Well in that case, this check is incorrect. `do_rescan` passes only > > > "ui_ready" and `ui_do_rescan` passes "force_first_diff ui_ready". > > > > > > But either way, I'm not a big fan of this. This check makes assumptions > > > about the behaviour of its callers based on what they pass to $after. > > > The way I see it, $after should be a black box to `rescan`, and it > > > should make absolutely no assumptions about it. > > > > > > Doing it this way is really brittle, and would break as soon as someone > > > changes the behaviour of `ui_do_rescan`. If someone in the future passes > > > a different value in $after, this would stop working as intended and > > > would not refresh the cached list on a rescan. > > > > > > So, I think a better place for this if statement would be in > > > `ui_do_rescan`. This would mean adding a new function that does this. > > > But if we unset _gitdir_cache in prime_gitdir_cache (I see no reason not > > > to), we can get away with just something like: > > > > > > proc ui_do_rescan {} { > > > rescan {prime_gitdir_cache; ui_ready} > > > } > > > > > > Though since `prime_gitdir_cache` does not really depend on the rescan > > > being finished, something like this would also work fine: > > > > > > proc ui_do_rescan {} { > > > rescan ui_ready > > > prime_gitdir_cache > > > } > > > > That was my first attempt. However, there is a very important piece of > > code that is even still quoted above: that `if {$rescan_active > 0 || > > ![lock_index read]} return` part. > > > > I do _not_ want to interfere with an actively-going-on rescan. If there > > is an active one, I don't want to re-prime the `_gitdir` cache. > > Good catch! In that case I suppose refreshing the cache in $after would > be the way to go (IOW, the former of my two suggestions). Anything > $after won't get executed if we return early from that check. The obvious problem with that solution is that the `_gitdir` is reset _after_ the rescan. In my solution, it is reset _before_, as I have no idea how often the `_gitdir` values are used during a rescan (and if none of they were, I would like to be very cautious to prepare for a future where any of those `_gitdir` values _are_ used during a rescan). So I am afraid that I find way too serious problems with both of your proposed alternatives. Ciao, Johannes > > > That was the reason why I put the additional code into `rescan` rather > > than into `ui_do_rescan()`. > > > > Ciao, > > Johannes > > > > > > > > This would allow us to do these two things in parallel since `rescan` is > > > asynchronous. But that would also mean it is possible that the status > > > bar would show "Ready" while `prime_gitdir_cache` is still executing. > > > > > > I can't really make up my mind on what is better. I'm inclining on using > > > the latter way, effectively trading a bit of UI inconsistency for > > > performance (at least in theory). > > > > > > Thoughts? > > > > > > > + array unset _gitdir_cache > > > > + prime_gitdir_cache > > > > + } > > > > + > > > > repository_state newType newHEAD newMERGE_HEAD > > > > if {[string match amend* $commit_type] > > > > && $newType eq {normal} > > -- > Regards, > Pratyush Yadav > >
Hi Pratyush, On Mon, 14 Oct 2019, Pratyush Yadav wrote: > On 12/10/19 11:24PM, Johannes Schindelin wrote: > > > > On Sat, 12 Oct 2019, Pratyush Yadav wrote: > > > > > On 08/10/19 04:33AM, Johannes Schindelin via GitGitGadget wrote: > > > > > > > @@ -1453,10 +1501,16 @@ proc rescan {after {honor_trustmtime 1}} { > > > > global HEAD PARENT MERGE_HEAD commit_type > > > > global ui_index ui_workdir ui_comm > > > > global rescan_active file_states > > > > - global repo_config > > > > + global repo_config _gitdir_cache > > > > > > > > if {$rescan_active > 0 || ![lock_index read]} return > > > > > > > > + # Only re-prime gitdir cache on a full rescan > > > > + if {$after ne "ui_ready"} { > > > > > > What do you mean by a "full rescan"? I assume you use it as the > > > differentiator between `ui_do_rescan` (called when you hit F5 or choose > > > rescan from the menu) and `do_rescan` (called when you revert a line or > > > hunk), and a "full rescan" refers to `ui_do_rescan`. > > > > > > Well in that case, this check is incorrect. `do_rescan` passes only > > > "ui_ready" and `ui_do_rescan` passes "force_first_diff ui_ready". > > > > > > But either way, I'm not a big fan of this. This check makes assumptions > > > about the behaviour of its callers based on what they pass to $after. > > > The way I see it, $after should be a black box to `rescan`, and it > > > should make absolutely no assumptions about it. > > > > > > Doing it this way is really brittle, and would break as soon as someone > > > changes the behaviour of `ui_do_rescan`. If someone in the future passes > > > a different value in $after, this would stop working as intended and > > > would not refresh the cached list on a rescan. > > > > > > So, I think a better place for this if statement would be in > > > `ui_do_rescan`. This would mean adding a new function that does this. > > > But if we unset _gitdir_cache in prime_gitdir_cache (I see no reason not > > > to), we can get away with just something like: > > > > > > proc ui_do_rescan {} { > > > rescan {prime_gitdir_cache; ui_ready} > > > } > > > > > > Though since `prime_gitdir_cache` does not really depend on the rescan > > > being finished, something like this would also work fine: > > > > > > proc ui_do_rescan {} { > > > rescan ui_ready > > > prime_gitdir_cache > > > } > > > > That was my first attempt. However, there is a very important piece of > > code that is even still quoted above: that `if {$rescan_active > 0 || > > ![lock_index read]} return` part. > > > > I do _not_ want to interfere with an actively-going-on rescan. If there > > is an active one, I don't want to re-prime the `_gitdir` cache. > > Good catch! In that case I suppose refreshing the cache in $after would > be the way to go (IOW, the former of my two suggestions). Anything > $after won't get executed if we return early from that check. But it also won't get executed before the actual rescan. Which is precisely what I tried to ensure. Ciao, Johannes > > > That was the reason why I put the additional code into `rescan` rather > > than into `ui_do_rescan()`. > > > > Ciao, > > Johannes > > > > > > > > This would allow us to do these two things in parallel since `rescan` is > > > asynchronous. But that would also mean it is possible that the status > > > bar would show "Ready" while `prime_gitdir_cache` is still executing. > > > > > > I can't really make up my mind on what is better. I'm inclining on using > > > the latter way, effectively trading a bit of UI inconsistency for > > > performance (at least in theory). > > > > > > Thoughts? > > > > > > > + array unset _gitdir_cache > > > > + prime_gitdir_cache > > > > + } > > > > + > > > > repository_state newType newHEAD newMERGE_HEAD > > > > if {[string match amend* $commit_type] > > > > && $newType eq {normal} > > -- > Regards, > Pratyush Yadav >
On 14/10/19 12:18AM, Johannes Schindelin wrote: > Hi Pratyush, > > On Mon, 14 Oct 2019, Pratyush Yadav wrote: > > > On 12/10/19 11:24PM, Johannes Schindelin wrote: > > > Hi Pratyush, > > > > > > On Sat, 12 Oct 2019, Pratyush Yadav wrote: > > > > > > > On 08/10/19 04:33AM, Johannes Schindelin via GitGitGadget wrote: > > > > > > > > > @@ -1453,10 +1501,16 @@ proc rescan {after {honor_trustmtime 1}} { > > > > > global HEAD PARENT MERGE_HEAD commit_type > > > > > global ui_index ui_workdir ui_comm > > > > > global rescan_active file_states > > > > > - global repo_config > > > > > + global repo_config _gitdir_cache > > > > > > > > > > if {$rescan_active > 0 || ![lock_index read]} return > > > > > > > > > > + # Only re-prime gitdir cache on a full rescan > > > > > + if {$after ne "ui_ready"} { > > > > > > > > What do you mean by a "full rescan"? I assume you use it as the > > > > differentiator between `ui_do_rescan` (called when you hit F5 or choose > > > > rescan from the menu) and `do_rescan` (called when you revert a line or > > > > hunk), and a "full rescan" refers to `ui_do_rescan`. > > > > > > > > Well in that case, this check is incorrect. `do_rescan` passes only > > > > "ui_ready" and `ui_do_rescan` passes "force_first_diff ui_ready". > > > > > > > > But either way, I'm not a big fan of this. This check makes assumptions > > > > about the behaviour of its callers based on what they pass to $after. > > > > The way I see it, $after should be a black box to `rescan`, and it > > > > should make absolutely no assumptions about it. > > > > > > > > Doing it this way is really brittle, and would break as soon as someone > > > > changes the behaviour of `ui_do_rescan`. If someone in the future passes > > > > a different value in $after, this would stop working as intended and > > > > would not refresh the cached list on a rescan. > > > > > > > > So, I think a better place for this if statement would be in > > > > `ui_do_rescan`. This would mean adding a new function that does this. > > > > But if we unset _gitdir_cache in prime_gitdir_cache (I see no reason not > > > > to), we can get away with just something like: > > > > > > > > proc ui_do_rescan {} { > > > > rescan {prime_gitdir_cache; ui_ready} > > > > } > > > > > > > > Though since `prime_gitdir_cache` does not really depend on the rescan > > > > being finished, something like this would also work fine: > > > > > > > > proc ui_do_rescan {} { > > > > rescan ui_ready > > > > prime_gitdir_cache > > > > } > > > > > > That was my first attempt. However, there is a very important piece of > > > code that is even still quoted above: that `if {$rescan_active > 0 || > > > ![lock_index read]} return` part. > > > > > > I do _not_ want to interfere with an actively-going-on rescan. If there > > > is an active one, I don't want to re-prime the `_gitdir` cache. > > > > Good catch! In that case I suppose refreshing the cache in $after would > > be the way to go (IOW, the former of my two suggestions). Anything > > $after won't get executed if we return early from that check. > > The obvious problem with that solution is that the `_gitdir` is reset > _after_ the rescan. In my solution, it is reset _before_, as I have no > idea how often the `_gitdir` values are used during a rescan (and if > none of they were, I would like to be very cautious to prepare for a > future where any of those `_gitdir` values _are_ used during a rescan). _gitdir values are used quite often during a rescan, so yes, this won't work. > So I am afraid that I find way too serious problems with both of your > proposed alternatives. One alternative I can see right now is adding another optional parameter to `rescan` that controls whether we refresh the gitdir cache or not. That parameter would default to 0/false. I'm not the biggest fan of something like this, but it might be the easiest way to do it given the constraints. I also thought about trying to acquire the index lock in `prime_gitdir_cache`, but that could create a race on the lock between `prime_gitdir_cache` and `rescan`. If you have any better ideas, I'm all ears, but otherwise, I maybe our best bet is to just go with adding an optional parameter to `rescan`.
diff --git a/git-gui.sh b/git-gui.sh index fd476b6999..c684dc7ae1 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -158,6 +158,7 @@ if {[tk windowingsystem] eq "aqua"} { set _appname {Git Gui} set _gitdir {} +array set _gitdir_cache {} set _gitworktree {} set _isbare {} set _gitexec {} @@ -197,12 +198,59 @@ proc appname {} { return $_appname } +proc prime_gitdir_cache {} { + global _gitdir _gitdir_cache + + set gitdir_cmd [list git rev-parse --git-dir] + + # `--git-path` is only supported since Git v2.5.0 + if {[package vcompare $::_git_version 2.5.0] >= 0} { + # This list was generated from a typical startup as well as from + # grepping through Git GUI's source code. + set gitdir_keys [list \ + CHERRY_PICK_HEAD FETCH_HEAD GITGUI_BCK GITGUI_EDITMSG \ + GITGUI_MSG HEAD hooks hooks/prepare-commit-msg \ + index.lock info info/exclude logs MERGE_HEAD MERGE_MSG \ + MERGE_RR objects "objects/4\[0-1\]/*" \ + "objects/4\[0-3\]/*" objects/info \ + objects/info/alternates objects/pack packed-refs \ + PREPARE_COMMIT_MSG rebase-merge/head-name remotes \ + rr-cache rr-cache/MERGE_RR SQUASH_MSG \ + ] + + foreach key $gitdir_keys { + lappend gitdir_cmd --git-path $key + } + } + + set i -1 + foreach path [split [eval $gitdir_cmd] "\n"] { + if {$i eq -1} { + set _gitdir $path + } else { + set _gitdir_cache([lindex $gitdir_keys $i]) $path + } + incr i + } +} + proc gitdir {args} { - global _gitdir + global _gitdir _gitdir_cache + if {$args eq {}} { return $_gitdir } - return [eval [list file join $_gitdir] $args] + + set args [eval [list file join] $args] + if {![info exists _gitdir_cache($args)]} { + if {[package vcompare $::_git_version 2.5.0] >= 0} { + set _gitdir_cache($args) [git rev-parse --git-path $args] + } else { + set _gitdir_cache($args) [file join $_gitdir $args] + } + } + + return $_gitdir_cache($args) } proc gitexec {args} { @@ -1242,7 +1290,7 @@ if {[catch { && [catch { # beware that from the .git dir this sets _gitdir to . # and _prefix to the empty string - set _gitdir [git rev-parse --git-dir] + prime_gitdir_cache set _prefix [git rev-parse --show-prefix] } err]} { load_config 1 @@ -1453,10 +1501,16 @@ proc rescan {after {honor_trustmtime 1}} { global HEAD PARENT MERGE_HEAD commit_type global ui_index ui_workdir ui_comm global rescan_active file_states - global repo_config + global repo_config _gitdir_cache if {$rescan_active > 0 || ![lock_index read]} return + # Only re-prime gitdir cache on a full rescan + if {$after ne "ui_ready"} { + array unset _gitdir_cache + prime_gitdir_cache + } + repository_state newType newHEAD newMERGE_HEAD if {[string match amend* $commit_type] && $newType eq {normal}