fix: rename partially "Triple" to "Tuple" to reflect the new terminology (pt.2)#4834
fix: rename partially "Triple" to "Tuple" to reflect the new terminology (pt.2)#4834motorailgun wants to merge 5 commits intorust-lang:mainfrom
Conversation
a8eafdd to
475cb07
Compare
475cb07 to
b07ea10
Compare
There was a problem hiding this comment.
Pull request overview
This PR continues the terminology migration from “Triple” to “Tuple” (part 2 of the effort), updating types, modules, and user-facing strings without intending behavioral changes.
Changes:
- Renames
TargetTriple/PartialTargetTripletoTargetTuple/PartialTargetTupleacross core dist/config/CLI flows. - Moves the generated “known targets” list from
dist/tripletodist/tupleand updates the generator test accordingly. - Updates CLI prompts, logs, and tests/snapshots to say “tuple” instead of “triple”.
Reviewed changes
Copilot reviewed 15 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/suite/known_tuples.rs | Updates generator test to target the new src/dist/tuple/known.rs and renames parser helper. |
| tests/suite/cli_v2.rs | Updates tests to use TargetTuple in manifest manipulation. |
| tests/suite/cli_rustup.rs | Renames tests and expected stderr text from “triple” to “tuple”. |
| tests/suite/cli_inst_interactive.rs | Updates interactive installer snapshots/prompts from “host triple” to “host tuple”. |
| src/toolchain/names.rs | Switches resolver APIs and test imports to TargetTuple / tuple::known. |
| src/toolchain.rs | Changes installed-target parsing to return TargetTuple. |
| src/test/dist.rs | Updates mock dist server to construct/insert TargetTuple keys/values. |
| src/test.rs | Updates host detection helper to use TargetTuple::from_host. |
| src/errors.rs | Re-types error fields from TargetTriple to TargetTuple. |
| src/dist/tuple/known.rs | Adds the generated known arch/os/env lists under the new tuple module. |
| src/dist/tuple.rs | Renames PartialTargetTriple to PartialTargetTuple and updates tests/docs accordingly. |
| src/dist/mod.rs | Replaces triple module with tuple, renames TargetTriple to TargetTuple, and updates associated logic/tests. |
| src/dist/manifestation/tests.rs | Updates manifestation tests to construct components with TargetTuple. |
| src/dist/manifestation.rs | Renames manifestation target field to target_tuple and updates comparisons and lookup logic. |
| src/dist/manifest.rs | Switches manifest target maps/components and (de)serialization helpers to TargetTuple. |
| src/config.rs | Updates config flows to use TargetTuple (including default host resolution and checks). |
| src/cli/setup_mode.rs | Renames install option field to default_host_tuple. |
| src/cli/self_update/windows.rs | Updates Windows self-update MSVC checks to use tuple terminology/types. |
| src/cli/self_update.rs | Renames install options field/prompt/logging to “tuple” and updates host/build tuple usage. |
| src/cli/rustup_mode.rs | Updates CLI logic to use TargetTuple for host/target checks and target add/remove. |
| src/cli/common.rs | Updates shared CLI helpers to accept/use TargetTuple. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if name.has_tuple() { | ||
| let host_arch = TargetTuple::from_host_or_build(cfg.process); | ||
| let target_triple = name.clone().resolve(&host_arch)?.target; | ||
| common::check_non_host_toolchain( |
There was a problem hiding this comment.
This local variable is still named target_triple, but it now holds a TargetTuple. Renaming it to target_tuple would keep terminology consistent and avoid confusion while reading the host/target compatibility check.
| let host_tuple = if let Some(trip) = opts.default_host_tuple.as_ref() { | ||
| trip.to_owned() |
There was a problem hiding this comment.
The option is now default_host_tuple, but the temporary binding is still named trip. Consider renaming this binding to tuple (or host_tuple) to match the new terminology and reduce confusion in the MSVC detection logic.
| let host_tuple = if let Some(trip) = opts.default_host_tuple.as_ref() { | |
| trip.to_owned() | |
| let host_tuple = if let Some(tuple) = opts.default_host_tuple.as_ref() { | |
| tuple.to_owned() |
|
|
||
| pub(crate) mod triple; | ||
| pub(crate) use triple::*; | ||
| pub(crate) mod tuple; |
There was a problem hiding this comment.
We need to be a bit more careful here because previously triple as an abbreviation for target_triple is more or less unambiguous. As discussed in #4668, we would like to rename this module to target_tuple instead of just tuple to avoid potential confusion with the builtin tuple type.
PS: In e.g. method names however, this is less of a problem, so you can do nothing for now. If anyone still finds it too problematic, we can use _target to replace _triple later on.
There was a problem hiding this comment.
Thanks! So do you mean we want to keep method names with _triple suffix (or whatsoever) as is, while renaming triple module to target_tuple? (Sorry I'm not really good at English)
There was a problem hiding this comment.
Thanks! So do you mean we want to keep method names with
_triplesuffix (or whatsoever) as is, while renamingtriplemodule totarget_tuple?
@motorailgun My apologies for not being super clear previously!
For method names you can perform the switch from _triple to _tuple as usual, depending on your progress.
If this doesn't feel quite right for you in certain places please let me know.
| } | ||
|
|
||
| /// Parses the given triple into 3 parts (target architecture, OS and environment). | ||
| /// Parses the given tuple into 3 parts (target architecture, OS and environment). |
There was a problem hiding this comment.
Similarly to b07ea10#r3144041456, this module should be renamed to known_target_tuples. The function names can stay with _tuple.
Partial effort to #4826, and continuous to #4827.
This PR still does not do functionality changes.
Opening as draft PR to rely on CI for Windows-only cfgs.<- fixed!