On Sun, 2 May 2010, Frank Terbeck wrote: > Simon Ruderich wrote: > > On Sat, May 01, 2010 at 02:17:19PM +0200, Frank Terbeck wrote: > >>> I would prefer mine because of the speed penalty of Benjamin's > >>> patch. Checking if the file exists in .git would be another > >>> possibility. > >> > >> And that would be the one from workers-27813¹? > >> > >> Regards, Frank > >> > >> ¹ > > > > Ah sorry. Yes, that's the one. > > Committed this one. Actually checking whether the head symbols are > available at any time probably requires something like this: > > gdir="$(git rev-parse --git-dir)" > [[ -f ${gdir}/ORIG_HEAD ]] && ... > [[ -f ${gdir}/FETCH_HEAD ]] && ... > ... > > I don't know if it's worth the additional fork. That's essentially what my 'compromise' patch does. The one from workers-27817 [links below]. ==== this portion: ==== + symbolic_heads=() + gitdir=$(_call_program gitdir git rev-parse --git-dir 2>/dev/null) + __git_command_successful && symbolic_heads=( $gitdir/{,{ORIG,FETCH,MERGE}_}HEAD(N:t) ) ======================= As stated before, I think the fork is worth it. The three options were: [27813]. always-complete: usually-incorrect, 0 extra forks [27817]. --git-dir + filesystem tests: possibly-incorrect, 1 extra fork [27814]. rev-parse each name: always correct, 4 extra forks Personally, I think the 4 extra forks are worth it (The particular execution path that this alters already does two _call_program git forks). But, I'm not stuck on a system where forking is expensive (100 git forks takes ~1sec)[2], so I see the need for the compromise [27817]. [27813] is usually incorrect (depending on your workflow) because FETCH_HEAD, ORIG_HEAD, and MERGE_HEAD come and go depending on what actions you take on your repository. The reason [27817] is possibly-incorrect is that the .git/*HEAD files can exist even if the branch hasn't been born. (e.g. try 'git rev-parse HEAD' in a newly-init'ed repo, even though .git/HEAD exists) But, it's rare enough that the correctness vs. speed tradeoff seems reasonable. -- Best, Ben [27813] http://www.zsh.org/mla/workers/2010/msg00265.html [27814] http://www.zsh.org/mla/workers/2010/msg00266.html [27817] http://www.zsh.org/mla/workers/2010/msg00269.html [2] $ date ; r=( HEAD ORIG_HEAD --git-dir ) ; for l in {1..100} ; git rev-parse $r[$(( 1 + $RANDOM % $#r ))] 2>/dev/null >/dev/null ; date Mon May 3 10:50:06 EDT 2010 Mon May 3 10:50:07 EDT 2010