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

Deprecate target_vendor #102

Closed
joshtriplett opened this issue Jul 2, 2021 · 10 comments
Closed

Deprecate target_vendor #102

joshtriplett opened this issue Jul 2, 2021 · 10 comments
Labels
final-comment-period The FCP has started, most (if not all) team members are in agreement major-change Major change proposal T-lang

Comments

@joshtriplett
Copy link
Member

joshtriplett commented Jul 2, 2021

Proposal

Summary and problem statement

Target names (sometimes called target "triples" despite having two, four, or five components) have a field for the "vendor" of a system. This field is vestigial, and ideally Rust code should never need to match on it to distinguish targets. All modern targets are effectively distinguished by architecture, OS, environment, and ABI, with the exception of a few target names invented solely for Rust. We should not use the target_vendor field to distinguish future targets; we should distinguish targets using more semantically meaningful fields.

Motivation, use-cases, and solution sketches

I propose to prevent any new targets from being distinguished solely by "vendor", and I propose to deprecate the target_vendor field, with a transition plan for the Rust-originated targets that currently exist. (I do not propose to change the name of any existing target.)

The existing Rust-originated targets where target_vendor matters:

Windows UWP targets use target_vendor as the only distinction from the corresponding non-UWP Windows targets:

aarch64-pc-windows-msvc
aarch64-uwp-windows-msvc
i686-pc-windows-gnu
i686-uwp-windows-gnu
i686-pc-windows-msvc
i686-uwp-windows-msvc
thumbv7a-pc-windows-msvc
thumbv7a-uwp-windows-msvc
x86_64-pc-windows-gnu
x86_64-uwp-windows-gnu
x86_64-pc-windows-msvc
x86_64-uwp-windows-msvc

For these targets, I propose to set target_abi = "uwp", and encourage code to use that instead. I don't propose to change the name of these targets.

The SGX target, x86_64-fortanix-unknown-sgx, uses target_vendor to distinguish between the fortanix SGX API/ABI and a hypothetical future SGX API/ABI from another vendor. I propose to set target_abi = "fortanix" for this target, as in practice that's the purpose this field serves.

We have a deprecated Solaris compatibility target, x86_64-sun-solaris, similar to x86_64-pc-solaris. I propose to leave this target untouched, as it's already deprecated; that transition should continue as planned.

Finally, Apple targets set target_vendor = "apple", and some code uses this to match both iOS and macOS. We could optionally add target_family = "apple" for these targets, in addition to the existing target_family = "unix". Rust supports having multiple "family" values set, though this has the possibility of breaking fragile build scripts that match the entire value of CARGO_CFG_TARGET_FAMILY rather than treating it as a list.

Once target_abi is stable and the UWP and Fortanix targets set target_abi, and the Apple targets set target_family, I propose to carefully deprecate the target_vendor field, by adding a warn-by-default future-compatibility lint. target_vendor should continue to work for several years. We may eventually choose to remove it in a future edition.

We may optionally choose to start warning about uses of target_vendor that don't look for uwp or fortanix early, and then just suppress warnings about checks for those two values until target_abi is stable.

For future new targets, I propose to omit the "vendor" component entirely unless needed for compatibility with naming already used by other existing widespread toolchains. Whenever we create a new target name, we should omit the "vendor" element entirely, as we did for wasm32-wasi.

Links and related work

Target "triples" originated with autotools. The maintainer of autoconf, @zackw, wrote about the vendor field at https://internals.rust-lang.org/t/expand-targets-acceptable-to-rustc/14885/26 👍

  • The VENDOR component of a GNU "canonical system name" is vestigial. It exists because there was a brief period in the late 1980s/early 1990s where there were a bunch of Unix workstation vendors all putting out kit based on the Motorola 68xxx CPU, with operating systems identifying themselves as "Unix System V", but providing different C-level programming environments. So m68k-foonly-sysv and m68k-barley-sysv could have been meaningfully different. Nobody does this anymore, because the marketing value of giving your company's operating system its own name became obvious almost immediately.
    If you look at how GCC's configure.ac, for instance, processes canonical system names, you will see that most of the time it looks only at $(host|target)_cpu or at $(host|target)_os, and the rest of the time it uses shell glob patterns that ignore the vendor field.
    I would recommend documenting that #[cfg(target_vendor = ...)] is supported for completeness and (as far as the Rust Project knows) is not useful for distinguishing between any two currently supported systems.

Initial people involved

