{"thread":{"id":"29826","subject":"[PATCH v2] git-svn: Simplify calculation of GIT_DIR","startedAt":"2012-03-03T19:53:17Z","lastAt":"2013-01-24T10:14:17Z","messageCount":15,"participants":["Barry Wardell","Eric Wong","Junio C Hamano","Joachim Schmitz","Philip Oakley"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"186011","messageId":"1330804397-43062-1-git-send-email-barry.wardell@gmail.com","threadId":"29826","inReplyTo":null,"subject":"[PATCH v2] git-svn: Simplify calculation of GIT_DIR","fromName":"Barry Wardell","fromEmail":"barry.wardell@gmail.com","sentAt":"2012-03-03T19:53:17Z","receivedAt":"2012-03-03T19:53:17Z","isPatch":true,"sender":{"key":"barry.wardell@gmail.com","avatar":"https://avatars.githubusercontent.com/u/354095?v=4"},"body":"Since git-rev-parse already checks for the $GIT_DIR environment\nvariable and that it returns an actual git repository, there is no\nneed to repeat the checks again here.\n\nThis also fixes a problem where git-svn did not work in cases where\n.git was a file with a gitdir: link.\n---\n git-svn.perl | 33 +++++++++++----------------------\n 1 file changed, 11 insertions(+), 22 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4334b95..bbfd351 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -15,8 +15,6 @@ my $cmd_dir_prefix = eval {\n \tcommand_oneline([qw/rev-parse --show-prefix/], STDERR => 0)\n } || '';\n \n-my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};\n-$ENV{GIT_DIR} ||= '.git';\n $Git::SVN::default_repo_id = 'svn';\n $Git::SVN::default_ref_id = $ENV{GIT_SVN_ID} || 'git-svn';\n $Git::SVN::Ra::_log_window_size = 100;\n@@ -292,26 +290,17 @@ for (my $i = 0; $i < @ARGV; $i++) {\n \n # make sure we're always running at the top-level working directory\n unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n-\tunless (-d $ENV{GIT_DIR}) {\n-\t\tif ($git_dir_user_set) {\n-\t\t\tdie \"GIT_DIR=$ENV{GIT_DIR} explicitly set, \",\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\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 \",\n-\t\t\t \"'$cdup'\\n\";\n-\t\t}\n-\t\t$ENV{GIT_DIR} = $git_dir;\n-\t}\n+\tmy $toplevel = undef;\n+\n+\tgit_cmd_try {\n+\t\t$toplevel = command_oneline([qw/rev-parse --show-toplevel/]);\n+\t} \"Unable to find toplevel directory\\n\";\n+\n+\tgit_cmd_try {\n+\t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n+\t} \"Unable to find .git directory\\n\";\n+\n+\tchdir $toplevel or die \"Unable to chdir to '$toplevel'\\n\";\n \t$_repository = Git->repository(Repository => $ENV{GIT_DIR});\n }\n \n-- \n1.7.9.2\n"},{"id":"186365","messageId":"20120308005103.GA27398@dcvr.yhbt.net","threadId":"29826","inReplyTo":"1330804397-43062-1-git-send-email-barry.wardell@gmail.com","subject":"Re: [PATCH v2] git-svn: Simplify calculation of GIT_DIR","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-03-08T00:51:03Z","receivedAt":"2012-03-08T00:51:03Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Barry Wardell wrote:\n> -my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};\n> -$ENV{GIT_DIR} ||= '.git';\n\n\n\n> unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n\n\n\n> +\tgit_cmd_try {\n> +\t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n> +\t} \"Unable to find .git directory\\n\";\n> +\n> +\tchdir $toplevel or die \"Unable to chdir to '$toplevel'\\n\";\n> \t$_repository = Git->repository(Repository => $ENV{GIT_DIR});\n> }\n\nIt looks like some places in \"git svn (init|clone|multi-init)\" rely on\nGIT_DIR being set. I noticed the first test of t9100-git-svn-basic.sh\nfailing (causing everything else in that test to fail) with your patch.\n\nCan you fix those use cases (and ensure tests pass)? Thanks.\n"},{"id":"207351","messageId":"1358731322-44600-1-git-send-email-barry.wardell@gmail.com","threadId":"29826","inReplyTo":"20120308005103.GA27398@dcvr.yhbt.net","subject":"[PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Barry Wardell","fromEmail":"barry.wardell@gmail.com","sentAt":"2013-01-21T01:22:00Z","receivedAt":"2013-01-21T01:22:00Z","isPatch":true,"sender":{"key":"barry.wardell@gmail.com","avatar":"https://avatars.githubusercontent.com/u/354095?v=4"},"body":"These patches fix a bug which prevented git-svn from working with repositories\nwhich use gitdir links.\n\nChanges since v2:\n - Rebased onto latest master.\n - Added test case which verifies that the problem has been fixed.\n - Fixed problems with git svn (init|clone|multi-init).\n - All git-svn test cases now pass (except two in t9101 which also failed\n before these patches).\n\nBarry Wardell (2):\n git-svn: Add test for git-svn repositories with a gitdir link\n git-svn: Simplify calculation of GIT_DIR\n\n git-svn.perl | 36 +++++++++++++-----------------------\n t/t9100-git-svn-basic.sh | 8 ++++++++\n 2 files changed, 21 insertions(+), 23 deletions(-)\n\n-- \n1.8.0\n"},{"id":"207349","messageId":"1358731322-44600-2-git-send-email-barry.wardell@gmail.com","threadId":"29826","inReplyTo":"1358731322-44600-1-git-send-email-barry.wardell@gmail.com","subject":"[PATCH 1/2] git-svn: Add test for git-svn repositories with a gitdir link","fromName":"Barry Wardell","fromEmail":"barry.wardell@gmail.com","sentAt":"2013-01-21T01:22:01Z","receivedAt":"2013-01-21T01:22:01Z","isPatch":true,"sender":{"key":"barry.wardell@gmail.com","avatar":"https://avatars.githubusercontent.com/u/354095?v=4"},"body":"Signed-off-by: Barry Wardell \n---\n t/t9100-git-svn-basic.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 749b75e..4fea8d9 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -306,5 +306,13 @@ test_expect_success 'git-svn works in a bare repository' '\n \tgit svn fetch ) &&\n \trm -rf bare-repo\n \t'\n+test_expect_success 'git-svn works in in a repository with a gitdir: link' '\n+\tmkdir worktree gitdir &&\n+\t( cd worktree &&\n+\tgit svn init \"$svnrepo\" &&\n+\tgit init --separate-git-dir ../gitdir &&\n+\tgit svn fetch ) &&\n+\trm -rf worktree gitdir\n+\t'\n \n test_done\n-- \n1.8.0\n"},{"id":"207350","messageId":"1358731322-44600-3-git-send-email-barry.wardell@gmail.com","threadId":"29826","inReplyTo":"1358731322-44600-1-git-send-email-barry.wardell@gmail.com","subject":"[PATCH 2/2] git-svn: Simplify calculation of GIT_DIR","fromName":"Barry Wardell","fromEmail":"barry.wardell@gmail.com","sentAt":"2013-01-21T01:22:02Z","receivedAt":"2013-01-21T01:22:02Z","isPatch":true,"sender":{"key":"barry.wardell@gmail.com","avatar":"https://avatars.githubusercontent.com/u/354095?v=4"},"body":"Since git-rev-parse already checks for the $GIT_DIR environment\nvariable and that it returns an actual git repository, there is no\nneed to repeat the checks again here.\n\nThis also fixes a problem where git-svn did not work in cases where\n.git was a file with a gitdir: link.\n\nSigned-off-by: Barry Wardell \n---\n git-svn.perl | 36 +++++++++++++-----------------------\n 1 file changed, 13 insertions(+), 23 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex bd5266c..3bcd769 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -61,8 +61,6 @@ my $cmd_dir_prefix = eval {\n \tcommand_oneline([qw/rev-parse --show-prefix/], STDERR => 0)\n } || '';\n \n-my $git_dir_user_set = 1 if defined $ENV{GIT_DIR};\n-$ENV{GIT_DIR} ||= '.git';\n $Git::SVN::Ra::_log_window_size = 100;\n \n if (! exists $ENV{SVN_SSH} && exists $ENV{GIT_SSH}) {\n@@ -325,27 +323,19 @@ for (my $i = 0; $i < @ARGV; $i++) {\n };\n \n # make sure we're always running at the top-level working directory\n-unless ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n-\tunless (-d $ENV{GIT_DIR}) {\n-\t\tif ($git_dir_user_set) {\n-\t\t\tdie \"GIT_DIR=$ENV{GIT_DIR} explicitly set, \",\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\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 \",\n-\t\t\t \"'$cdup'\\n\";\n-\t\t}\n-\t\t$ENV{GIT_DIR} = $git_dir;\n-\t}\n+if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n+\t$ENV{GIT_DIR} ||= \".git\";\n+} else {\n+\tgit_cmd_try {\n+\t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n+\t} \"Unable to find .git directory\\n\";\n+\tmy $cdup = undef;\n+\tgit_cmd_try {\n+\t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n+\t\tchomp $cdup if ($cdup);\n+\t\t$cdup = \".\" unless ($cdup && length $cdup);\n+\t} \"Already at toplevel, but $ENV{GIT_DIR} not found\\n\";\n+\tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n \t$_repository = Git->repository(Repository => $ENV{GIT_DIR});\n }\n \n-- \n1.8.0\n"},{"id":"207357","messageId":"7vwqv7i9su.fsf@alter.siamese.dyndns.org","threadId":"29826","inReplyTo":"1358731322-44600-1-git-send-email-barry.wardell@gmail.com","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-21T01:50:25Z","receivedAt":"2013-01-21T01:50:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Barry Wardell writes:\n\n> These patches fix a bug which prevented git-svn from working with repositories\n> which use gitdir links.\n>\n> Changes since v2:\n> - Rebased onto latest master.\n> - Added test case which verifies that the problem has been fixed.\n> - Fixed problems with git svn (init|clone|multi-init).\n> - All git-svn test cases now pass (except two in t9101 which also failed\n> before these patches).\n>\n> Barry Wardell (2):\n> git-svn: Add test for git-svn repositories with a gitdir link\n> git-svn: Simplify calculation of GIT_DIR\n\nThanks for your persistence ;-) As this is a pretty old topic, I'll\ngive two URLs for people who are interested to view the previous\nthreads:\n\n http://thread.gmane.org/gmane.comp.version-control.git/192133\n http://thread.gmane.org/gmane.comp.version-control.git/192127\n\nYou would want to mark it as test_expect_failure in the first patch\nand then flip it to text_expect_success in the second patch where\nyou fix the breakage? Otherwise, after applying the first patch,\nthe testsuite will break needlessly.\n\nI've Cc'ed Eric Wong (git-svn maintainer) and CMN who helped in the\nprevious round. If the only issue is the above success/failure one,\nI think Eric can tweak the patches while applying them (I didn't\nlook at the changes carefully myself, by the way).\n\nThanks.\n"},{"id":"207400","messageId":"kdjip9$4j7$1@ger.gmane.org","threadId":"29826","inReplyTo":"7vwqv7i9su.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2013-01-21T14:19:19Z","receivedAt":"2013-01-21T14:19:19Z","isPatch":true,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Junio C Hamano wrote:\n> Barry Wardell writes:\n>\n>> These patches fix a bug which prevented git-svn from working with\n>> repositories which use gitdir links.\n>>\n>> Changes since v2:\n>> - Rebased onto latest master.\n>> - Added test case which verifies that the problem has been fixed.\n>> - Fixed problems with git svn (init|clone|multi-init).\n>> - All git-svn test cases now pass (except two in t9101 which also\n>> failed before these patches).\n>>\n>> Barry Wardell (2):\n>> git-svn: Add test for git-svn repositories with a gitdir link\n>> git-svn: Simplify calculation of GIT_DIR\n>\n> Thanks for your persistence ;-) As this is a pretty old topic, I'll\n> give two URLs for people who are interested to view the previous\n> threads:\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/192133\n> http://thread.gmane.org/gmane.comp.version-control.git/192127\n>\n> You would want to mark it as test_expect_failure in the first patch\n> and then flip it to text_expect_success in the second patch where\n> you fix the breakage? Otherwise, after applying the first patch,\n> the testsuite will break needlessly.\n\nI'd just apply them the other way round, 1st fix the problem, 2nd add a test \nfor it\n\nBye, Jojo \n"},{"id":"207443","messageId":"2931F4CC43E4406DBB878482C2F0E4F4@PhilipOakley","threadId":"29826","inReplyTo":"kdjip9$4j7$1@ger.gmane.org","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2013-01-21T20:29:46Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Joachim Schmitz\" \nSent: Monday, January 21, 2013 2:19 PM\n> Junio C Hamano wrote:\n>> Barry Wardell writes:\n[...]\n>> Thanks for your persistence ;-) As this is a pretty old topic, I'll\n>> give two URLs for people who are interested to view the previous\n>> threads:\n>>\n>> http://thread.gmane.org/gmane.comp.version-control.git/192133\n>> http://thread.gmane.org/gmane.comp.version-control.git/192127\n>>\n>> You would want to mark it as test_expect_failure in the first patch\n>> and then flip it to text_expect_success in the second patch where\n>> you fix the breakage? Otherwise, after applying the first patch,\n>> the testsuite will break needlessly.\n>\n> I'd just apply them the other way round, 1st fix the problem, 2nd add \n> a test for it\n\nIsn't it a case of, 1st demonstrate the problem with a test, and then \n2nd fix the problem.\n\nThose less principled could could simply \"fix\" a non-existent problem \nmerely to get themselves into the change log, or worse, even if one may \nfix-test under the hood.\n\n>\n> Bye, Jojo\n\nPhilip \n"},{"id":"207444","messageId":"7vfw1ufauc.fsf@alter.siamese.dyndns.org","threadId":"29826","inReplyTo":"2931F4CC43E4406DBB878482C2F0E4F4@PhilipOakley","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-21T22:08:27Z","receivedAt":"2013-01-21T22:08:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" writes:\n\n> From: \"Joachim Schmitz\" \n> Sent: Monday, January 21, 2013 2:19 PM\n>> Junio C Hamano wrote:\n>>> Barry Wardell writes:\n> [...]\n>>> Thanks for your persistence ;-) As this is a pretty old topic, I'll\n>>> give two URLs for people who are interested to view the previous\n>>> threads:\n>>>\n>>> http://thread.gmane.org/gmane.comp.version-control.git/192133\n>>> http://thread.gmane.org/gmane.comp.version-control.git/192127\n>>>\n>>> You would want to mark it as test_expect_failure in the first patch\n>>> and then flip it to text_expect_success in the second patch where\n>>> you fix the breakage? Otherwise, after applying the first patch,\n>>> the testsuite will break needlessly.\n>>\n>> I'd just apply them the other way round, 1st fix the problem, 2nd\n>> add a test for it\n>\n> Isn't it a case of, 1st demonstrate the problem with a test, and then\n> 2nd fix the problem.\n>\n> Those less principled could could simply \"fix\" a non-existent problem\n> merely to get themselves into the change log, or worse, even if one\n> may fix-test under the hood.\n\nFor a small/trivial fix, fixing the code and protecting the fix from\nfuture breakages by adding tests that expect success in a single\ncommit is the most sensible thing to do. People who are interested,\nand people who are auditing, can locally revert only the code change\nto see the new tests fail fairly easily in such a case.\n\nFor a more involved series, it is easier to demonstrate a breakage\nby adding tests that expect failure in the first commit, and then in\nsubsequent commits, to fix a class of bugs in the code and flipping\nexpect_failure into expect_success for the tests that the updated\ncode in the commit fixes.\n\nFor this particular topic, squashing the two patches into a single\ncommit may probably be the more appropriate between the two.\n\nThanks.\n"},{"id":"207546","messageId":"20130123023235.GA24135@dcvr.yhbt.net","threadId":"29826","inReplyTo":"1358731322-44600-1-git-send-email-barry.wardell@gmail.com","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2013-01-23T02:32:35Z","receivedAt":"2013-01-23T02:32:35Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Barry Wardell wrote:\n> These patches fix a bug which prevented git-svn from working with repositories\n> which use gitdir links.\n> \n> Changes since v2:\n> - Rebased onto latest master.\n> - Added test case which verifies that the problem has been fixed.\n> - Fixed problems with git svn (init|clone|multi-init).\n> - All git-svn test cases now pass (except two in t9101 which also failed\n> before these patches).\n\nt9101 did not fail for me before your patches. However I have a\npatch on top of your 2/2 which should fix things.\n\n`git rev-parse --show-cdup` outputs nothing if GIT_DIR is set,\nso I unset GIT_DIR temporarily.\n\nI'm not sure why --show-cdup behaves like this, though..\n\nDoes squashing this on top of your changes fix all your failures?\nI plan on squashing both your changes together with the below:\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex c232798..e5bd292 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n \t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n \t} \"Unable to find .git directory\\n\";\n \tmy $cdup = undef;\n+\tmy $git_dir = delete $ENV{GIT_DIR};\n \tgit_cmd_try {\n \t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n \t\tchomp $cdup if ($cdup);\n \t\t$cdup = \".\" unless ($cdup && length $cdup);\n-\t} \"Already at toplevel, but $ENV{GIT_DIR} not found\\n\";\n+\t} \"Already at toplevel, but $git_dir not found\\n\";\n+\t$ENV{GIT_DIR} = $git_dir;\n \tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n \t$_repository = Git->repository(Repository => $ENV{GIT_DIR});\n }\n"},{"id":"207551","messageId":"7vbocgk3t4.fsf@alter.siamese.dyndns.org","threadId":"29826","inReplyTo":"20130123023235.GA24135@dcvr.yhbt.net","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-23T02:53:43Z","receivedAt":"2013-01-23T02:53:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> `git rev-parse --show-cdup` outputs nothing if GIT_DIR is set,\n> so I unset GIT_DIR temporarily.\n>\n> I'm not sure why --show-cdup behaves like this, though..\n\nSetting GIT_DIR is to say \"That is the directory that has the\nrepository objects and refs; I am letting you know the location\nexplicitly because it does not have any relation with the location\nof the working tree. The $(cwd) is at the root of the working\ntree\".\n\nIf you want to say \"That is the directory that has metainformation,\nand that other one is the root of the working tree\", you use\nGIT_WORK_TREE to name the latter.\n\nSo by definition, if you only set GIT_DIR without setting\nGIT_WORK_TREE, show-cdup must say \"you are already at the top\".\n\n>\n> Does squashing this on top of your changes fix all your failures?\n> I plan on squashing both your changes together with the below:\n>\n> diff --git a/git-svn.perl b/git-svn.perl\n> index c232798..e5bd292 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n> \t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n> \t} \"Unable to find .git directory\\n\";\n> \tmy $cdup = undef;\n> +\tmy $git_dir = delete $ENV{GIT_DIR};\n> \tgit_cmd_try {\n> \t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n> \t\tchomp $cdup if ($cdup);\n> \t\t$cdup = \".\" unless ($cdup && length $cdup);\n> -\t} \"Already at toplevel, but $ENV{GIT_DIR} not found\\n\";\n> +\t} \"Already at toplevel, but $git_dir not found\\n\";\n> +\t$ENV{GIT_DIR} = $git_dir;\n> \tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n> \t$_repository = Git->repository(Repository => $ENV{GIT_DIR});\n> }\n"},{"id":"207587","messageId":"CAHrK+Z8kc_O1CE4Le=XpiXWJ2Fadh906nbfgf0rqmvL0e6=P6A@mail.gmail.com","threadId":"29826","inReplyTo":"20130123023235.GA24135@dcvr.yhbt.net","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Barry Wardell","fromEmail":"barry.wardell@gmail.com","sentAt":"2013-01-23T12:08:37Z","receivedAt":"2013-01-23T12:08:37Z","isPatch":true,"sender":{"key":"barry.wardell@gmail.com","avatar":"https://avatars.githubusercontent.com/u/354095?v=4"},"body":"On Wed, Jan 23, 2013 at 2:32 AM, Eric Wong wrote:\n>\n> Barry Wardell wrote:\n> > These patches fix a bug which prevented git-svn from working with repositories\n> > which use gitdir links.\n> >\n> > Changes since v2:\n> > - Rebased onto latest master.\n> > - Added test case which verifies that the problem has been fixed.\n> > - Fixed problems with git svn (init|clone|multi-init).\n> > - All git-svn test cases now pass (except two in t9101 which also failed\n> > before these patches).\n>\n> t9101 did not fail for me before your patches. However I have a\n> patch on top of your 2/2 which should fix things.\n>\n> `git rev-parse --show-cdup` outputs nothing if GIT_DIR is set,\n> so I unset GIT_DIR temporarily.\n>\n> I'm not sure why --show-cdup behaves like this, though..\n>\n> Does squashing this on top of your changes fix all your failures?\n> I plan on squashing both your changes together with the below:\n>\n> diff --git a/git-svn.perl b/git-svn.perl\n> index c232798..e5bd292 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n> $ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n> } \"Unable to find .git directory\\n\";\n> my $cdup = undef;\n> + my $git_dir = delete $ENV{GIT_DIR};\n> git_cmd_try {\n> $cdup = command_oneline(qw/rev-parse --show-cdup/);\n> chomp $cdup if ($cdup);\n> $cdup = \".\" unless ($cdup && length $cdup);\n> - } \"Already at toplevel, but $ENV{GIT_DIR} not found\\n\";\n> + } \"Already at toplevel, but $git_dir not found\\n\";\n> + $ENV{GIT_DIR} = $git_dir;\n> chdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n> $_repository = Git->repository(Repository => $ENV{GIT_DIR});\n> }\n\n\nYes, I can confirm that applying this patch on top of mine makes all\ngit-svn tests pass again. I have also re-run the tests without my\npatch applied and found that they do all indeed pass, so I apologize\nfor my previous incorrect comment.\n"},{"id":"207653","messageId":"20130124012724.GA8112@dcvr.yhbt.net","threadId":"29826","inReplyTo":"CAHrK+Z-uXAEgd_HuisbioO8=D7DEdmceeUEz3A1Jr_rtm7a3WA@mail.gmail.com","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2013-01-24T01:27:24Z","receivedAt":"2013-01-24T01:27:24Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Barry Wardell wrote:\n> On Wed, Jan 23, 2013 at 2:32 AM, Eric Wong wrote:\n> > Does squashing this on top of your changes fix all your failures?\n> > I plan on squashing both your changes together with the below:\n> \n> Yes, I can confirm that applying this patch on top of mine makes all\n> git-svn tests pass again. I have also re-run the tests without my patch\n> applied and found that they do all indeed pass, so I apologize for my\n> previous incorrect comment.\n\nThanks, squashed, tested and pushed (have another unrelated patch coming)\n"},{"id":"207663","messageId":"7vehhbdu8y.fsf@alter.siamese.dyndns.org","threadId":"29826","inReplyTo":"20130123023235.GA24135@dcvr.yhbt.net","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-24T05:29:01Z","receivedAt":"2013-01-24T05:29:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> diff --git a/git-svn.perl b/git-svn.perl\n> index c232798..e5bd292 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n> \t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n> \t} \"Unable to find .git directory\\n\";\n> \tmy $cdup = undef;\n> +\tmy $git_dir = delete $ENV{GIT_DIR};\n> \tgit_cmd_try {\n> \t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n> \t\tchomp $cdup if ($cdup);\n> \t\t$cdup = \".\" unless ($cdup && length $cdup);\n> -\t} \"Already at toplevel, but $ENV{GIT_DIR} not found\\n\";\n> +\t} \"Already at toplevel, but $git_dir not found\\n\";\n> +\t$ENV{GIT_DIR} = $git_dir;\n> \tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n> \t$_repository = Git->repository(Repository => $ENV{GIT_DIR});\n> }\n\nThis does not look quite right, though.\n\nCan't the user have his own $GIT_DIR when this command is invoked?\nThe first command_oneline() runs rev-parse with that environment and\nget the user specified value of GIT_DIR in $ENV{GIT_DIR}, but by\ndoing a \"delete\" before running --show-cdup, you are not honoring\nthat GIT_DIR (and GIT_WORK_TREE if exists) the user gave you. You\nalready used that GIT_DIR when you asked rev-parse --git-dir to find\nwhat the GIT_DIR value should be, so you would be operating with\nvalues of $git_dir and $cdup that you discovered in an inconsistent\nway, no?\n\nShouldn't it be more like this instead?\n\n\tmy ($git_dir, $cdup) = undef;\n try {\n\t\t$git_dir = command_oneline(qw(rev-parse --git-dir));\n\t} \"Unable to ...\";\n try {\n\t\t$cdup = command_oneline(qw(rev-parse --show-cdup));\n\t\t... tweak $cdup ...\n\t} \"Unable to ...\";\n\tif (defined $git_dir) { $ENV{GIT_DIR} = $git_dir; }\n\tchdir $cdup;\n"},{"id":"207686","messageId":"20130124101417.GA22138@dcvr.yhbt.net","threadId":"29826","inReplyTo":"7vehhbdu8y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/2] Make git-svn work with gitdir links","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2013-01-24T10:14:17Z","receivedAt":"2013-01-24T10:14:17Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> \n> > diff --git a/git-svn.perl b/git-svn.perl\n> > index c232798..e5bd292 100755\n> > --- a/git-svn.perl\n> > +++ b/git-svn.perl\n> > @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n> > \t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n> > \t} \"Unable to find .git directory\\n\";\n> > \tmy $cdup = undef;\n> > +\tmy $git_dir = delete $ENV{GIT_DIR};\n> > \tgit_cmd_try {\n> > \t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n> > \t\tchomp $cdup if ($cdup);\n> > \t\t$cdup = \".\" unless ($cdup && length $cdup);\n> > -\t} \"Already at toplevel, but $ENV{GIT_DIR} not found\\n\";\n> > +\t} \"Already at toplevel, but $git_dir not found\\n\";\n> > +\t$ENV{GIT_DIR} = $git_dir;\n> > \tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n> > \t$_repository = Git->repository(Repository => $ENV{GIT_DIR});\n> > }\n> \n> This does not look quite right, though.\n> \n> Can't the user have his own $GIT_DIR when this command is invoked?\n> The first command_oneline() runs rev-parse with that environment and\n> get the user specified value of GIT_DIR in $ENV{GIT_DIR}, but by\n> doing a \"delete\" before running --show-cdup, you are not honoring\n> that GIT_DIR (and GIT_WORK_TREE if exists) the user gave you. You\n> already used that GIT_DIR when you asked rev-parse --git-dir to find\n> what the GIT_DIR value should be, so you would be operating with\n> values of $git_dir and $cdup that you discovered in an inconsistent\n> way, no?\n> \n> Shouldn't it be more like this instead?\n> \n> \tmy ($git_dir, $cdup) = undef;\n> try {\n> \t\t$git_dir = command_oneline(qw(rev-parse --git-dir));\n> \t} \"Unable to ...\";\n> try {\n> \t\t$cdup = command_oneline(qw(rev-parse --show-cdup));\n> \t\t... tweak $cdup ...\n> \t} \"Unable to ...\";\n> \tif (defined $git_dir) { $ENV{GIT_DIR} = $git_dir; }\n> \tchdir $cdup;\n\nThanks, I'll squash the following and push a new branch. I don't\nbelieve the (defined $git_dir) check is necessary since we already\nchecked for errors with git_cmd_try.\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex e5bd292..b46795f 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -324,19 +324,18 @@ for (my $i = 0; $i < @ARGV; $i++) {\n \t}\n };\n \n # make sure we're always running at the top-level working directory\n if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {\n \t$ENV{GIT_DIR} ||= \".git\";\n } else {\n+\tmy ($git_dir, $cdup);\n \tgit_cmd_try {\n-\t\t$ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);\n+\t\t$git_dir = command_oneline([qw/rev-parse --git-dir/]);\n \t} \"Unable to find .git directory\\n\";\n-\tmy $cdup = undef;\n-\tmy $git_dir = delete $ENV{GIT_DIR};\n \tgit_cmd_try {\n \t\t$cdup = command_oneline(qw/rev-parse --show-cdup/);\n \t\tchomp $cdup if ($cdup);\n \t\t$cdup = \".\" unless ($cdup && length $cdup);\n \t} \"Already at toplevel, but $git_dir not found\\n\";\n \t$ENV{GIT_DIR} = $git_dir;\n \tchdir $cdup or die \"Unable to chdir up to '$cdup'\\n\";\n\n-- \nEric Wong\n"}]}