Fix missing declarations of "POSIX" read()/write() on Windows with Clang#45
Fix missing declarations of "POSIX" read()/write() on Windows with Clang#45MarijnS95 wants to merge 1 commit intoKarypisLab:masterfrom
read()/write() on Windows with Clang#45Conversation
|
Unfortunately, I do not have access to a windows machine to test the above. Can other people verify that the above fix works across recent versions of windows? |
|
Hey @karypis, if I remember correctly this also happened specifically while cross-compiling the mentioned Rust crate referencing this library from a Linux/MacOS machine to Windows using |
|
Actually, I figured, why not compile natively on Windows using MarijnS95@6760c62 |
|
Of course, applying the commit from this PR on top fixes the issue: https://github.com/MarijnS95/GKlib/actions/runs/16081310139/job/45386384098 The deprecation warning is still there, maybe we should use Note that the MSVC build still fails. Once that's over, do you mind if I PR my other |
…h Clang
When (cross?) compiling this crate to Windows (in a Rust project) using
Clang 16 or newer, we're seeing these declaration errors:
> cargo b --target x86_64-pc-windows-msvc
warning: metis-sys@0.3.1: /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/metis-sys-0.3.1/vendor/GKlib/io.c(63,18): error: call to undeclared function 'read'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
warning: metis-sys@0.3.1: 63 | if ((rsize = read(fd, buf, tsize)) == -1)
warning: metis-sys@0.3.1: | ^
...
warning: metis-sys@0.3.1: /usr/local/cargo/registry/src/index.crates.io-1949cf8c6b5b557f/metis-sys-0.3.1/vendor/GKlib/io.c(84,17): error: call to undeclared function 'write'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
warning: metis-sys@0.3.1: 84 | if ((size = write(fd, buf, tsize)) == -1)
warning: metis-sys@0.3.1: | ^
(LIHPC-Computational-Geometry/metis-rs#43 (comment))
Now it's yet unknown (we haven't checked) if older Clang had these
declarations in a header, or didn't treat this warning as error yet,
but the functions are POSIX which Windows isn't required to implement.
Fortunately they provide it but with a deprecation warning and a
recommended replacement of `_read()` (and `_write()`), but for this
`<io.h>` needs to be included:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/posix-read
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/read
The same is true for `getpid()` that commit a8e7e25 ("port to vs2017")
used to provide a `win32/adapt.c/h` workaround for, but this can instead
be pulled from `<process.h>` (equally with a deprecation warning, but
we should probably address these all in a consistent manner instead
of defining a workaround). Let's take this as a starting point and go
from there.
9cb7831 to
2230482
Compare
|
Note that this PR implictly fixes yet another build issue caused by the recent merge of the https://github.com/MarijnS95/GKlib/actions/runs/16098906662/job/45425457255#step:3:444 If I'm not mistaken this is caused by this removal: 84658df#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aL13 As I don't think adding a single header file to the sources makes the entire surrounding folder part of the include directories: 84658df#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR66 |
I can confirm this fixed my Windows build issue. |
|
Thanks for testing @mattmatician. I'm still waiting for @karypis to take a look at #46 which will test this in CI instead to demonstrate compatibility directly in PRs rather than requiring many users to test on a variety of setups. |
When (cross?) compiling this crate to Windows (in a Rust project) using Clang 16 or newer, we're seeing these declaration errors:
(LIHPC-Computational-Geometry/metis-rs#43 (comment))
Now it's yet unknown (we haven't checked) if older Clang had these declarations in a header, or didn't treat this warning as error yet, but the functions are POSIX which Windows isn't required to implement. Fortunately they provide it but with a deprecation warning and a recommended replacement of
_read()(and_write()), but for this<io.h>needs to be included:https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/posix-read
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/read
The same is true for
getpid()that commit a8e7e25 ("port to vs2017") used to provide awin32/adapt.c/hworkaround for, but this can instead be pulled from<process.h>.This warning is already disabled by
CMakeusing_CRT_SECURE_NO_DEPRECATE, but only forMSVCbuilds but not yet when targeting clang (s/MSVC/WIN32to fix):GKlib/cmake/GKlibSystem.cmake
Line 6 in 84658df