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

Target tier policy #2803

Merged
merged 166 commits into from
Apr 29, 2021
Merged

Target tier policy #2803

merged 166 commits into from
Apr 29, 2021

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Nov 4, 2019

Rust developers regularly implement new targets in the Rust compiler,
and reviewers of pull requests for such new targets would like a clear,
consistent policy to cite for accepting or rejecting such targets.
Currently, individual reviewers do not know what overall policy to
apply, and whether to apply solely their own judgment or defer to a Rust
governance team.

Rust developers regularly ask how they can raise an existing target to
tier 2 (and in particular how they can make it available via rustup),
and occasionally ask what it would take to add a new tier 1 target. The
Rust project has no clear official policy for target tiers. People not
only don't know, they don't know who to ask or where to start.

This proposal documents an official policy for adding new (tier 3)
targets, and for raising targets to tier 2 (with rustup builds) or to
tier 1.

Based on discussions with the compiler team and representatives from
other teams.

Note: when merging this RFC, please don't squash-merge.
This policy has gone through substantial revisions, and the history of those revisions would be useful.

Rendered

Rust developers regularly implement new targets in the Rust compiler,
and reviewers of pull requests for such new targets would like a clear,
consistent policy to cite for accepting or rejecting such targets.
Currently, individual reviewers do not know what overall policy to
apply, and whether to apply solely their own judgment or defer to a Rust
governance team.

Rust developers regularly ask how they can raise an existing target to
tier 2 (and in particular how they can make it available via rustup),
and occasionally ask what it would take to add a new tier 1 target. The
Rust project has no clear official policy for target tiers. People not
only don't know, they don't know who to ask or where to start.

This proposal documents an official policy for adding new (tier 3)
targets, and for raising targets to tier 2 (with rustup builds) or to
tier 1.

Based on discussions with the compiler team and representatives from
other teams.
@joshtriplett joshtriplett added the T-compiler Relevant to the compiler team, which will review and decide on the RFC. label Nov 4, 2019
@joshtriplett
Copy link
Member Author

cc @rust-lang/compiler @rust-lang/infra @rust-lang/release

text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Show resolved Hide resolved
@pietroalbini pietroalbini added T-infra Relevant to the infrastructure team, which will review and decide on the RFC. T-release Relevant to the release team, which will review and decide on the RFC. labels Nov 4, 2019
@pietroalbini
Copy link
Member

Thanks for writing this Josh! 🎉

Left a few comments and added T-infra and T-release.

@pietroalbini
Copy link
Member

Something we might want to clarify is the distinction between being tier 1/2/3 as a cross-compilation target and being tier 1/2/3 as a host platform, possibly including even tools in the last definition. I think formalizing that (and adding the categorization to the forge) will reduce the confusion about what's actually supported.

Copy link
Contributor

@jethrogb jethrogb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I support bringing clarity to the tier system. Has there been any effort of notifying existing tier 2 & 3 target owners of this ongoing RFC?

text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
@ehuss
Copy link
Contributor

ehuss commented Nov 6, 2019

This may be off-topic: but I think it would be nice to be clearer about the definition of a tier and a platform.

For example, tier 1 says "pass tests", but can I just add #[cfg(not(my_platform))] in front of a test to make it pass? Can I do that to a few tests? A lot of tests? There is also some amount of graduated support. For example, some features may be disabled or degraded on older versions of a platform. Usually that's because the platform doesn't fundamentally support it, but what if it is because I'm not in the mood to load a Windows 7 VM to fix it, so I just disable something because it is "too hard" to support?

Also, which version of a platform is supported. The forge page currently lists macos 10.7+ and Windows 7+ as "tier 1" status, but I would be surprised if macos 10.7 still works. And very few tier 2 platforms mention which version is built. Should targets list their version? Should that version be kept up-to-date with reality? For tier 1, that would be macos 10.13, Windows Server 2016, linux is still kernel 2.6.18. And only those versions are tested, not newer ones. Should it be documented which version is tested, and which versions are intended to be supported?

Is there any expectation about when newer versions of a platform will be supported? If 1.39 was released without supporting macos 10.15, is that OK? What about 1.40? How soon should newer versions be supported?

Platforms also have a suite of tooling that may be required, such as linkers. Are there any guarantees about which versions of those are required? Can I decide to stop supporting an old version of xcode? Can I say "you must use clang, gcc is not supported"?

Feel free to ignore if this is too out of scope. Most of these are judgement calls, but some guidance might help making those decisions and communicating what is actually supported and tested.

