Skip to content
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 from

Conversation

MarcelRaad
Copy link
Member

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.

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
Copy link
Member Author

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.

@@ -3200,6 +3201,7 @@ sub subVariables {

$$thing =~ s/%CURL/$CURL/g;
$$thing =~ s/%PWD/$pwd/g;
$$thing =~ s/%POSIX_PWD/$posix_pwd/g;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aah, ok. Thanks!

@MarcelRaad MarcelRaad deleted the ssh_msys branch August 31, 2018 07:15
falconindy pushed a commit to falconindy/curl that referenced this pull request Sep 10, 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 of the Windows path created in
checksystem to fix this.

Closes curl#2920
@lock lock bot locked as resolved and limited conversation to collaborators Nov 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants