rust-lang / docs.rs

crates.io documentation generator
https://docs.rs
MIT License
989 stars 198 forks source link

Docs not built on target platform, causing failures #1957

Open CamJN opened 1 year ago

CamJN commented 1 year ago

Crate name

getargv

Build failure link

https://docs.rs/crate/getargv/0.1.1/builds/701636

Additional details

This crate has a dep getargv-sys which requires an apple OS to build, I specify:

[package.metadata.docs.rs]
targets = ["x86_64-apple-darwin", "aarch64-apple-darwin"]

In both crates' Cargo.toml; but when docs.rs builds the getargv crate, the getargv-sys crate is built with an incompatible system/command.

CamJN commented 1 year ago

Actually I just checked and it fails to build getargv-sys directly too... I see in the log that the build command has the "--target" "x86_64-apple-darwin" flag, but does it actually do anything? the build script still seems to know it's not an apple OS.

CamJN commented 1 year ago

I can detect building in docs.rs and bypass the OS check, but the sys/types.h header will be wrong on a non-apple OS so the generated docs will probably be wrong.

CamJN commented 1 year ago

not mine but i noticed https://docs.rs/crate/mac-notification-sys/0.5.6 is broken too since docs.rs doesn't build macOS sys crates on macOS, and i imagine there are others too.

syphar commented 1 year ago

Thank you for the report!

Docs.rs currently only builds x86_64-unknown-linux-gnu natively and uses cross-compilation for all other targets (see here).

I see you fixed this (e.g. worked around the limitation) for getargv and getargv-sys already, does this mean this issue is solved for you?

Building on native targets is a bigger topic and not easily solveable right now.

CamJN commented 1 year ago

Yeah I made it build, but I had to shim in a header, and I was wondering what the current advice is for bindgen based crates on non-linux platforms?

I'm also still a bit confused why cfg!(not(target_vendor = "apple")) does not reflect the target triple set with the --target flag in the build command.

Nemo157 commented 1 year ago

I'm also still a bit confused why cfg!(not(target_vendor = "apple")) does not reflect the target triple set with the --target flag in the build command.

The build script is always built for the host, which is what cfg! checks; if you want to know what target it is being run for you need to inspect the environment variable CARGO_CFG_TARGET_VENDOR while it is running.

jyn514 commented 1 year ago

potential mis-compilation of docs

I think this bit is not true, the cfg was set correctly, the build script was just interpreting it wrong.

CamJN commented 1 year ago

@jyn514 it can very much cause miscompilation. If you run bindgen on the wrong platform it might pick up a header with a type defined for say x86 linux instead of aarch64 macOS, which would result in mistakes in the generated docs.

jyn514 commented 1 year ago

@CamJN that sounds like a bug in bindgen, that would break when cross-compiling as well.

CamJN commented 1 year ago

Cross compiling anything that links to C requires you to have all of the C headers of the target installed. Bindgen supports this fine by allowing you to specify where to look for C stdlib headers, your environment does not provide these things, so cross compiling does not work properly.

CamJN commented 1 year ago

If you'd like some docs on how to get the headers this does a decent job of explaining what's required: https://github.com/tpoechtrager/osxcross#packaging-the-sdk

Nemo157 commented 1 year ago

Xcode's license terms do not allow using it for cross-compiling from non-Apple hardware. I don't believe AWS offers Linux-on-Apple-hardware, so there's really no way we can support cross-compilation that requires the Xcode SDK, the only solution would be setting up native macOS build instances (something I think will eventually be needed to properly support macOS-only libraries, but is not going to happen soon as there's a lot of architectural questions that would have to be resolved first).

CamJN commented 1 year ago

Fair enough, in looking at the headers that I use specifically the license seems fine (it's pretty much bsd-4-clause + don't pirate macOS), but the license for the whole sdk is probably not compatible.

You may want to put something in your docs or elsewhere that generated docs for crates that link to C and are cross-compiled will likely have incorrect types atm, since I don't believe the headers are present for any non-linux OS, and I don't know if the arch is set at the docker image level (ie building arm docs will see arm C-headers) or just at the cargo level.

This is probably an even bigger problem for crates used for embedded development, since those machines are more likely to have unusual C-type sizes.