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

Rebuild if deployment target changes #118204

Open
madsmtm opened this issue Nov 23, 2023 · 3 comments · May be fixed by #129342
Open

Rebuild if deployment target changes #118204

madsmtm opened this issue Nov 23, 2023 · 3 comments · May be fixed by #129342
Labels
C-bug Category: This is a bug. O-ios Operating system: iOS O-macos Operating system: macOS O-tvos Operating system: tvOS (including simulator) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@madsmtm
Copy link
Contributor

madsmtm commented Nov 23, 2023

rustc reads the MACOSX_DEPLOYMENT_TARGET environment variable (and similar other *_DEPLOYMENT_TARGET variables), and uses that value to determine (probably amongst other things) certain options for LLVM and the linker.

However, it seems like this environment variable is not tracked as an external dependency, meaning that changing it does not cause rustc/cargo to rebuild the target, as is otherwise expected when changing environment variables that modify compilation.

E.g. running the following code:

cargo clean
MACOSX_DEPLOYMENT_TARGET=12.0 cargo build
MACOSX_DEPLOYMENT_TARGET=13.0 cargo build

I expected to see this happen: The project was built twice.

Instead, this happened: The project was built once, and then the cache was used the second time.

Meta

rustc --version --verbose:

rustc 1.76.0-nightly (2f8d81f9d 2023-11-21)
binary: rustc
commit-hash: 2f8d81f9dbac6b8df982199f69da04a4c8357227
commit-date: 2023-11-21
host: aarch64-apple-darwin
release: 1.76.0-nightly
LLVM version: 17.0.5
@madsmtm madsmtm added the C-bug Category: This is a bug. label Nov 23, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 23, 2023
@madsmtm
Copy link
Contributor Author

madsmtm commented Nov 23, 2023

I know that a crate can emit cargo:rerun-if-env-changed=MACOSX_DEPLOYMENT_TARGET in their build.rs to opt in to this behaviour, but I'm not sure that's enough if there are other cached crates in the dependency graph that may have been built with a different deployment target?

@Noratrieb Noratrieb added O-macos Operating system: macOS T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Dec 4, 2023
@Noratrieb
Copy link
Member

This also affects other apple targets with their *_DEPLOYMENT_TARGETs. Looks like the relevant code should somehow add the env var to sess.parse_sess.env_depinfo. I'm not sure what the nicest way to ensure that would be.
cc @BlackHoleFox

@madsmtm
Copy link
Contributor Author

madsmtm commented May 3, 2024

I'll add that this also affects the environment variable SDKROOT

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 27, 2024
…trochenkov

Move Apple linker args from `rustc_target` to `rustc_codegen_ssa`

They are dependent on the deployment target and SDK version, but having these in `rustc_target` makes it hard to introduce that dependency. Part of the work needed to do rust-lang#118204, see rust-lang#129342 for some discussion.

Tested using:
```console
./x test tests/run-make/apple-deployment-target --target="aarch64-apple-darwin,aarch64-apple-ios,aarch64-apple-ios-macabi,aarch64-apple-ios-sim,aarch64-apple-tvos,aarch64-apple-tvos-sim,aarch64-apple-visionos,aarch64-apple-visionos-sim,aarch64-apple-watchos,aarch64-apple-watchos-sim,arm64_32-apple-watchos,armv7k-apple-watchos,armv7s-apple-ios,x86_64-apple-darwin,x86_64-apple-ios,x86_64-apple-ios-macabi,x86_64-apple-tvos,x86_64-apple-watchos-sim,x86_64h-apple-darwin"
IPHONEOS_DEPLOYMENT_TARGET=10.0 ./x test tests/run-make/apple-deployment-target --target=i386-apple-ios
```

`arm64e-apple-darwin` and `arm64e-apple-ios` have not been tested, see rust-lang#130085, neither is `i686-apple-darwin`, since that requires using an x86_64 macbook, and I currently can't get mine to work, see rust-lang#130434.

CC `@petrochenkov`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 27, 2024
Rollup merge of rust-lang#130435 - madsmtm:move-apple-link-args, r=petrochenkov

Move Apple linker args from `rustc_target` to `rustc_codegen_ssa`

They are dependent on the deployment target and SDK version, but having these in `rustc_target` makes it hard to introduce that dependency. Part of the work needed to do rust-lang#118204, see rust-lang#129342 for some discussion.

Tested using:
```console
./x test tests/run-make/apple-deployment-target --target="aarch64-apple-darwin,aarch64-apple-ios,aarch64-apple-ios-macabi,aarch64-apple-ios-sim,aarch64-apple-tvos,aarch64-apple-tvos-sim,aarch64-apple-visionos,aarch64-apple-visionos-sim,aarch64-apple-watchos,aarch64-apple-watchos-sim,arm64_32-apple-watchos,armv7k-apple-watchos,armv7s-apple-ios,x86_64-apple-darwin,x86_64-apple-ios,x86_64-apple-ios-macabi,x86_64-apple-tvos,x86_64-apple-watchos-sim,x86_64h-apple-darwin"
IPHONEOS_DEPLOYMENT_TARGET=10.0 ./x test tests/run-make/apple-deployment-target --target=i386-apple-ios
```

`arm64e-apple-darwin` and `arm64e-apple-ios` have not been tested, see rust-lang#130085, neither is `i686-apple-darwin`, since that requires using an x86_64 macbook, and I currently can't get mine to work, see rust-lang#130434.

CC `@petrochenkov`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 1, 2024
…g, r=petrochenkov

Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`

Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc.

We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion.

The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session.

Tested with the same commands as in rust-lang#130435.

r? `@petrochenkov`
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Nov 2, 2024
…g, r=petrochenkov

Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`

Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc.

We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion.

The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session.

Tested with the same commands as in rust-lang#130435.

r? ```@petrochenkov```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 2, 2024
…g, r=petrochenkov

Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`

Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc.

We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion.

The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session.

Tested with the same commands as in rust-lang#130435.

r? ````@petrochenkov````
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Nov 2, 2024
…g, r=petrochenkov

Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`

Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc.

We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion.

The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session.

Tested with the same commands as in rust-lang#130435.

r? `````@petrochenkov`````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 2, 2024
…g, r=petrochenkov

Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`

Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc.

We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion.

The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session.

Tested with the same commands as in rust-lang#130435.

r? ``````@petrochenkov``````
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 2, 2024
Rollup merge of rust-lang#131037 - madsmtm:move-llvm-target-versioning, r=petrochenkov

Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa`

Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc.

We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion.

The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session.

Tested with the same commands as in rust-lang#130435.

r? ``````@petrochenkov``````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-ios Operating system: iOS O-macos Operating system: macOS O-tvos Operating system: tvOS (including simulator) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants