Hi,
whenever I read "worktree" I get cautious.
_Very_ cautious.
I mean, _VERY_ cautious. And for a reason.
The whole idea of a worktree was bolted onto Git, so much so that almost
invariably, every attempt at making it work properly, fails.
Honestly, I think it caused us more pain than any other part of Git,
_including_ the nasty bugs in git_config_set().
On Thu, 19 Feb 2009, Giuseppe Bilotta wrote:
> Don't
Personally, I prefer a less colloquial (and while at it, a more concise)
commit message.
> +# _gitdir exists, so try loading the config
> +load_config 0
> +apply_config
> +# try to set work tree from environment, falling back to core.worktree
> +if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
> + set _gitworktree [get_config core.worktree]
> +}
> if {$_prefix ne {}} {
> - regsub -all {[^/]+/} $_prefix ../ cdup
> + if {$_gitworktree eq {}} {
> + regsub -all {[^/]+/} $_prefix ../ cdup
> + } else {
> + set cdup $_gitworktree
> + }
It appears as if you redo a the logic laid out in setup.c. Don't.
Instead, teach rev-parse to output the path of the work tree.
Oh, wait, --show-cdup already shows that...
> @@ -1076,11 +1089,15 @@ if {$_prefix ne {}} {
> error_popup [strcat [mc "Cannot use funny .git directory:"] "\n\n$_gitdir"]
> exit 1
> }
> - if {[catch {cd [file dirname $_gitdir]} err]} {
> + if {$_gitworktree eq {}} {
> + set _gitworktree [file dirname $_gitdir]
> + }
This is certainly wrong in bare repositories.
(If that code is only exercized in non-bare repositories, you failed to
indicate that. The commit message is the place for such comments.)
> + if {[catch {cd $_gitworktree} err]} {
> catch {wm withdraw .}
> - error_popup [strcat [mc "No working directory"] " [file dirname $_gitdir]:\n\n$err"]
> + error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
> exit 1
> }
> + set _gitworktree [pwd]
And why do you have to cd to the worktree? You might not be able to come
back to the GIT_DIR, after all.
> @@ -1895,8 +1913,10 @@ proc do_gitk {revs} {
> }
>
> set pwd [pwd]
> - cd [file dirname [gitdir]]
> - set env(GIT_DIR) [file tail [gitdir]]
> + if { $_gitworktree ne {} } {
> + cd $_gitworktree
> + }
> + set env(GIT_DIR) [file normalize [gitdir]]
>
> eval exec $cmd $revs &
>
Just out of curiosity: what was your method of making sure that you catch
all sites where the worktree is assumed to be $GIT_DIR/..?
That is a most crucial information for the commit message!!!
Besides, have you read about the fallout of making "git rev-parse
--git-dir" showing absolute paths always?
If yes, why do you ignore it in your patch?
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
