[EXPERIMENT] Crater adding target_family = "linux"
#124355
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use |
rustbot
added
S-waiting-on-review
|
These commits modify compiler targets. |
|
Someday I will remember to r? @ghost preemptively. |
This comment has been minimized.
This comment has been minimized.
|
Failed to set assignee to
|
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 24, 2024
…ux, r=<try> [EXPERIMENT] Crater adding `target_family = "linux"` The lang team proposed, a while back, [deprecating `target_vendor`](rust-lang#100343). With five operating systems all from Apple, using very similar, unifying characteristics for each, it is untenable to ask people to `cfg(any(target_os = "macos", target_os = "ios", target_os = "watchos", target_os = "tvos", target_os = "visionos"))`. It is commonly preferred to use `target_vendor = "apple"` for things that are true for all of these OS variants, and we have to offer a replacement that is equally parsimonious. [The "obvious" choice is to add a `target_family = "apple"`](rust-lang/lang-team#102 (comment)). However, [there is a concern that adding a `target_family = "apple"` won't work without massive ecosystem breakage, because of poorly-designed build scripts](rust-lang/lang-team#102 (comment)). But we currently can't crater a test of that effectively, because most tests only test host compilation, and crater only runs on Linux. We could, however, test adding a hypothetical `target_family = "linux"`! This would be a hypothetical `target_family` that unites both `target_os = "linux"` and `target_os = "android"` targets. This isn't necessarily an "apples to apples" comparison (more like "penguins to robots"?) but it seems worth trying. Note that this is **not** a proposal to actually add such a `target_family`. I don't have any arguments about the merits beyond "it seems like a plausible target family we may one day add, it can be added in a way that disrupts relevant build scripts, and it impacts a 'high-value' target". For maximum coverage I have in fact added the relevant `target_family = "apple"` and `target_family = "linux"` also just in case any test suites somehow run cross-compilation tests.
This comment has been minimized.
This comment has been minimized.
|
☀️ Try build successful - checks-actions |
|
cc @Urgau I'm not quite sure how far the check-cfg work has come along. Should I be concerned about these test failures as an omen for this crater run? |
Urgau
reviewed
Apr 25, 2024
The rustc
No, no need to be concerned; most of the check-cfg tests are regression tests, they are mostly here to make sure that if something changes, it is intentional. you can bless them when you're ready |
Co-authored-by: Urgau <[email protected]>
|
Okay! Thanks for that info! I thought about it and I think this is the only thing that can possibly bring us closer to an answer, atm, of "will this work", without doing a bunch of work no one wants to do anyways. |
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 25, 2024
…ux, r=<try> [EXPERIMENT] Crater adding `target_family = "linux"` The lang team proposed, a while back, [deprecating `target_vendor`](rust-lang#100343). With five operating systems all from Apple, using very similar, unifying characteristics for each, it is untenable to ask people to `cfg(any(target_os = "macos", target_os = "ios", target_os = "watchos", target_os = "tvos", target_os = "visionos"))`. It is commonly preferred to use `target_vendor = "apple"` for things that are true for all of these OS variants, and we have to offer a replacement that is equally parsimonious. [The "obvious" choice is to add a `target_family = "apple"`](rust-lang/lang-team#102 (comment)). However, [there is a concern that adding a `target_family = "apple"` won't work without massive ecosystem breakage, because of poorly-designed build scripts](rust-lang/lang-team#102 (comment)). But we currently can't crater a test of that effectively, because most tests only test host compilation, and crater only runs on Linux. We could, however, test adding a hypothetical `target_family = "linux"`! This would be a hypothetical `target_family` that unites both `target_os = "linux"` and `target_os = "android"` targets. This isn't necessarily an "apples to apples" comparison (more like "penguins to robots"?) but it seems worth trying. Note that this is **not** a proposal to actually add such a `target_family`. I don't have any arguments about the merits beyond "it seems like a plausible target family we may one day add, it can be added in a way that disrupts relevant build scripts, and it impacts a 'high-value' target". For maximum coverage I have in fact added the relevant `target_family = "apple"` and `target_family = "linux"` also just in case any test suites somehow run cross-compilation tests.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☀️ Try build successful - checks-actions |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
|
Cursory impressions:
|
3 tasks
This was referenced Jun 28, 2024
|
I've filed PRs to fix the specific affected crates you identified: |
This was referenced Jun 28, 2024
bors
added a commit
to rust-lang/cargo
that referenced
this pull request
Jun 28, 2024
Make it clear that `CARGO_CFG_TARGET_FAMILY` is multi-valued `target_family` / `CARGO_CFG_TARGET_FAMILY` can take multiple values, but this is not particularly clear in the documentation. To resolve this, I've changed the example to document this. [A crater run](rust-lang/rust#124355 (comment)) found a few instances where this value was incorrectly assumed to be a single value.
|
All the affected crates have been fixed, and new versions have been released for the crates that broke the most, I don't know how these things go, but would it make sense to do another crater run? And would the way forwards be to try to lint, do you think? |
|
Oh nice! |
|
yeah if you could draft a lint that triggers only inside a build script (I believe |
Sorry to disappoint, I probably cannot at all (yet) |
|
I don't think it should be considered absolutely blocking, fwiw, @madsmtm, it's just hard to figure out how to migrate people without a something. |
Dylan-DPC
added
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The lang team proposed, a while back, deprecating
target_vendor.With five operating systems all from Apple, using very similar, unifying characteristics for each, it is untenable to ask people to
cfg(any(target_os = "macos", target_os = "ios", target_os = "watchos", target_os = "tvos", target_os = "visionos")). It is commonly preferred to usetarget_vendor = "apple"for things that are true for all of these OS variants, and we have to offer a replacement that is equally parsimonious.The "obvious" choice is to add a
target_family = "apple". However, there is a concern that adding atarget_family = "apple"won't work without massive ecosystem breakage, because of poorly-designed build scripts. But we currently can't crater a test of that effectively, because most tests only test host compilation, and crater only runs on Linux.We could, however, test adding a hypothetical
target_family = "linux"! This would be a hypotheticaltarget_familythat unites bothtarget_os = "linux"andtarget_os = "android"targets. This isn't necessarily an "apples to apples" comparison (more like "penguins to robots"?) but it seems worth trying.Note that this is not a proposal to actually add such a
target_family. I don't have any arguments about the merits beyond "it seems like a plausible target family we may one day add, it can be added in a way that disrupts relevant build scripts, and it impacts a 'high-value' target".For maximum coverage I have in fact added the relevant
target_family = "apple"andtarget_family = "linux"also just in case any test suites somehow run cross-compilation tests.