{"thread":{"id":"38474","subject":"[git-gui] bug report: \"Open existing repository\" dialog fails on submodules","startedAt":"2015-01-30T21:46:08Z","lastAt":"2015-02-05T16:20:15Z","messageCount":14,"participants":["Rémi Rampin","Chris Packham","Remi Rampin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"255454","messageId":"CAMto89Dvz-u+at4CPLGQRak4niOJk1trSCn2wQugLUnD1h=Fjw@mail.gmail.com","threadId":"38474","inReplyTo":null,"subject":"[git-gui] bug report: \"Open existing repository\" dialog fails on submodules","fromName":"Rémi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-01-30T21:46:08Z","receivedAt":"2015-01-30T21:46:08Z","isPatch":false,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"Hi,\n\nThis bug report concerns git-gui. Apologies if this is not the right\nmailing-list.\n\nBy submodule I mean a repository for which .git is not a regular Git\ndirectory, but rather a \"gitdir: ...\" file.\nWhile running \"git gui\" from such a directory will work fine, trying\nto open it from the choose_repository window will fail with \"Not a Git\nrepository\". This is because of the simplistic implementation of proc\n_is_git in lib/choose_repository.tcl.\n\nI suggest fixing that function, or using Git directly to perform that\ncheck, for instance checking \"git rev-parse --show-toplevel\". I'd\nattempt a patch but my tcl-fu is weak.\n\nBest\n-- \nRémi Rampin\n"},{"id":"255506","messageId":"CAFOYHZBpVf0Dk=aM3hbpVjwc-f_WtZx+Myaja6=V2KXCDijsQA@mail.gmail.com","threadId":"38474","inReplyTo":"CAMto89Dvz-u+at4CPLGQRak4niOJk1trSCn2wQugLUnD1h=Fjw@mail.gmail.com","subject":"Re: [git-gui] bug report: \"Open existing repository\" dialog fails on submodules","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2015-02-02T08:41:16Z","receivedAt":"2015-02-02T08:41:16Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Hi,\n\nOn Sat, Jan 31, 2015 at 10:46 AM, Rémi Rampin <remirampin@gmail.com> wrote:\n> Hi,\n>\n> This bug report concerns git-gui. Apologies if this is not the right\n> mailing-list.\n>\n> By submodule I mean a repository for which .git is not a regular Git\n> directory, but rather a \"gitdir: ...\" file.\n> While running \"git gui\" from such a directory will work fine, trying\n> to open it from the choose_repository window will fail with \"Not a Git\n> repository\". This is because of the simplistic implementation of proc\n> _is_git in lib/choose_repository.tcl.\n>\n> I suggest fixing that function, or using Git directly to perform that\n> check, for instance checking \"git rev-parse --show-toplevel\". I'd\n> attempt a patch but my tcl-fu is weak.\n>\n\nI would have thought the following would work\n\n--- 8< ---\nSubject: [PATCH] git-gui: use git rev-parse to validate paths\n\nThe current _is_git function to validate a path as a git repository does\nnot handle a gitfiles which have been used for submodules for some time.\nInstead of using a custom function let's just ask git rev-parse.\n\nSigned-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>\n---\n lib/choose_repository.tcl | 15 ++++-----------\n 1 file changed, 4 insertions(+), 11 deletions(-)\n\ndiff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\nindex 92d6022..944ab50 100644\n--- a/lib/choose_repository.tcl\n+++ b/lib/choose_repository.tcl\n@@ -339,19 +339,12 @@ method _git_init {} {\n }\n\n proc _is_git {path} {\n-       if {[file exists [file join $path HEAD]]\n-        && [file exists [file join $path objects]]\n-        && [file exists [file join $path config]]} {\n+       puts $path\n+       if {[catch {exec git rev-parse --resolve-git-dir $path}]} {\n+               return 0\n+       } else {\n                return 1\n        }\n-       if {[is_Cygwin]} {\n-               if {[file exists [file join $path HEAD]]\n-                && [file exists [file join $path objects.lnk]]\n-                && [file exists [file join $path config.lnk]]} {\n-                       return 1\n-               }\n-       }\n-       return 0\n }\n\n proc _objdir {path} {\n-- \n2.3.0.rc2\n--- >8 ---\n\nBut it actually looks like git rev-parse --resolve-git-dir $path needs\nto be run inside a git repository _any_ git repository, which seems a\nbit backwards to me.\n\n  $ cd\n  $ git rev-parse --resolve-git-dir ~/src/git/.git\n  fatal: Not a git repository (or any parent up to mount point /home)\n  Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).\n\n  $ cd ~/src/git\n  $ git rev-parse --resolve-git-dir ~/src/git-gui/.git\n  /home/chrisp/src/git-gui/.git\n\nSo one potential fix git a gui-gui bug, one new(?) bug in git rev-parse.\n"},{"id":"255507","messageId":"CAFOYHZAz9LOsAG53GbxQtb3Hp4d_zMFCKbp_Z5NViwCCm+cLAg@mail.gmail.com","threadId":"38474","inReplyTo":"CAFOYHZBpVf0Dk=aM3hbpVjwc-f_WtZx+Myaja6=V2KXCDijsQA@mail.gmail.com","subject":"Re: [git-gui] bug report: \"Open existing repository\" dialog fails on submodules","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2015-02-02T08:43:08Z","receivedAt":"2015-02-02T08:43:08Z","isPatch":false,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Mon, Feb 2, 2015 at 9:41 PM, Chris Packham <judge.packham@gmail.com> wrote:\n> Hi,\n>\n> On Sat, Jan 31, 2015 at 10:46 AM, Rémi Rampin <remirampin@gmail.com> wrote:\n>> Hi,\n>>\n>> This bug report concerns git-gui. Apologies if this is not the right\n>> mailing-list.\n>>\n>> By submodule I mean a repository for which .git is not a regular Git\n>> directory, but rather a \"gitdir: ...\" file.\n>> While running \"git gui\" from such a directory will work fine, trying\n>> to open it from the choose_repository window will fail with \"Not a Git\n>> repository\". This is because of the simplistic implementation of proc\n>> _is_git in lib/choose_repository.tcl.\n>>\n>> I suggest fixing that function, or using Git directly to perform that\n>> check, for instance checking \"git rev-parse --show-toplevel\". I'd\n>> attempt a patch but my tcl-fu is weak.\n>>\n>\n> I would have thought the following would work\n>\n> --- 8< ---\n> Subject: [PATCH] git-gui: use git rev-parse to validate paths\n>\n> The current _is_git function to validate a path as a git repository does\n> not handle a gitfiles which have been used for submodules for some time.\n> Instead of using a custom function let's just ask git rev-parse.\n>\n> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>\n> ---\n>  lib/choose_repository.tcl | 15 ++++-----------\n>  1 file changed, 4 insertions(+), 11 deletions(-)\n>\n> diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\n> index 92d6022..944ab50 100644\n> --- a/lib/choose_repository.tcl\n> +++ b/lib/choose_repository.tcl\n> @@ -339,19 +339,12 @@ method _git_init {} {\n>  }\n>\n>  proc _is_git {path} {\n> -       if {[file exists [file join $path HEAD]]\n> -        && [file exists [file join $path objects]]\n> -        && [file exists [file join $path config]]} {\n> +       puts $path\n> +       if {[catch {exec git rev-parse --resolve-git-dir $path}]} {\n> +               return 0\n> +       } else {\n>                 return 1\n>         }\n> -       if {[is_Cygwin]} {\n> -               if {[file exists [file join $path HEAD]]\n> -                && [file exists [file join $path objects.lnk]]\n> -                && [file exists [file join $path config.lnk]]} {\n> -                       return 1\n> -               }\n> -       }\n> -       return 0\n>  }\n>\n>  proc _objdir {path} {\n> --\n> 2.3.0.rc2\n> --- >8 ---\n>\n> But it actually looks like git rev-parse --resolve-git-dir $path needs\n> to be run inside a git repository _any_ git repository, which seems a\n> bit backwards to me.\n>\n>   $ cd\n>   $ git rev-parse --resolve-git-dir ~/src/git/.git\n>   fatal: Not a git repository (or any parent up to mount point /home)\n>   Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).\n>\n>   $ cd ~/src/git\n>   $ git rev-parse --resolve-git-dir ~/src/git-gui/.git\n>   /home/chrisp/src/git-gui/.git\n>\n> So one potential fix git a gui-gui bug, one new(?) bug in git rev-parse.\n\nNot a new one. Happens in 1.9.1. Still a bit counter-intuitive IMO.\n"},{"id":"255512","messageId":"CAMto89CHf4OT_S05SaRrVRZvF-PH2_6DrcEpdGiUfaRGutJQHw@mail.gmail.com","threadId":"38474","inReplyTo":"CAFOYHZBpVf0Dk=aM3hbpVjwc-f_WtZx+Myaja6=V2KXCDijsQA@mail.gmail.com","subject":"Re: [git-gui] bug report: \"Open existing repository\" dialog fails on submodules","fromName":"Rémi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-02T15:59:41Z","receivedAt":"2015-02-02T15:59:41Z","isPatch":false,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"2015-02-02 3:41 UTC-05:00, Chris Packham <judge.packham@gmail.com>:\n> [...]\n> But it actually looks like git rev-parse --resolve-git-dir $path needs\n> to be run inside a git repository _any_ git repository, which seems a\n> bit backwards to me.\n> [...]\n\nIndeed, looking at git-rev-parse(1), the correct option might be\n--show-toplevel, which will print the cwd if it is the top-level of a\nnon-bare repository:\n\n    cd $candidate && test $(git rev-parse --show-toplevel) = $candidate\n\nor\n\n    test $(git --git-dir=$candidate rev-parse --show-toplevel) = $candidate\n\nOf course Git will resolve symlinks at this point, so $candidate has\nto be resolved first for the equality to make sense.\n\nOther solution is to parse the \"gitdir: ...\" format and recurse, which\nis not exactly hard (provided you speak Tcl).\n"},{"id":"255516","messageId":"1422897883-11036-1-git-send-email-remirampin@gmail.com","threadId":"38474","inReplyTo":"CAMto89CHf4OT_S05SaRrVRZvF-PH2_6DrcEpdGiUfaRGutJQHw@mail.gmail.com","subject":"[PATCH 1/2] Fixes _is_git","fromName":"Remi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-02T17:24:42Z","receivedAt":"2015-02-02T17:24:42Z","isPatch":true,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"Function _git_dir would previously fail to accept a \"gitdir: ...\" file\nas a valid Git repository.\n---\n lib/choose_repository.tcl | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\nindex 92d6022..49ff641 100644\n--- a/lib/choose_repository.tcl\n+++ b/lib/choose_repository.tcl\n@@ -339,6 +339,16 @@ method _git_init {} {\n }\n \n proc _is_git {path} {\n+\tif {[file isfile $path]} {\n+\t\tset fp [open $path r]\n+\t\tgets $fp line\n+\t\tclose $fp\n+\t\tif {[regexp \"^gitdir: (.+)$\" $line line link_target]} {\n+\t\t\treturn [_is_git [file join [file dirname $path] $link_target]]\n+\t\t}\n+\t\treturn 0\n+\t}\n+\n \tif {[file exists [file join $path HEAD]]\n \t && [file exists [file join $path objects]]\n \t && [file exists [file join $path config]]} {\n-- \n1.9.5.msysgit.0\n"},{"id":"255517","messageId":"1422897883-11036-2-git-send-email-remirampin@gmail.com","threadId":"38474","inReplyTo":"1422897883-11036-1-git-send-email-remirampin@gmail.com","subject":"[PATCH 2/2] Makes _do_open2 set _gitdir to actual path","fromName":"Remi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-02T17:24:43Z","receivedAt":"2015-02-02T17:24:43Z","isPatch":true,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"If _is_git had to follow \"gitdir: ...\" files to reach the actual Git\ndirectory, we set _gitdir to that final path.\n---\n lib/choose_repository.tcl | 14 ++++++++++----\n 1 file changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\nindex 49ff641..641068d 100644\n--- a/lib/choose_repository.tcl\n+++ b/lib/choose_repository.tcl\n@@ -338,13 +338,17 @@ method _git_init {} {\n \treturn 1\n }\n \n-proc _is_git {path} {\n+proc _is_git {path {outdir_var \"\"}} {\n+\tif {$outdir_var ne \"\"} {\n+\t\tupvar 1 $outdir_var outdir\n+\t}\n \tif {[file isfile $path]} {\n \t\tset fp [open $path r]\n \t\tgets $fp line\n \t\tclose $fp\n \t\tif {[regexp \"^gitdir: (.+)$\" $line line link_target]} {\n-\t\t\treturn [_is_git [file join [file dirname $path] $link_target]]\n+\t\t\tset link_target_abs [file join [file dirname $path] $link_target]\n+\t\t\treturn [_is_git $link_target_abs outdir]\n \t\t}\n \t\treturn 0\n \t}\n@@ -352,12 +356,14 @@ proc _is_git {path} {\n \tif {[file exists [file join $path HEAD]]\n \t && [file exists [file join $path objects]]\n \t && [file exists [file join $path config]]} {\n+\t\tset outdir $path\n \t\treturn 1\n \t}\n \tif {[is_Cygwin]} {\n \t\tif {[file exists [file join $path HEAD]]\n \t\t && [file exists [file join $path objects.lnk]]\n \t\t && [file exists [file join $path config.lnk]]} {\n+\t\t\tset outdir $path\n \t\t\treturn 1\n \t\t}\n \t}\n@@ -1103,7 +1109,7 @@ method _open_local_path {} {\n }\n \n method _do_open2 {} {\n-\tif {![_is_git [file join $local_path .git]]} {\n+\tif {![_is_git [file join $local_path .git] actualgit]} {\n \t\terror_popup [mc \"Not a Git repository: %s\" [file tail $local_path]]\n \t\treturn\n \t}\n@@ -1116,7 +1122,7 @@ method _do_open2 {} {\n \t}\n \n \t_append_recentrepos [pwd]\n-\tset ::_gitdir .git\n+\tset ::_gitdir $actualgit\n \tset ::_prefix {}\n \tset done 1\n }\n-- \n1.9.5.msysgit.0\n"},{"id":"255565","messageId":"CAFOYHZBHoXC34gBu_Lx347f=-uUcVM1nHYT87SzxfeMa=KdFgw@mail.gmail.com","threadId":"38474","inReplyTo":"1422897883-11036-1-git-send-email-remirampin@gmail.com","subject":"Re: [PATCH 1/2] Fixes _is_git","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2015-02-03T08:44:33Z","receivedAt":"2015-02-03T08:44:33Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Hi Remi,\n\nAdded Pat Thoyts the git-gui maintainer.\n\n(Disclaimer, it's been years since I did anything with Tcl).\n\nOn Tue, Feb 3, 2015 at 6:24 AM, Remi Rampin <remirampin@gmail.com> wrote:\n> Function _git_dir would previously fail to accept a \"gitdir: ...\" file\n> as a valid Git repository.\n> ---\n>  lib/choose_repository.tcl | 10 ++++++++++\n>  1 file changed, 10 insertions(+)\n>\n> diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\n> index 92d6022..49ff641 100644\n> --- a/lib/choose_repository.tcl\n> +++ b/lib/choose_repository.tcl\n> @@ -339,6 +339,16 @@ method _git_init {} {\n>  }\n>\n>  proc _is_git {path} {\n> +       if {[file isfile $path]} {\n> +               set fp [open $path r]\n> +               gets $fp line\n> +               close $fp\n> +               if {[regexp \"^gitdir: (.+)$\" $line line link_target]} {\n\nIt might be simpler to use one of the 'string' commands e.g. string\nwordend \"gitdir: \" I also suspect the string functions would be faster\nthan regexp but that probably doesn't matter.\n\n> +                       return [_is_git [file join [file dirname $path] $link_target]]\n\nDo we want to avoid pathological cases of infinite recursion? Someone\nwould have to maliciously create such a situation.\n\n> +               }\n> +               return 0\n> +       }\n> +\n>         if {[file exists [file join $path HEAD]]\n>          && [file exists [file join $path objects]]\n>          && [file exists [file join $path config]]} {\n> --\n> 1.9.5.msysgit.0\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"255566","messageId":"CAFOYHZB_c3U9jpAq=jrGgMU+wMMf8w5D9iqLC9ccGC8S3hhXZg@mail.gmail.com","threadId":"38474","inReplyTo":"1422897883-11036-2-git-send-email-remirampin@gmail.com","subject":"Re: [PATCH 2/2] Makes _do_open2 set _gitdir to actual path","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2015-02-03T08:51:31Z","receivedAt":"2015-02-03T08:51:31Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Tue, Feb 3, 2015 at 6:24 AM, Remi Rampin <remirampin@gmail.com> wrote:\n> If _is_git had to follow \"gitdir: ...\" files to reach the actual Git\n> directory, we set _gitdir to that final path.\n> ---\n>  lib/choose_repository.tcl | 14 ++++++++++----\n>  1 file changed, 10 insertions(+), 4 deletions(-)\n>\n> diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\n> index 49ff641..641068d 100644\n> --- a/lib/choose_repository.tcl\n> +++ b/lib/choose_repository.tcl\n> @@ -338,13 +338,17 @@ method _git_init {} {\n>         return 1\n>  }\n>\n> -proc _is_git {path} {\n> +proc _is_git {path {outdir_var \"\"}} {\n> +       if {$outdir_var ne \"\"} {\n> +               upvar 1 $outdir_var outdir\n> +       }\n>         if {[file isfile $path]} {\n>                 set fp [open $path r]\n>                 gets $fp line\n>                 close $fp\n>                 if {[regexp \"^gitdir: (.+)$\" $line line link_target]} {\n> -                       return [_is_git [file join [file dirname $path] $link_target]]\n> +                       set link_target_abs [file join [file dirname $path] $link_target]\n\nAt this point link_target_abs is something like\nsub/../.git/modules/sub. It might be nice to normalize this with 'git\nrev-parse --git-dir' or even just (cd $link_target_abs && pwd). I'm\nnot sure if tcl has anything built in that could do this kind of\nnormalization.\n\n> +                       return [_is_git $link_target_abs outdir]\n>                 }\n>                 return 0\n>         }\n> @@ -352,12 +356,14 @@ proc _is_git {path} {\n>         if {[file exists [file join $path HEAD]]\n>          && [file exists [file join $path objects]]\n>          && [file exists [file join $path config]]} {\n> +               set outdir $path\n>                 return 1\n>         }\n>         if {[is_Cygwin]} {\n>                 if {[file exists [file join $path HEAD]]\n>                  && [file exists [file join $path objects.lnk]]\n>                  && [file exists [file join $path config.lnk]]} {\n> +                       set outdir $path\n>                         return 1\n>                 }\n>         }\n> @@ -1103,7 +1109,7 @@ method _open_local_path {} {\n>  }\n>\n>  method _do_open2 {} {\n> -       if {![_is_git [file join $local_path .git]]} {\n> +       if {![_is_git [file join $local_path .git] actualgit]} {\n>                 error_popup [mc \"Not a Git repository: %s\" [file tail $local_path]]\n>                 return\n>         }\n> @@ -1116,7 +1122,7 @@ method _do_open2 {} {\n>         }\n>\n>         _append_recentrepos [pwd]\n> -       set ::_gitdir .git\n> +       set ::_gitdir $actualgit\n>         set ::_prefix {}\n>         set done 1\n>  }\n> --\n> 1.9.5.msysgit.0\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"255574","messageId":"54D0EEB9.1090803@gmail.com","threadId":"38474","inReplyTo":"CAFOYHZBHoXC34gBu_Lx347f=-uUcVM1nHYT87SzxfeMa=KdFgw@mail.gmail.com","subject":"Re: [PATCH 1/2] Fixes _is_git","fromName":"Rémi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-03T15:52:25Z","receivedAt":"2015-02-03T15:52:25Z","isPatch":true,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"2015-02-02 12:24 UTC-05:00, Remi Rampin <remirampin@gmail.com>:\n>>  proc _is_git {path} {\n>> +       if {[file isfile $path]} {\n>> +               set fp [open $path r]\n>> +               gets $fp line\n>> +               close $fp\n>> +               if {[regexp \"^gitdir: (.+)$\" $line line link_target]} {\n\n2015-02-03 3:44 UTC-05:00, Chris Packham <judge.packham@gmail.com>:\n> It might be simpler to use one of the 'string' commands e.g. string\n> wordend \"gitdir: \" I also suspect the string functions would be faster\n> than regexp but that probably doesn't matter.\n\nI want to check that the file actually begins with \"gitdir: \" and then\nextract the path, so I'm not sure if using string functions is that\nsimple/fast.\n\n>> +                       return [_is_git [file join [file dirname $path] $link_target]]\n\n> Do we want to avoid pathological cases of infinite recursion? Someone\n> would have to maliciously create such a situation.\n\nLimiting the recursion is very simple, but I'm not sure people are\nsupposed to stumble on that. More importantly this probably calls for a\ndifferent error message, thus a new error result that I am not ready to\nimplement. But it could be another patch.\nBut I suppose I can add a simple \"return 0\" limit to the recursion if\nneeded, let me know.\n"},{"id":"255575","messageId":"54D0F0A9.3080607@gmail.com","threadId":"38474","inReplyTo":"CAFOYHZB_c3U9jpAq=jrGgMU+wMMf8w5D9iqLC9ccGC8S3hhXZg@mail.gmail.com","subject":"Re: [PATCH 2/2] Makes _do_open2 set _gitdir to actual path","fromName":"Rémi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-03T16:00:41Z","receivedAt":"2015-02-03T16:00:41Z","isPatch":true,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"2015-02-02 12:24 UTC-05:00, Remi Rampin <remirampin@gmail.com>:\n>> -                       return [_is_git [file join [file dirname $path] $link_target]]\n>> +                       set link_target_abs [file join [file dirname $path] $link_target]\n\n2015-02-03 3:51 UTC-05:00, Chris Packham <judge.packham@gmail.com>:\n> At this point link_target_abs is something like\n> sub/../.git/modules/sub. It might be nice to normalize this with 'git\n> rev-parse --git-dir' or even just (cd $link_target_abs && pwd). I'm\n> not sure if tcl has anything built in that could do this kind of\n> normalization.\n\nThere is 'file normalize' according to the docs. I can update the patch\nif needed.\n"},{"id":"255618","messageId":"CAFOYHZAerQWpeOPzD5D3gqKdWYvaCE3vB88Y_iD30eRF5MC2DQ@mail.gmail.com","threadId":"38474","inReplyTo":"54D0EEB9.1090803@gmail.com","subject":"Re: [PATCH 1/2] Fixes _is_git","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2015-02-05T08:13:45Z","receivedAt":"2015-02-05T08:13:45Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Wed, Feb 4, 2015 at 4:52 AM, Rémi Rampin <remirampin@gmail.com> wrote:\n> 2015-02-02 12:24 UTC-05:00, Remi Rampin <remirampin@gmail.com>:\n>>>  proc _is_git {path} {\n>>> +       if {[file isfile $path]} {\n>>> +               set fp [open $path r]\n>>> +               gets $fp line\n>>> +               close $fp\n>>> +               if {[regexp \"^gitdir: (.+)$\" $line line link_target]} {\n>\n> 2015-02-03 3:44 UTC-05:00, Chris Packham <judge.packham@gmail.com>:\n>> It might be simpler to use one of the 'string' commands e.g. string\n>> wordend \"gitdir: \" I also suspect the string functions would be faster\n>> than regexp but that probably doesn't matter.\n>\n> I want to check that the file actually begins with \"gitdir: \" and then\n> extract the path, so I'm not sure if using string functions is that\n> simple/fast.\n\nMakes sense.\n\n>\n>>> +                       return [_is_git [file join [file dirname $path] $link_target]]\n>\n>> Do we want to avoid pathological cases of infinite recursion? Someone\n>> would have to maliciously create such a situation.\n>\n> Limiting the recursion is very simple, but I'm not sure people are\n> supposed to stumble on that. More importantly this probably calls for a\n> different error message, thus a new error result that I am not ready to\n> implement. But it could be another patch.\n> But I suppose I can add a simple \"return 0\" limit to the recursion if\n> needed, let me know.\n\nIt'd have to be fairly intentional to cause any real problems. The one\nthing I was thinking was to factor out the part that checks for HEAD\ninfo objects etc into a __is_git that _is_git could call thus\neliminating recursion but I don't see it really being anything more\nthan a theoretical issue.\n"},{"id":"255629","messageId":"1423153215-9706-1-git-send-email-remirampin@gmail.com","threadId":"38474","inReplyTo":"CAMto89CHf4OT_S05SaRrVRZvF-PH2_6DrcEpdGiUfaRGutJQHw@mail.gmail.com","subject":"[PATCH 0/2] gitfile support git git-gui","fromName":"Remi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-05T16:20:13Z","receivedAt":"2015-02-05T16:20:13Z","isPatch":true,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"New patch series. I hadn't realized Git doesn't recurse on \"gitdir: ...\"\nlinks itself, it only follows one.\n\nAlso normalizes the path to the Git repository as requested.\n\nRemi Rampin (2):\n  Fixes chooser not accepting gitfiles\n  Makes chooser set 'gitdir' to the resolved path\n\n lib/choose_repository.tcl | 21 ++++++++++++++++++---\n 1 file changed, 18 insertions(+), 3 deletions(-)\n\n-- \n1.9.5.msysgit.0\n"},{"id":"255631","messageId":"1423153215-9706-2-git-send-email-remirampin@gmail.com","threadId":"38474","inReplyTo":"1423153215-9706-1-git-send-email-remirampin@gmail.com","subject":"[PATCH 1/2] Fixes chooser not accepting gitfiles","fromName":"Remi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-05T16:20:14Z","receivedAt":"2015-02-05T16:20:14Z","isPatch":true,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"Makes _is_git handle the case where the path is a \"gitdir: ...\" file.\n---\n lib/choose_repository.tcl | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\nindex 92d6022..abc6b1d 100644\n--- a/lib/choose_repository.tcl\n+++ b/lib/choose_repository.tcl\n@@ -339,6 +339,16 @@ method _git_init {} {\n }\n \n proc _is_git {path} {\n+\tif {[file isfile $path]} {\n+\t\tset fp [open $path r]\n+\t\tgets $fp line\n+\t\tclose $fp\n+\t\tif {[regexp \"^gitdir: (.+)$\" $line line link_target]} {\n+\t\t\tset path [file join [file dirname $path] $link_target]\n+\t\t\tset path [file normalize $path]\n+\t\t}\n+\t}\n+\n \tif {[file exists [file join $path HEAD]]\n \t && [file exists [file join $path objects]]\n \t && [file exists [file join $path config]]} {\n-- \n1.9.5.msysgit.0\n"},{"id":"255630","messageId":"1423153215-9706-3-git-send-email-remirampin@gmail.com","threadId":"38474","inReplyTo":"1423153215-9706-1-git-send-email-remirampin@gmail.com","subject":"[PATCH 2/2] Makes chooser set 'gitdir' to the resolved path","fromName":"Remi Rampin","fromEmail":"remirampin@gmail.com","sentAt":"2015-02-05T16:20:15Z","receivedAt":"2015-02-05T16:20:15Z","isPatch":true,"sender":{"key":"remirampin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/426784?v=4"},"body":"If _is_git follows a \"gitdir: ...\" file link to get to the actual\nrepository, we want _gitdir to be set to that final path.\n---\n lib/choose_repository.tcl | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\nindex abc6b1d..75d1da8 100644\n--- a/lib/choose_repository.tcl\n+++ b/lib/choose_repository.tcl\n@@ -338,7 +338,10 @@ method _git_init {} {\n \treturn 1\n }\n \n-proc _is_git {path} {\n+proc _is_git {path {outdir_var \"\"}} {\n+\tif {$outdir_var ne \"\"} {\n+\t\tupvar 1 $outdir_var outdir\n+\t}\n \tif {[file isfile $path]} {\n \t\tset fp [open $path r]\n \t\tgets $fp line\n@@ -352,12 +355,14 @@ proc _is_git {path} {\n \tif {[file exists [file join $path HEAD]]\n \t && [file exists [file join $path objects]]\n \t && [file exists [file join $path config]]} {\n+\t\tset outdir $path\n \t\treturn 1\n \t}\n \tif {[is_Cygwin]} {\n \t\tif {[file exists [file join $path HEAD]]\n \t\t && [file exists [file join $path objects.lnk]]\n \t\t && [file exists [file join $path config.lnk]]} {\n+\t\t\tset outdir $path\n \t\t\treturn 1\n \t\t}\n \t}\n@@ -1103,7 +1108,7 @@ method _open_local_path {} {\n }\n \n method _do_open2 {} {\n-\tif {![_is_git [file join $local_path .git]]} {\n+\tif {![_is_git [file join $local_path .git] actualgit]} {\n \t\terror_popup [mc \"Not a Git repository: %s\" [file tail $local_path]]\n \t\treturn\n \t}\n@@ -1116,7 +1121,7 @@ method _do_open2 {} {\n \t}\n \n \t_append_recentrepos [pwd]\n-\tset ::_gitdir .git\n+\tset ::_gitdir $actualgit\n \tset ::_prefix {}\n \tset done 1\n }\n-- \n1.9.5.msysgit.0\n"}]}