text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Show resolved Hide resolved
Copy link
Member

@shepmaster shepmaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this document would be better represented as lists of requirements for each tier and a separate list of transitions between the tiers. Each tier's requirements should be a strict superset of the tier before it. Only transitions between immediately adjacent tiers should be defined, and both directions of change should be explicitly called out. Anything that isn't objective could be separated into a "guidance" sub-section for the requirements / transition process.

You may wish to specifically bequeath the ability / responsibility of determining the subjective aspects to the team(s) responsible. Then the teams can iterate on whatever process they decide on, gating on a simple "yes" or "no" for any tier transition.

There's a number of value-judgement conditions present in this RFC. I have no strong objection to such, but I'm not sure that it actually improves things. As I see the current situation, it's basically "hey do you think this should be tier X? I dunno, what do you think?". After the change, the answer feels more like "the RFC states that we should think about if it deserves to go up/down a tier".

text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
text/0000-target-tier-policy.md Show resolved Hide resolved
text/0000-target-tier-policy.md Outdated Show resolved Hide resolved
@XAMPPRocky
Copy link
Member

XAMPPRocky commented Nov 18, 2019

@shepmaster I made a PR changing the style from a flat list of bullet points to using IETF verbs, "MUST", "MUST NOT", etc. Is this along the lines you were thinking? I only did tier 3 as I wanted to get feedback before writing the rest.

https://github.com/XAMPPRocky/rfcs/blob/target-tier-policy/text/0000-target-tier-policy.md

@shepmaster
Copy link
Member

shepmaster commented Nov 18, 2019

to using IETF verbs

I'm personally fine with either style.

I won't claim to be an IETF-style expert. There's something nice about the certainty of those established words, but I'm not sure how well the current content fits within that framework.

I don't object to Rust's looser styling either, so long as we are clear about which pieces we expect to be (mostly) objective and which to be (mostly) subjective.

I think the key piece I'd like to see is the clearly differentiated transitions. Using some silly made-up requirements for example:

# Tier 1
## Requirements
### Objective

- Code comments for this target must be written in Pig Latin
- Compiling this code speeds up the compiler build process by 10 minutes
- All Tier 2 objective requirements

### Subjective

- The target is used by Really Big and Awesome companies that we like.
- We like the primary terminal emulator on this platform
- All Tier 2 subjective requirements

# Tier 2
## Requirements
### Objective

- Code must use a maximum width of 5 characters
- Compiling this code introduces at least 10 new compiler warnings
- All Tier 1 objective requirements

### Subjective

- Code must make us feel warm and fuzzy
- All Tier 1 subjective requirements

# Transitioning
## Tier 1 to Tier 2

If the target fails to adhere to the Tier 1 requirements for 1 day, then the X
team(s) may downgrade this target to tier 2. 

## Tier 2 to Tier 3

If the target fails to adhere to the Tier 2 requirements for 1 year, then the X
team(s) may downgrade this target to tier 3. 

## Tier 2 to Tier 1

If the target adheres to the Tier 1 requirements for 1 month and @shepmaster receives
a payout of $1000USD,  then the X team(s) may upgrade this target to tier 1. 

@shepmaster
Copy link
Member

Another aspect that occurs to me is that it's likely worth expressing why we have a given requirement (or even a lack of requirement). While it might seem self-evident why having CI is important, including the rationale for that will give us more context when we apply subjective criteria or attempt to update the rules in the future.

@Lokathor
Copy link
Contributor

I'd like to see more focus on target tier vs host tier. Many embedded devices will never, ever, be hosts to the rust compiler, but they should be able to be above tier 3 a targets for a build.

@BatmanAoD
Copy link
Member

The draft states that Tier 1 targets may be "demoted or removed", but in #2837, there's some discussion (starting here) about how surprising it is for a Tier 1 target to jump all the way to Tier 3 in a single release. I think it's reasonable to permit this sort of "jump", but the document should probably explicitly state that it's allowed.

I do think it would be quite surprising to entirely remove a Tier 1 target. Perhaps a better policy would be that Tier 1 targets must be demoted to Tier 3 for at least one release cycle before being removed.

@XAMPPRocky
Copy link
Member

XAMPPRocky commented Dec 11, 2019

@BatmanAoD That PR doesn't propose removal of the code. It only proposes stopping to build the artifacts and run tests on that platform. It will be one release before dropping to tier 3 support.

@Lokathor
Copy link
Contributor

