cc-rs icon indicating copy to clipboard operation
cc-rs copied to clipboard

Use SDKROOT when set.

Open dvc94ch opened this issue 3 years ago • 9 comments

This fixes #650

dvc94ch avatar Mar 07 '22 15:03 dvc94ch

Is there existing precedent for using an environment variable like this? If so is there documentation or precedent that this could link to? (I know nothing of this environment variable myself)

alexcrichton avatar Mar 07 '22 18:03 alexcrichton

well, I don't think there is much precedent for cross compiling ios applications from non macos systems without xcode installed, even though it is certainly possible.

xcode sets the SDKROOT for the target platform and both clang and rust pick it up from there. alternative the sdk root can be provided with --sysroot or -isystem. not sure there is much documentation.

dvc94ch avatar Mar 07 '22 18:03 dvc94ch

There's a fair bit of handling of SDKROOT in the Rust compiler around special strings and such, and none of that is mirrored here? That seems like it could cause bugs?

alexcrichton avatar Mar 08 '22 15:03 alexcrichton

well, worst case something doesn't compile. since apple believes everyone runs a mac, sdkroot should always be the target sdk and not the host sdk. this means that rust needs to remove the sdk root when compiling a build script or proc macro. without this change I can't cross compile any crate that uses cc-rs, like for example the objc-exception crate.

dvc94ch avatar Mar 08 '22 19:03 dvc94ch

Sorry but I've decided that the current development trajectory of cc is not one that I can maintain, so this crate will need to grow new maintainers before merging.

alexcrichton avatar Mar 10 '22 17:03 alexcrichton

I [for one] find all this environment variable chasing tedious and even counterproductive, so I what you would think of #664.

dot-asm avatar Mar 14 '22 16:03 dot-asm

This PR is explicitly about avoiding xcrun. I understand that that is fine for mac users, but this PR is explicitly about enabling linux users to build ios binaries. This by itself does not make developing ios applications possible from linux, but in conjunction with other efforts like the ones listed below would indeed make linux a suitable host environment for ios developers.

  • https://github.com/cloudpeers/x (cross compile)
  • https://github.com/indygreg/PyOxidizer/pull/515 (codesign)
  • https://github.com/dvc94ch/noxc (certificates and provisioning profiles)
  • https://github.com/libimobiledevice (install and run)

dvc94ch avatar Mar 14 '22 17:03 dvc94ch

This PR is explicitly about avoiding xcrun.

Currently xcrun is used to find sdk path and suggestion is to instead use xcrun to fire cc without having to bother with sdk path specifics in cc-rs. As for possible-future non-apple developments, why not aim for xcrun replacement there? I'd say it would be of great benefit for all projects...

dot-asm avatar Mar 14 '22 17:03 dot-asm

Tangentially related to this, I've implemented Apple SDK discovery in pure Rust in https://github.com/indygreg/PyOxidizer/blob/ae848c9083c9c8563d5890f5d39a8e04a1d71832/tugger-apple/src/sdk.rs. This includes parsing the SDKSettings.json files to extract SDK metadata such as versions and which targets are supported. Like xcode-select and xcrun, it supports honoring DEVELOPER_DIR for finding the install root of SDKs. There's also some related code at https://github.com/indygreg/PyOxidizer/blob/ae848c9083c9c8563d5890f5d39a8e04a1d71832/pyoxidizer/src/environment.rs#L531 (and elsewhere in this file) for attempting to find an appropriate SDK to use given constraints. (This code should arguably be moved to the aforementioned crate/module.)

This code probably isn't suitable for inclusion in this crate as-is. But if you want me to extract it to its own crate or incorporate it into cc-rs somehow, let's talk (in another issue presumably).

As for this PR, externally specifying SDKROOT to force the Apple SDK to use seems reasonable to me. This is what I've done with PyOxidizer to allow users to override the built-in/default SDK finding. I say this without knowing how the Rust compiler internally handles SDKROOT, so I may be missing some important context! But my understanding is that build tools commonly export SDKROOT to force use of a specific SDK to avoid downstream tools performing their own SDK discovery.

indygreg avatar Mar 15 '22 02:03 indygreg

cc @thomcc. I see you're the only one watching this crate from the crate maintainers team [0]. So I guess you're the maintainer of this crate now?

  • [0] https://github.com/rust-lang/team/pull/861

dvc94ch avatar Oct 25 '22 17:10 dvc94ch

