Re: [PATCH 1/2] Fixes _is_git
On Wed, Feb 4, 2015 at 4:52 AM, Rémi Rampin wrote: > 2015-02-02 12:24 UTC-05:00, Remi Rampin : >>> proc _is_git {path} { >>> + if {[file isfile $path]} { >>> + set fp [open $path r] >>> + gets $fp line >>> + close $fp >>> + if {[regexp "^gitdir: (.+)$" $line line link_target]} { > > 2015-02-03 3:44 UTC-05:00, Chris Packham : >> It might be simpler to use one of the 'string' commands e.g. string >> wordend "gitdir: " I also suspect the string functions would be faster >> than regexp but that probably doesn't matter. > > I want to check that the file actually begins with "gitdir: " and then > extract the path, so I'm not sure if using string functions is that > simple/fast. Makes sense. > >>> + return [_is_git [file join [file dirname $path] >>> $link_target]] > >> Do we want to avoid pathological cases of infinite recursion? Someone >> would have to maliciously create such a situation. > > Limiting the recursion is very simple, but I'm not sure people are > supposed to stumble on that. More importantly this probably calls for a > different error message, thus a new error result that I am not ready to > implement. But it could be another patch. > But I suppose I can add a simple "return 0" limit to the recursion if > needed, let me know. It'd have to be fairly intentional to cause any real problems. The one thing I was thinking was to factor out the part that checks for HEAD info objects etc into a __is_git that _is_git could call thus eliminating recursion but I don't see it really being anything more than a theoretical issue. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Fixes _is_git
2015-02-02 12:24 UTC-05:00, Remi Rampin : >> proc _is_git {path} { >> + if {[file isfile $path]} { >> + set fp [open $path r] >> + gets $fp line >> + close $fp >> + if {[regexp "^gitdir: (.+)$" $line line link_target]} { 2015-02-03 3:44 UTC-05:00, Chris Packham : > It might be simpler to use one of the 'string' commands e.g. string > wordend "gitdir: " I also suspect the string functions would be faster > than regexp but that probably doesn't matter. I want to check that the file actually begins with "gitdir: " and then extract the path, so I'm not sure if using string functions is that simple/fast. >> + return [_is_git [file join [file dirname $path] >> $link_target]] > Do we want to avoid pathological cases of infinite recursion? Someone > would have to maliciously create such a situation. Limiting the recursion is very simple, but I'm not sure people are supposed to stumble on that. More importantly this probably calls for a different error message, thus a new error result that I am not ready to implement. But it could be another patch. But I suppose I can add a simple "return 0" limit to the recursion if needed, let me know. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Fixes _is_git
Hi Remi, Added Pat Thoyts the git-gui maintainer. (Disclaimer, it's been years since I did anything with Tcl). On Tue, Feb 3, 2015 at 6:24 AM, Remi Rampin wrote: > Function _git_dir would previously fail to accept a "gitdir: ..." file > as a valid Git repository. > --- > lib/choose_repository.tcl | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl > index 92d6022..49ff641 100644 > --- a/lib/choose_repository.tcl > +++ b/lib/choose_repository.tcl > @@ -339,6 +339,16 @@ method _git_init {} { > } > > proc _is_git {path} { > + if {[file isfile $path]} { > + set fp [open $path r] > + gets $fp line > + close $fp > + if {[regexp "^gitdir: (.+)$" $line line link_target]} { It might be simpler to use one of the 'string' commands e.g. string wordend "gitdir: " I also suspect the string functions would be faster than regexp but that probably doesn't matter. > + return [_is_git [file join [file dirname $path] > $link_target]] Do we want to avoid pathological cases of infinite recursion? Someone would have to maliciously create such a situation. > + } > + return 0 > + } > + > if {[file exists [file join $path HEAD]] > && [file exists [file join $path objects]] > && [file exists [file join $path config]]} { > -- > 1.9.5.msysgit.0 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Fixes _is_git
Function _git_dir would previously fail to accept a "gitdir: ..." file as a valid Git repository. --- lib/choose_repository.tcl | 10 ++ 1 file changed, 10 insertions(+) diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl index 92d6022..49ff641 100644 --- a/lib/choose_repository.tcl +++ b/lib/choose_repository.tcl @@ -339,6 +339,16 @@ method _git_init {} { } proc _is_git {path} { + if {[file isfile $path]} { + set fp [open $path r] + gets $fp line + close $fp + if {[regexp "^gitdir: (.+)$" $line line link_target]} { + return [_is_git [file join [file dirname $path] $link_target]] + } + return 0 + } + if {[file exists [file join $path HEAD]] && [file exists [file join $path objects]] && [file exists [file join $path config]]} { -- 1.9.5.msysgit.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html