This fixes issue #1318 - on Solarish systems __posix_readdir_r is only #1319
Conversation
defined for 32-bit Solarish targets. Since rust doesn't currently support 32-bit Solarish targets, the line is both unnecessary and wrong.
|
r? @gnzlbg (rust_highfive has picked a reviewer for you, use r? to override) |
|
It would be nice if someone with solaris experience could take a look, maybe @bgermann ? |
|
It can be confirmed by running |
|
I would be surprised if Solaris has changed behavior. This appears to have been the case since at least 2006 (looking at what symbols are exported in libc in the commit history which goes back to the OpenSolaris launch) which predates the Solaris/illumos split, and was strictly for backwards compatibility. It looks like what happened was prior to 64-bit systems arriving on the scene, the interface was changed by POSIX, so the old interface (taking only two arguments) was left for backwards compatibility, while the final POSIX standard interface was added (using some header tricks for new programs to use the new version -- |
|
The change is accurate. Please also see |
|
Thank you for taking a look. Would it be possible to add a documentation
comment explaining that and containing the relevant links? And maybe extend
the commit message with this information as well?
There is a lot of knowledge that remains spread in the PRs that is hard to
find when looking at a blame or when somebody tries to make a change. Right
now me and Alex are the single points of failure when it comes to remember
where to find what or who to ping, and I am trying to slowly change that.
Particularly for linkage, which is specially tricky due to how libc in
crates.io interacts with the built-in one.
|
|
Sure -- would something like work? I figure it'll be easier to hash out the wording here than do a lot of churn on the PR. I used the illumos man link since that's likely to be a more stable URL than docs.oracle.com (I think they've reorganized things at least once or twice on that site, making older links no longer work). |
|
I'd rather err in the side of too many links, and add the links that @bgermann mentioned to the comment as well, but otherwise LGTM. I'll merge when the PR is updated. Thank you both! |
…nd 64-bit libc for readdir_r
|
@bors: r+ |
|
📌 Commit e45fa4a has been approved by |
This fixes issue #1318 - on Solarish systems __posix_readdir_r is only defined for 32-bit Solarish targets. Since rust doesn't currently support 32-bit Solarish targets, the line is both unnecessary and wrong. As found with the latest fd binary, removing the line fixed the fd build (and fd then works as expected).
|
☀️ Test successful - checks-cirrus, checks-travis, status-appveyor |
defined for 32-bit Solarish targets. Since rust doesn't currently support 32-bit Solarish targets, the line is both unnecessary and wrong.
As found with the latest fd binary, removing the line fixed the fd build (and fd then works as expected).