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

remove requirement on HAVE_SYS_UN_H for unix domain sockets on Windows #7737

Closed
wants to merge 1 commit into from

Conversation

@mzr
Copy link
Contributor

@mzr mzr commented Sep 17, 2021

As described in #5162 curl with option --unix-socket on Windows was failing with:

* sa_addr inet_ntop() failed with errno 10047: Address family not supported

It happened because in Curl_addr2string's switch statement, case with AF_UNIX was behind define(HAVE_SYS_UN_H) && defined(AF_UNIX). There is no sys/un.h on Windows. On Windows sockaddr_un is defined in afunix.h (not in sys/un.h as it's on unix-like OSs). After this change, curl with --unix-socket worked for me as expected.

I'm not sure if this is the proper way to fix this, so I'd welcome some feedback. Also, I'm counting on tests that should run on this PR to see if things are OK.

bagder
bagder approved these changes Sep 17, 2021
Copy link
Member

@bagder bagder left a comment

👍

@bagder
Copy link
Member

@bagder bagder commented Sep 17, 2021

It seems to not be quite true everywhere:

connect.c: In function 'Curl_addr2string':
connect.c:663:48: error: invalid use of undefined type 'struct sockaddr_un'
  663 |         msnprintf(addr, MAX_IPADR_LEN, "%s", su->sun_path);
      |                                                ^~

@mzr
Copy link
Contributor Author

@mzr mzr commented Sep 19, 2021

I'll take a look into that. I'm guessing there is a mismatch between different toolchains/compilers? I'm not very familiar with Windows world :/ Also I might have had wrong compilation options (or just missing something) when I was compiling it myself on my machine - I was just using this as is. However, I think this might be unlikely as HAVE_SYS_UN_H seems like it's required for unix sockets to work and doesn't seem to have much sense on Windows.

@gvanem
Copy link
Contributor

@gvanem gvanem commented Sep 19, 2021

I'm guessing there is a mismatch between different toolchains/compilers?

The MinGW I have (TDM-gcc 8.0.0) have AF_UNIX defined in it's winsock*.h. But no struct sockaddr_un anywhere.
So maybe this somewhere:

#if !defined(HAVE_SOCKADDR_UN)
struct sockaddr_un {
       sa_family_t sun_family;   
       char        sun_path [200];
     };
#endif

@mzr
Copy link
Contributor Author

@mzr mzr commented Sep 19, 2021

Hmm, I see there is something similar already in the code here. Introduced in
#7034

@mzr mzr force-pushed the windows-unix-socket branch from 73bec01 to 23e31db Sep 19, 2021
@mzr
Copy link
Contributor Author

@mzr mzr commented Sep 19, 2021

Ahh this doesn't seem to do the trick as well. I think it might work but only with cmake build.

@mzr
Copy link
Contributor Author

@mzr mzr commented Sep 20, 2021

I added a little change, so it should now work not only with cmake.

@mzr mzr requested a review from bagder Sep 20, 2021
lib/connect.c Outdated Show resolved Hide resolved
@mzr mzr force-pushed the windows-unix-socket branch from 757c00b to 429917b Sep 28, 2021
@mzr
Copy link
Contributor Author

@mzr mzr commented Sep 29, 2021

I changed it to something much simpler. It should work for all cases and builds where it previously worked and for cases where sockaddr_un is manually defined on Windows in curl_setup.h. Might not be perfect, but it's still an improvement.
Tests that are failing I believe are not related.

@mzr mzr force-pushed the windows-unix-socket branch from 429917b to 584b7e6 Sep 29, 2021
@mzr
Copy link
Contributor Author

@mzr mzr commented Sep 29, 2021

rebased

@mzr mzr requested a review from bagder Sep 29, 2021
bagder
bagder approved these changes Sep 30, 2021
@bagder
Copy link
Member

@bagder bagder commented Sep 30, 2021

Thanks!

@bagder bagder closed this in 0fe9018 Sep 30, 2021
mzr added a commit to mzr/curl that referenced this issue Oct 2, 2021
@mzr mzr deleted the windows-unix-socket branch Oct 2, 2021
facebook-github-bot added a commit to facebookexperimental/eden that referenced this issue Oct 4, 2021
Summary:
Vendoring this patch: curl/curl#7737 to curl-sys rust crate. On windows the hg client is using curl that's bundled with sys-curl. I need this patch to have unix domain sockets working in hg client on windows.

