{"thread":{"id":"16145","subject":"[RFC PATCH] git-svn: proper detection of bare repositories","startedAt":"2008-11-03T00:09:03Z","lastAt":"2008-11-06T09:45:22Z","messageCount":4,"participants":["Deskin Miller","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"94708","messageId":"20081103000903.GA1135@euler","threadId":"16145","inReplyTo":null,"subject":"[RFC PATCH] git-svn: proper detection of bare repositories","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-03T00:09:03Z","receivedAt":"2008-11-03T00:09:03Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"I keep coming across commands like this, which don't work properly in bare\nrepositories, and thinking that they need to be patched (see e.g. ddff8563, or\nDuy's comments on the thread starting at\nhttp://thread.gmane.org/gmane.comp.version-control.git/98849), but now I'm not\nso sure.  For one, despite this patch working, it turns out that 'git --bare\nsvn <cmd>' also works (and presumably has) for some time.\n\nIs git --bare the correct way to deal with this situation?  That is to say, do\nwe intend commands to 'just work' regardless of whether the repo is bare or\nnot, or should the user be thinking about the difference and including --bare\nin the command invocation when necessary?  I'm a vote for the 'just work' camp,\nbut it seems a lot of things aren't necessarily that way.  On the other hand,\nthe majority of commands do just work.\n\nI guess I'm asking for a sanity check before I write any more such patches;\ncertainly I find them useful, as the issues come up during my normal use of\nGit, but I don't want to be pursuing things of no use to anyone else, or\n(worse) things that are fundamentally wrong for some reason I don't understand\nyet.\n\n-- 8< --\n\nWhen in a bare repository (or .git, for that matter), git-svn would fail\nto initialise properly, since git rev-parse --show-cdup would not output\nanything.  However, git rev-parse --show-cdup actually returns an error\ncode if it's really not in a git directory.\n\nFix the issue by checking for an explicit error from git rev-parse, and\nsetting $git_dir appropriately if instead it just does not output.\n\nSigned-off-by: Deskin Miller <deskinm@umich.edu>\n---\n git-svn.perl             |   14 ++++++++++----\n t/t9100-git-svn-basic.sh |    9 +++++++++\n 2 files changed, 19 insertions(+), 4 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 56238da..d25e9be 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -42,6 +42,7 @@ use File::Path qw/mkpath/;\n use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;\n use IPC::Open3;\n use Git;\n+use Error qw/:try/;\n \n BEGIN {\n \t# import functions from Git into our packages, en masse\n@@ -214,11 +215,16 @@ unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n \t\t\t    \"but it is not a directory\\n\";\n \t\t}\n \t\tmy $git_dir = delete $ENV{GIT_DIR};\n-\t\tchomp(my $cdup = command_oneline(qw/rev-parse --show-cdup/));\n-\t\tunless (length $cdup) {\n-\t\t\tdie \"Already at toplevel, but $git_dir \",\n-\t\t\t    \"not found '$cdup'\\n\";\n-\t\t}\n+\t\tmy $cdup = undef;\n+\t\ttry {\n+\t\t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n+\t\t\t$git_dir = '.' unless ($cdup);\n+\t\t\tchomp $cdup if ($cdup);\n+\t\t\t$cdup = \".\" unless ($cdup && length $cdup);\n+\t\t}\n+\t\tcatch Git::Error::Command with {\n+\t\t\tdie \"Already at toplevel, but $git_dir not found\\n\";\n+\t\t};\n \t\tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n \t\tunless (-d $git_dir) {\n \t\t\tdie \"$git_dir still not found after going to \",\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 843a501..fdbc23a 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -265,4 +265,13 @@ test_expect_success 'able to set-tree to a subdirectory' \"\n \ttest -z \\\"\\`git diff refs/heads/my-bar refs/remotes/bar\\`\\\"\n \t\"\n \n+test_expect_success 'git-svn works in a bare repository' '\n+\tmkdir bare-repo &&\n+\t( cd bare-repo &&\n+\tgit init --bare &&\n+\tGIT_DIR=. git svn init \"$svnrepo\" &&\n+\tgit svn fetch ) &&\n+\trm -rf bare-repo\n+\t'\n+\n test_done\n-- \n1.6.0.3.524.g47d14\n"},{"id":"94869","messageId":"20081104083015.GA14405@untitled","threadId":"16145","inReplyTo":"20081103000903.GA1135@euler","subject":"Re: [RFC PATCH] git-svn: proper detection of bare repositories","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-11-04T08:30:15Z","receivedAt":"2008-11-04T08:30:15Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Deskin Miller <deskinm@umich.edu> wrote:\n> I keep coming across commands like this, which don't work properly in bare\n> repositories, and thinking that they need to be patched (see e.g. ddff8563, or\n> Duy's comments on the thread starting at\n> http://thread.gmane.org/gmane.comp.version-control.git/98849), but now I'm not\n> so sure.  For one, despite this patch working, it turns out that 'git --bare\n> svn <cmd>' also works (and presumably has) for some time.\n\nInteresting.  I've never even looked at --bare myself.  It always\nshould've worked if GIT_DIR= was explicitly set and I guess back in the\nold days when I wrote git-svn, --bare wasn't even a flag :)\n\n> Is git --bare the correct way to deal with this situation?  That is to say, do\n> we intend commands to 'just work' regardless of whether the repo is bare or\n> not, or should the user be thinking about the difference and including --bare\n> in the command invocation when necessary?  I'm a vote for the 'just work' camp,\n> but it seems a lot of things aren't necessarily that way.  On the other hand,\n> the majority of commands do just work.\n>\n> I guess I'm asking for a sanity check before I write any more such patches;\n> certainly I find them useful, as the issues come up during my normal use of\n> Git, but I don't want to be pursuing things of no use to anyone else, or\n> (worse) things that are fundamentally wrong for some reason I don't understand\n> yet.\n\nI don't think there's anything fundamentally wrong with things 'just\nworking' on bare repos.  It's just another case of something that not\nmany people end up using (especially for git-svn) and hence got fewer\ntesters and bug reports.\n\n> -- 8< --\n> \n> When in a bare repository (or .git, for that matter), git-svn would fail\n> to initialise properly, since git rev-parse --show-cdup would not output\n> anything.  However, git rev-parse --show-cdup actually returns an error\n> code if it's really not in a git directory.\n> \n> Fix the issue by checking for an explicit error from git rev-parse, and\n> setting $git_dir appropriately if instead it just does not output.\n> \n> Signed-off-by: Deskin Miller <deskinm@umich.edu>\n> ---\n>  git-svn.perl             |   14 ++++++++++----\n>  t/t9100-git-svn-basic.sh |    9 +++++++++\n>  2 files changed, 19 insertions(+), 4 deletions(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index 56238da..d25e9be 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -42,6 +42,7 @@ use File::Path qw/mkpath/;\n>  use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;\n>  use IPC::Open3;\n>  use Git;\n> +use Error qw/:try/;\n>  \n>  BEGIN {\n>  \t# import functions from Git into our packages, en masse\n> @@ -214,11 +215,16 @@ unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n>  \t\t\t    \"but it is not a directory\\n\";\n>  \t\t}\n>  \t\tmy $git_dir = delete $ENV{GIT_DIR};\n> -\t\tchomp(my $cdup = command_oneline(qw/rev-parse --show-cdup/));\n> -\t\tunless (length $cdup) {\n> -\t\t\tdie \"Already at toplevel, but $git_dir \",\n> -\t\t\t    \"not found '$cdup'\\n\";\n> -\t\t}\n> +\t\tmy $cdup = undef;\n> +\t\ttry {\n> +\t\t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n> +\t\t\t$git_dir = '.' unless ($cdup);\n> +\t\t\tchomp $cdup if ($cdup);\n> +\t\t\t$cdup = \".\" unless ($cdup && length $cdup);\n> +\t\t}\n> +\t\tcatch Git::Error::Command with {\n> +\t\t\tdie \"Already at toplevel, but $git_dir not found\\n\";\n> +\t\t};\n\nHow about using git_cmd_try instead?\n\nThe Error.pm try/catch stuff makes me a bit uncomfortable.  I realize\nit's (unfortunately) in Git.pm; but I'd rather keep it confined there so\nwe can more easily remove it later if someone were inclined.\n\nOtherwise I like the idea of this patch.\n\nThanks,\n\n-- \nEric Wong\n"},{"id":"95028","messageId":"20081106050739.GA7713@euler","threadId":"16145","inReplyTo":"20081104083015.GA14405@untitled","subject":"[PATCH v2] git-svn: proper detection of bare repositories","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-06T05:07:39Z","receivedAt":"2008-11-06T05:07:39Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"On Tue, Nov 04, 2008 at 12:30:15AM -0800, Eric Wong wrote:\n> How about using git_cmd_try instead?\n> \n> The Error.pm try/catch stuff makes me a bit uncomfortable.  I realize\n> it's (unfortunately) in Git.pm; but I'd rather keep it confined there so\n> we can more easily remove it later if someone were inclined.\n\nYeah, major thinko; I read the Git(3pm) manpage, looked at git_cmd_try, and for\nsome reason thought that it wasn't what I want.  But it's exactly what I want.\n\nDeskin Miller\n\n-- 8< --\n\nWhen in a bare repository (or .git, for that matter), git-svn would fail\nto initialise properly, since git rev-parse --show-cdup would not output\nanything.  However, git rev-parse --show-cdup actually returns an error\ncode if it's really not in a git directory.\n\nFix the issue by checking for an explicit error from git rev-parse, and\nsetting $git_dir appropriately if instead it just does not output.\n\nSigned-off-by: Deskin Miller <deskinm@umich.edu>\n---\n git-svn.perl             |   12 +++++++-----\n t/t9100-git-svn-basic.sh |    9 +++++++++\n 2 files changed, 16 insertions(+), 5 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 56238da..829a323 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -214,11 +214,13 @@ unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n \t\t\t    \"but it is not a directory\\n\";\n \t\t}\n \t\tmy $git_dir = delete $ENV{GIT_DIR};\n-\t\tchomp(my $cdup = command_oneline(qw/rev-parse --show-cdup/));\n-\t\tunless (length $cdup) {\n-\t\t\tdie \"Already at toplevel, but $git_dir \",\n-\t\t\t    \"not found '$cdup'\\n\";\n-\t\t}\n+\t\tmy $cdup = undef;\n+\t\tgit_cmd_try {\n+\t\t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n+\t\t\t$git_dir = '.' unless ($cdup);\n+\t\t\tchomp $cdup if ($cdup);\n+\t\t\t$cdup = \".\" unless ($cdup && length $cdup);\n+\t\t} \"Already at toplevel, but $git_dir not found\\n\";\n \t\tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n \t\tunless (-d $git_dir) {\n \t\t\tdie \"$git_dir still not found after going to \",\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 843a501..fdbc23a 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -265,4 +265,13 @@ test_expect_success 'able to set-tree to a subdirectory' \"\n \ttest -z \\\"\\`git diff refs/heads/my-bar refs/remotes/bar\\`\\\"\n \t\"\n \n+test_expect_success 'git-svn works in a bare repository' '\n+\tmkdir bare-repo &&\n+\t( cd bare-repo &&\n+\tgit init --bare &&\n+\tGIT_DIR=. git svn init \"$svnrepo\" &&\n+\tgit svn fetch ) &&\n+\trm -rf bare-repo\n+\t'\n+\n test_done\n-- \n1.6.0.3.524.g47d14\n"},{"id":"95036","messageId":"20081106094522.GB15686@untitled","threadId":"16145","inReplyTo":"20081106050739.GA7713@euler","subject":"Re: [PATCH v2] git-svn: proper detection of bare repositories","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-11-06T09:45:22Z","receivedAt":"2008-11-06T09:45:22Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Deskin Miller <deskinm@umich.edu> wrote:\n> On Tue, Nov 04, 2008 at 12:30:15AM -0800, Eric Wong wrote:\n> > How about using git_cmd_try instead?\n> > \n> > The Error.pm try/catch stuff makes me a bit uncomfortable.  I realize\n> > it's (unfortunately) in Git.pm; but I'd rather keep it confined there so\n> > we can more easily remove it later if someone were inclined.\n> \n> Yeah, major thinko; I read the Git(3pm) manpage, looked at git_cmd_try, and for\n> some reason thought that it wasn't what I want.  But it's exactly what I want.\n\nThanks, acked and pushed out to git://git.bogomips.org/git-svn.git\n\n-- \nEric Wong\n"}]}