I don't believe this will require a formal "project", just a few PRs, which I'll either write or coordinate. However, I'd like to seek consensus on this change before beginning work on it.

What happens now?

This issue is part of the experimental MCP process described in RFC 2936. Once this issue is filed, a Zulip topic will be opened for discussion, and the lang-team will review open MCPs in its weekly triage meetings. You should receive feedback within a week or two.

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@joshtriplett joshtriplett added major-change Major change proposal T-lang labels Jul 2, 2021
@rustbot
Copy link
Collaborator

rustbot commented Jul 2, 2021

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@rustbot rustbot added the to-announce Not yet announced MCP proposals label Jul 2, 2021
@nagisa
Copy link
Member

nagisa commented Jul 2, 2021

@rustbot second

@rustbot rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Jul 2, 2021
@scottmcm
Copy link
Member

scottmcm commented Jul 2, 2021

This is persuasive to me.

On question: What, technically, do you expect "deprecated" to mean here?

My guess: target_vendor continues to work exactly as it does today, but in the documentation we discourage its use, and going forward we don't set it when adding new targets.

(Is there any desire to also add warnings about it?)

@nagisa
Copy link
Member

nagisa commented Jul 2, 2021

Oh wait, this is a t-lang not a t-compiler MCP. I guess somebody else will have to second and mine doesn't count (though I do think this should happen regardless)

@joshtriplett
Copy link
Member Author

@nagisa Your support still carries weight, though procedurally this does need a second from a lang team member.

@joshtriplett
Copy link
Member Author

@scottmcm (Responded on Zulip.)

@zackw
Copy link

zackw commented Jul 3, 2021

[moved to zulip - sorry, I scrolled right past the "don't post technical comments here" notice]

@Mark-Simulacrum Mark-Simulacrum removed to-announce Not yet announced MCP proposals final-comment-period The FCP has started, most (if not all) team members are in agreement labels Jul 6, 2021
@scottmcm
Copy link
Member

scottmcm commented Mar 2, 2022

@rustbot second

Sure, I can liason this.

@rustbot rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Mar 2, 2022
@thomcc
Copy link
Member

thomcc commented Apr 13, 2022

(This isn't the place for the discussion, but commenting in Zulip tends to get forgotten after things merge, and I think this is worth recording -- responses to this should be in Zulip, probably)

While I'm l actually in favor of the proposal, there's one part I think is problematic, which I would like to record somewhere.

Finally, Apple targets set target_vendor = "apple", and some code uses this to match both iOS and macOS. We could optionally add target_family = "apple" for these targets, in addition to the existing target_family = "unix". Rust supports having multiple "family" values set, though this has the possibility of breaking fragile build scripts that match the entire value of CARGO_CFG_TARGET_FAMILY rather than treating it as a list.

This is could be a bigger issue than anticipated -- As I note here: https://internals.rust-lang.org/t/futher-extensions-to-cfg-target-family-and-concerns-about-breakage/16313, it's pretty likely that CARGO_CFG_TARGET_FAMILY was not considered when making target_family multi-valued, and targetting emscripten is uncommon enough for rust, that we "got away" with it. I think we should hesitate before taking it as a given that we can add more values to target_family. I am also pretty willing to forgive these build scripts because pulling these values from the environment is what they should do, since otherwise they won't support cross-compilation. (It also seems silly for us to argue that they all should treat every value as multi-valued).

Anyway, this isn't an objection to deprecating target vendor, it's an objection to the proposed solution for apple targets. I think we should avoid adding further multi-valued to target-family until we have a better idea of how much this will break.

@pnkfelix
Copy link
Member

pnkfelix commented Aug 9, 2022

I created a tracking issue as described on the initiatives process.

@joshtriplett , if you think a distinct zulip stream is warranted here, let me know and I can create it. I'm honestly a little hesitant to create a distinct zulip stream for each one of these MCPs.

@pnkfelix pnkfelix closed this as completed Aug 9, 2022
Thomasdezeeuw added a commit to Thomasdezeeuw/socket2 that referenced this issue Feb 25, 2023
Thomasdezeeuw added a commit to Thomasdezeeuw/socket2 that referenced this issue Feb 25, 2023
Thomasdezeeuw added a commit to rust-lang/socket2 that referenced this issue Feb 25, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue 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.
bors added a commit to rust-lang-ci/rust that referenced this issue 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period The FCP has started, most (if not all) team members are in agreement major-change Major change proposal T-lang
Projects
None yet
Development

No branches or pull requests

8 participants