Skip to content

Commit

Permalink
Introduce strongly-typed strings, starting with TargetTriple
Browse files Browse the repository at this point in the history
As discussed, for the price of having to think about `TargetTriple` (like `String`)
vs `&TargetTripleRef` (like `&str`), we get:

  * No accidentally passing some other kind of string to a thing expecting a `TargetTriple`
  * Serialization/deserialization is still transparent, no schema changes or anything
  * We can add methods to it (like `is_windows()` in this PR - note that I dream of a `ParsedTargetTriple` in a separate PR)
  * Those methods are the only place where we check properties of the string
    (before this commit, we have `.contains("windows")` and `.contains("pc-windows")` for example)
  * We can "find all references" to the type itself ("where do we care about targets?")
  * We can "find all references" to `TargetTriple::new` ("where do we build targets from strings?")
  * We can "find all references" to `TargetTripleRef::as_str` ("where do we coerce it back
    into a string to pass it to a tool like cargo/wix/etc.)

That kind of change is invaluable for me when working on cross-compilation
support, and I suspect it will be invaluable for any current and future
maintainers of cargo-dist as well (I've used it with great success in other
large codebases).

You can still treat `TargetTriple` as a string, but it'll be uglier (on purpose).

There is however, some ugliness that isn't on purpose. In this changeset I
discovered some annoyances around `.iter()` (which returns an `Iterator<Item = &TargetTriple>`
instead of an `Iterator<Item = &TargetTripleRef>`. I've added `.as_explicit_ref` to work
around those cases.

Similarly, calling `Vec<TargetTriple>::contains()` with a `&TargetTripleRef` doesn't
work (and you cannot convert a `&TargetTripleRef` into a `&TargetTriple`, the same way
you cannot convert a `&str` back into a `&String` - you don't know where it's allocated
from!).

Finally, I ran into <rust-lang/rfcs#1445> while making this
change: there was a big `match` for converting target triples to their display names,
and although that works with `&str` constants, it doesn't work with `&TargetTripleRef`
constants, due to Rust limitations right now. That explains the lazy_static (which
we already depended on transitively, so at least that). I would've used `LazyLock`
but our MSRV is currently 1.79 and LazyLock is since 1.80 :(
  • Loading branch information
fasterthanlime committed Oct 22, 2024
1 parent edd157d commit 3a9c433
Show file tree
Hide file tree
Showing 32 changed files with 874 additions and 432 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ schemars = "0.8.21"
serde_yml = "0.0.10"
spdx = "0.10.6"
base64 = "0.22.1"
lazy_static = "1.4.0"

[workspace.metadata.release]
shared-version = true
Expand Down
1 change: 0 additions & 1 deletion axoproject/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ pub mod generic;
#[cfg(feature = "npm-projects")]
pub mod javascript;
pub mod local_repo;
pub mod platforms;
mod repo;
#[cfg(feature = "cargo-projects")]
pub mod rust;
Expand Down
241 changes: 0 additions & 241 deletions axoproject/src/platforms.rs

This file was deleted.

61 changes: 59 additions & 2 deletions cargo-dist-schema/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,69 @@
//!
//! The root type of the schema is [`DistManifest`][].

pub mod macros;

use std::collections::BTreeMap;

use schemars::JsonSchema;
use semver::Version;
use serde::{Deserialize, Serialize};

declare_strongly_typed_string! {
/// A rust target-triple (e.g. "x86_64-pc-windows-msvc")
pub struct TargetTriple => &TargetTripleRef;
}

impl TargetTripleRef {
/// Returns true if this target triple refers to a musl target
pub fn is_musl(&self) -> bool {
self.0.contains("musl")
}

/// Returns true if this target triple refers to a Linux target
pub fn is_linux(&self) -> bool {
self.0.contains("linux")
}

/// Returns true if this target triple refers to an Apple target
pub fn is_apple(&self) -> bool {
self.0.contains("apple")
}

/// Returns true if this target triple refers to a Darwin target
pub fn is_darwin(&self) -> bool {
self.0.contains("darwin")
}

/// Returns true if this target triple refers to a Windows target
pub fn is_windows(&self) -> bool {
self.0.contains("windows")
}

/// Returns true if this target triple refers to an Intel Apple target
pub fn is_x86_64(&self) -> bool {
self.0.contains("x86_64")
}

/// Returns true if this target triple refers to an ARM target
pub fn is_aarch64(&self) -> bool {
self.0.contains("aarch64")
}

//---------------------------
// common combinations

/// Returns true if this target triple refers to a Linux target with MUSL libc
pub fn is_linux_musl(&self) -> bool {
self.0.contains("linux-musl")
}

/// Returns true if this target triple refers to a Windows target with MSVC CRT
pub fn is_windows_msvc(&self) -> bool {
self.0.contains("windows-msvc")
}
}

/// A local system path on the machine cargo-dist was run.
///
/// This is a String because when deserializing this may be a path format from a different OS!
Expand Down Expand Up @@ -176,7 +233,7 @@ pub struct AssetInfo {
/// * length 0: not a meaningful question, maybe some static file
/// * length 1: typical of binaries
/// * length 2+: some kind of universal binary
pub target_triples: Vec<String>,
pub target_triples: Vec<TargetTriple>,
/// the linkage of this Asset
pub linkage: Option<Linkage>,
}
Expand Down Expand Up @@ -346,7 +403,7 @@ pub struct Artifact {
/// The target triple of the bundle
#[serde(skip_serializing_if = "Vec::is_empty")]
#[serde(default)]
pub target_triples: Vec<String>,
pub target_triples: Vec<TargetTriple>,
/// The location of the artifact on the local system
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(default)]
Expand Down
Loading

0 comments on commit 3a9c433

Please sign in to comment.