Might we have a maximum of 1 level of demotion per release? So tier-1 would drop to tier-2 for a release before becoming tier-3 the next release. Helps ease the process a little bit for end users, though I'm not sure how much burden that puts on the infra team.

@BatmanAoD
Copy link
Member

@XAMPPRocky I don't understand the objection; stopping building the artifacts is demoting to tier 3, isn't it?

In any case, my point is not about that specific demotion; it's just that this document should be more explicit about how demotions work in general.

@pietroalbini
Copy link
Member

Might we have a maximum of 1 level of demotion per release? So tier-1 would drop to tier-2 for a release before becoming tier-3 the next release. Helps ease the process a little bit for end users, though I'm not sure how much burden that puts on the infra team.

The burden on the infra team is negligible (demoting a target basically means removing CI builders, so for Tier 1 => Tier 2 we remove the test builder and for Tier 2 => Tier 3 we remove the dist builder), but I'd argue demoting step by step is worse for end users.

When a platform is Tier 1 it's guaranteed to work (as we run test), when it's Tier 2 it's available through rustup but not tested anymore, and when it's Tier 3 it's not available through rustup at all.

If we go directly from Tier 1 to Tier 3 users will go from having a build that's guaranteed to work to no build at all. Instead, if we demote step by step they'll rustup update to a possibly broken build, and they'll be stuck with that as the last available compiler.

Tier 2 and tier 1 targets place work on Rust project developers,
specifically; describe it as such rather than "the Rust community".

Also add the observation that the broader community may feel more
inclination to support higher-tier targets, but clarify that people are
not obligated to do so.
@Mark-Simulacrum
Copy link
Member

@rfcbot resolve clarifying-questions-and-edits

I've had a chance to review the latest draft and some of the feedback you gave in response to my broader comments, @joshtriplett -- I think this looks good to me. We may want to iterate further, but I think this gets us meaningful progress, and as it is definitely true we're continuing to see interest, I think it makes sense to aim to merge this. We can iron out problems as they arise with implementation in practice, and perhaps document patterns common to its application to existing and future targets.

Thanks so much for the hard work here!

@pnkfelix
Copy link
Member

@rfcbot reviewed

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 14, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Apr 14, 2021
Also qualify that as "existing target"; an existing target could use
issues to track progress towards tier 2 or tier 1, but an aspiring tier
3 target using issues on the Rust repo may or may not be appropriate.
Just reference the tidy tool itself.
@alarixnia
Copy link

alarixnia commented Apr 23, 2021

Hi, I was linked to this RFC from rust-lang/rust#84475 by @jyn514. I've read through the recommendations on moving a target from Tier 3 to Tier 2. My primary interest is in seeing builds published for a different (very well supported by LLVM) CPU architecture for an operating system that already has a Tier 2 target, which from my perspective isn't a major compiler change, but a testing and infrastructure change.

From looking at the major change proposal template, it seems to be intended for major code/language changes. As a person external to the rust compiler project, it's unclear how I should format my proposal. My impression of how this worked in the past was if you provide the interest and hardware/infrastructure, you get moved up a tier, which makes more sense to me as the developer of a target who's not on the rust compiler team.

@joshtriplett
Copy link
Member Author

@Niacat I would suggest taking the tier 3 and tier 2 sections of this policy, quoting them in full, and responding to each bullet point stating how the target meets that requirement. That should provide all the information needed to evaluate the MCP.

@jyn514
Copy link
Member

jyn514 commented Apr 23, 2021

@joshtriplett IIUC aarch64-unknown-netbsd is already tier 3, so I think @Niacat should only have to explain the tier 2 requirements, right?

@joshtriplett
Copy link
Member Author

@jyn514 Given that all of the tiers incorporate the requirements of each lower tier by reference, I think it's generally a good idea to confirm all the requirements on each proposed tier promotion.

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this RFC. label Apr 24, 2021
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 24, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Apr 24, 2021
@joshtriplett
Copy link
Member Author

I've submitted rust-lang/rust#84583 to add this policy alongside the platform support page, in the rustc documentation.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you to everyone who helped write the RFC or give feedback!

@jyn514 jyn514 merged commit 575b9d6 into rust-lang:master Apr 29, 2021
@joshtriplett joshtriplett deleted the target-tier-policy branch April 29, 2021 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-compiler Relevant to the compiler team, which will review and decide on the RFC. T-infra Relevant to the infrastructure team, which will review and decide on the RFC. T-release Relevant to the release team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.