Sure, cc is one of the crates I volunteered for, and one I have a vested interest in. Let me take a look.

thomcc avatar Oct 25 '22 17:10 thomcc

Apparently I don't have the ability to merge yet, though 😩.

thomcc avatar Oct 25 '22 17:10 thomcc

Can you rebase in order to remove the merge commit? I think we want a linear history like we do in the other repos.

thomcc avatar Oct 25 '22 19:10 thomcc

done

dvc94ch avatar Oct 25 '22 20:10 dvc94ch

Hi, I suspect that picking SDKROOT unconditionally from env has set a wrong sdk_path for cc when targeting {x86_64,aarch64}-apple-ios. Could you please take a look into it?

The bump that triggers the problem: rust-lang/rust#111701 Some log info: https://github.com/rust-lang/rust/pull/111701#issuecomment-1558707965

r-value avatar May 23 '23 11:05 r-value

when targeting ios you should set the SDKROOT to the ios sdk and not the host sdk. if you don't set the SDKROOT it should pick it up automatically.

dvc94ch avatar May 23 '23 13:05 dvc94ch

when targeting ios you should set the SDKROOT to the ios sdk and not the host sdk. if you don't set the SDKROOT it should pick it up automatically.

I think the SDKROOT is automatically set somewhere in the ci test and got it wrong for some reason. The code in rustc contains lots of code ignoring SDKROOT if it's clearly wrong.

r-value avatar May 23 '23 14:05 r-value

So set it correctly or don't...

dvc94ch avatar May 23 '23 14:05 dvc94ch

I guess we can check if the sdk root is set correctly and if it isn't ignore it. Seems like the apple toolchain doesn't care?

dvc94ch avatar May 23 '23 14:05 dvc94ch

I looked deeper into the ci workflow and found that SDKROOT might be set here: https://github.com/rust-lang/rust/blob/f3d597b31c0f101a02c230798afa31a36bdacbc6/src/ci/scripts/install-clang.sh#L30-L35 It used a value returned by xcrun and I think that's clearly wrong since we have multiple targets aside from macosx.

FYI the code filtering irrational env was originally introduced in commit https://github.com/rust-lang/rust/commit/fe6d626abcc6ca994f0554bb78858ccd2b33dcfd

r-value avatar May 23 '23 15:05 r-value

So set it correctly or don't...

fyi, I found something here: rust-lang/cargo#7283 resolved by rust-lang/rust#64254 Maybe SDKROOT is just wrong in some situation when cross building? tbh I'm not an apple dev so I can't understand the issue quite well.

I guess we can check if the sdk root is set correctly and if it isn't ignore it.

That's exactly what rustc did - perform a sanity check against SDKROOT and ignore it if it's wrong. But I don't have appropriate test environment so I guess we need someone else to implement and test it.

btw, shall we print some warning if the SDKROOT doesn't pass the sanity check? I don't know if it's a common situation that a wrong SDKROOT is automatically set.

r-value avatar May 25 '23 06:05 r-value

Maybe SDKROOT is just wrong in some situation when cross building?

No, it shouldn't be wrong when compiling for the target. All the links you mention are saying that when compiling native parts, target env variables weren't sanitized correctly.

Do build scripts compile in debug mode when passing the debug flag and release mode when passing the release flag? The point is that it doesn't really matter how build scripts are built, just needs to run on the machine it's built on.

dvc94ch avatar May 25 '23 09:05 dvc94ch

I haven't looked into the issues revealed after landing this deeply, but I think we should probably filter out irrational SDKROOTs.

They'll be wrong when running cc as a dependency of a build script (for example, if compiling for iOS, if you have a build script which wants to use lzma-sys or something to compress some data, then it will need to run cc and compile targeting the host).

We already do this for some environment variables.

thomcc avatar May 25 '23 14:05 thomcc

Maybe SDKROOT is just wrong in some situation when cross building?

No, it shouldn't be wrong when compiling for the target. All the links you mention are saying that when compiling native parts, target env variables weren't sanitized correctly.

Do build scripts compile in debug mode when passing the debug flag and release mode when passing the release flag? The point is that it doesn't really matter how build scripts are built, just needs to run on the machine it's built on.

The main issue is that this PR breaks the cross build CI of rustc. Any better proposals? I think we should sanitize SDKROOT anyway.

r-value avatar Aug 09 '23 04:08 r-value