I had to manually vendor https://raw.githubusercontent.com/mzr/curl/57e7ec4dbe4dd2831de51f2644879387d2ea7b44/docs/INSTALL because reindeer didn't do it. IDK why.

oss-eden-{darwin,linux,windows}-getdeps fail with:
```
FAILED: eden/scm/lib/backingstore/CMakeFiles/rust_backingstore.cargo eden/scm/lib/backingstore/debug/libbackingstore.a eden/scm/lib/backingstore/release/libbackingstore.a
cd /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E remove -f /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore/Cargo.lock && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E env CARGO_TARGET_DIR=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/eden/scm/lib/backingstore CARGO_HOME=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/_cargo_home cargo build --release -p backingstore --features fb
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on package cache
error: failed to calculate checksum of: /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL
Caused by:
  failed to open file `/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL`
Caused by:
  No such file or directory (os error 2)
```

Not idea how to fix this. Seems related to the fact that reindeer didn't vendor docs/INSTALL.

# EDIT:
# It might been caused by some bug in hg. now it's fine
# The failure in oss-eden-linux-getdeps looks unrelated (something with rocksdb)

Reviewed By: krallin

Differential Revision: D31370778

fbshipit-source-id: a1245f8cb6b58f5765e34c95dfd78325a8e6e457
facebook-github-bot added a commit to facebook/hhvm that referenced this issue Oct 4, 2021
Summary:
Vendoring this patch: curl/curl#7737 to curl-sys rust crate. On windows the hg client is using curl that's bundled with sys-curl. I need this patch to have unix domain sockets working in hg client on windows.

I had to manually vendor https://raw.githubusercontent.com/mzr/curl/57e7ec4dbe4dd2831de51f2644879387d2ea7b44/docs/INSTALL because reindeer didn't do it. IDK why.

oss-eden-{darwin,linux,windows}-getdeps fail with:
```
FAILED: eden/scm/lib/backingstore/CMakeFiles/rust_backingstore.cargo eden/scm/lib/backingstore/debug/libbackingstore.a eden/scm/lib/backingstore/release/libbackingstore.a
cd /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E remove -f /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore/Cargo.lock && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E env CARGO_TARGET_DIR=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/eden/scm/lib/backingstore CARGO_HOME=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/_cargo_home cargo build --release -p backingstore --features fb
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on package cache
error: failed to calculate checksum of: /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL
Caused by:
  failed to open file `/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL`
Caused by:
  No such file or directory (os error 2)
```

Not idea how to fix this. Seems related to the fact that reindeer didn't vendor docs/INSTALL.

# EDIT:
# It might been caused by some bug in hg. now it's fine
# The failure in oss-eden-linux-getdeps looks unrelated (something with rocksdb)

Reviewed By: krallin

Differential Revision: D31370778

fbshipit-source-id: a1245f8cb6b58f5765e34c95dfd78325a8e6e457
facebook-github-bot added a commit to facebookexperimental/rust-shed that referenced this issue Oct 4, 2021
Summary:
Vendoring this patch: curl/curl#7737 to curl-sys rust crate. On windows the hg client is using curl that's bundled with sys-curl. I need this patch to have unix domain sockets working in hg client on windows.

I had to manually vendor https://raw.githubusercontent.com/mzr/curl/57e7ec4dbe4dd2831de51f2644879387d2ea7b44/docs/INSTALL because reindeer didn't do it. IDK why.

oss-eden-{darwin,linux,windows}-getdeps fail with:
```
FAILED: eden/scm/lib/backingstore/CMakeFiles/rust_backingstore.cargo eden/scm/lib/backingstore/debug/libbackingstore.a eden/scm/lib/backingstore/release/libbackingstore.a
cd /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E remove -f /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore/Cargo.lock && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E env CARGO_TARGET_DIR=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/eden/scm/lib/backingstore CARGO_HOME=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/_cargo_home cargo build --release -p backingstore --features fb
    Blocking waiting for file lock on package cache
    Blocking waiting for file lock on package cache
error: failed to calculate checksum of: /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL
Caused by:
  failed to open file `/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL`
Caused by:
  No such file or directory (os error 2)
```

Not idea how to fix this. Seems related to the fact that reindeer didn't vendor docs/INSTALL.

# EDIT:
# It might been caused by some bug in hg. now it's fine
# The failure in oss-eden-linux-getdeps looks unrelated (something with rocksdb)

Reviewed By: krallin

Differential Revision: D31370778

fbshipit-source-id: a1245f8cb6b58f5765e34c95dfd78325a8e6e457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants