New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use Windows path %PWD for SSH tests #2920

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@MarcelRaad
Member

MarcelRaad commented Aug 30, 2018

All these tests failed on Windows because something like
sftp://%HOSTIP:%SSHPORT%PWD/
expanded to
sftp://127.0.0.1:1234c:/msys64/home/bla/curl/
and then curl complained about the port number ending with a letter.

Use the original POSIX path instead instead of the Windows path created
in checksystem to fix this.

Don't use Windows path %PWD for SSH tests
All these tests failed on Windows because something like
sftp://%HOSTIP:%SSHPORT%PWD/
expanded to
sftp://127.0.0.1:1234c:/msys64/home/bla/curl
and then curl complained about the port number ending with a letter.

Use the original POSIX path instead instead of the Windows path created
in checksystem to fix this.

Closes
@MarcelRaad

This comment has been minimized.

Show comment
Hide comment
@MarcelRaad

MarcelRaad Aug 30, 2018

Member

TODO: remove extra "instead" from commit message.

The problem is still visible at https://curl.haxx.se/dev/log.cgi?id=20180824152941-32197#prob1388, but this tester has since disappeared.

Member

MarcelRaad commented Aug 30, 2018

TODO: remove extra "instead" from commit message.

The problem is still visible at https://curl.haxx.se/dev/log.cgi?id=20180824152941-32197#prob1388, but this tester has since disappeared.

@bagder

bagder approved these changes Aug 30, 2018

@@ -3200,6 +3201,7 @@ sub subVariables {
$$thing =~ s/%CURL/$CURL/g;
$$thing =~ s/%PWD/$pwd/g;
$$thing =~ s/%POSIX_PWD/$posix_pwd/g;

This comment has been minimized.

@bagder

bagder Aug 30, 2018

Member

But is there a reason to keep the old one? I mean couldn't you just use this method for %PWD ?

@bagder

bagder Aug 30, 2018

Member

But is there a reason to keep the old one? I mean couldn't you just use this method for %PWD ?

This comment has been minimized.

@MarcelRaad

MarcelRaad Aug 31, 2018

Member

The Windows path version in %PWD is still required to pass to a native Windows curl in the general case, e.g. when using the FILE protocol or just local file names. The new one is only required for OpenSSH.

@MarcelRaad

MarcelRaad Aug 31, 2018

Member

The Windows path version in %PWD is still required to pass to a native Windows curl in the general case, e.g. when using the FILE protocol or just local file names. The new one is only required for OpenSSH.

This comment has been minimized.

@bagder

bagder Aug 31, 2018

Member

aah, ok. Thanks!

@bagder

bagder Aug 31, 2018

Member

aah, ok. Thanks!

@MarcelRaad MarcelRaad deleted the MarcelRaad:ssh_msys branch Aug 31, 2018

falconindy added a commit to falconindy/curl that referenced this pull request Sep 10, 2018

Don't use Windows path %PWD for SSH tests
All these tests failed on Windows because something like
sftp://%HOSTIP:%SSHPORT%PWD/
expanded to
sftp://127.0.0.1:1234c:/msys64/home/bla/curl
and then curl complained about the port number ending with a letter.

Use the original POSIX path instead of the Windows path created in
checksystem to fix this.

Closes curl#2920
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment