Karthik R <karthikr@xxxxxxxxxxx> writes:
>> Two questions.
>>
>> - What if a user has SVN_SSH exported _and_ wants to use a different one
>> from the one s/he uses for git? Naturally such a user would set both
>> environment variables and differently, but this seems to override the
>> value in SVN_SSH;
>>
> Do you mean user wants to use a different one with "git svn
> ... svn+ssh://" (than the one with "git clone ssh://") ?
Yes.
> In this case
> - defining SVN_SSH, but not GIT_SSH will still work (with this patch,
> GIT_SSH overrides)
Which means if you need to use GIT_SSH to specify one and SVN_SSH to
specify another, you have trouble. IOW, you cannot use anything but
whatever the default is for native git access over ssh:// protocol.
> - but SVN_SSH needs to have \\s.
>
> So unless the user already knew of this quirk, we'll only see
> unescaped \s - so it *does* make sense to escape the \s (if the user
> knew, then too many escaped \s still work).
>
>> - Can a user have SVN_SSH exported, on MSWin32 or msys, and use svn
>> outside git? If so, what does the value of SVN_SSH look like? Does it
>> typically have necessary doubling of backslashes already?
>>
> With subversion for Windows, these \\s are not needed (but doesn't
> cause any break). The doubling is something to do with the bash (in
> msys) I think.
>
Ok, so does that mean the logic should look more like the one you quoted
below without saying yes/no/anything? The points are:
(1) do not muck with SVN_SSH if already given by the user.
(2) when and only when we reuse value from GIT_SSH for SVN_SSH, double
the backslashes.
>> What I am getting at is, if the patch should look something like this
>> instead:
>>
>> if (! exists $ENV{SVN_SSH}) {
>> if (exists $ENV{GIT_SSH}) {
>> $ENV{SVN_SSH} = $ENV{GIT_SSH};
>> if ($^O eq 'MSWin32' || $^O eq 'msys') {
>> $ENV{SVN_SSH} =~ s/\\/\\\\/g;
>> }
>> }
>> }
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html