Junio C Hamano <[email protected]> wrote:
> Eric Wong <[email protected]> writes:
>
> > diff --git a/git-svn.perl b/git-svn.perl
> > index c232798..e5bd292 100755
> > --- a/git-svn.perl
> > +++ b/git-svn.perl
> > @@ -332,11 +332,13 @@ if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
> > $ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);
> > } "Unable to find .git directory\n";
> > my $cdup = undef;
> > + my $git_dir = delete $ENV{GIT_DIR};
> > git_cmd_try {
> > $cdup = command_oneline(qw/rev-parse --show-cdup/);
> > chomp $cdup if ($cdup);
> > $cdup = "." unless ($cdup && length $cdup);
> > - } "Already at toplevel, but $ENV{GIT_DIR} not found\n";
> > + } "Already at toplevel, but $git_dir not found\n";
> > + $ENV{GIT_DIR} = $git_dir;
> > chdir $cdup or die "Unable to chdir up to '$cdup'\n";
> > $_repository = Git->repository(Repository => $ENV{GIT_DIR});
> > }
>
> This does not look quite right, though.
>
> Can't the user have his own $GIT_DIR when this command is invoked?
> The first command_oneline() runs rev-parse with that environment and
> get the user specified value of GIT_DIR in $ENV{GIT_DIR}, but by
> doing a "delete" before running --show-cdup, you are not honoring
> that GIT_DIR (and GIT_WORK_TREE if exists) the user gave you. You
> already used that GIT_DIR when you asked rev-parse --git-dir to find
> what the GIT_DIR value should be, so you would be operating with
> values of $git_dir and $cdup that you discovered in an inconsistent
> way, no?
>
> Shouldn't it be more like this instead?
>
> my ($git_dir, $cdup) = undef;
> try {
> $git_dir = command_oneline(qw(rev-parse --git-dir));
> } "Unable to ...";
> try {
> $cdup = command_oneline(qw(rev-parse --show-cdup));
> ... tweak $cdup ...
> } "Unable to ...";
> if (defined $git_dir) { $ENV{GIT_DIR} = $git_dir; }
> chdir $cdup;
Thanks, I'll squash the following and push a new branch. I don't
believe the (defined $git_dir) check is necessary since we already
checked for errors with git_cmd_try.
diff --git a/git-svn.perl b/git-svn.perl
index e5bd292..b46795f 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -324,19 +324,18 @@ for (my $i = 0; $i < @ARGV; $i++) {
}
};
# make sure we're always running at the top-level working directory
if ($cmd && $cmd =~ /(?:clone|init|multi-init)$/) {
$ENV{GIT_DIR} ||= ".git";
} else {
+ my ($git_dir, $cdup);
git_cmd_try {
- $ENV{GIT_DIR} = command_oneline([qw/rev-parse --git-dir/]);
+ $git_dir = command_oneline([qw/rev-parse --git-dir/]);
} "Unable to find .git directory\n";
- my $cdup = undef;
- my $git_dir = delete $ENV{GIT_DIR};
git_cmd_try {
$cdup = command_oneline(qw/rev-parse --show-cdup/);
chomp $cdup if ($cdup);
$cdup = "." unless ($cdup && length $cdup);
} "Already at toplevel, but $git_dir not found\n";
$ENV{GIT_DIR} = $git_dir;
chdir $cdup or die "Unable to chdir up to '$cdup'\n";
--
Eric Wong
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
