From f662147d1d0891dbe9f40d28932a2754b2c715ec Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 12 May 2021 05:42:37 +0900 Subject: [PATCH 1/2] Fix build error with targets that do not support atomic CAS operations Some no-std targets (e.g., ARMv6-M) do not support atomic CAS operations and cannot use `Arc`, etc. This patch detects those targets using the `TARGET` environment variables provided by cargo for the build script, and a list of targets that do not support atomic CAS operations. --- .github/workflows/ci.yml | 17 +++++++++++++--- ci/no_atomic_cas.sh | 28 ++++++++++++++++++++++++++ valuable/build.rs | 41 ++++++++++++++++++++++++++++++++++++++ valuable/no_atomic_cas.rs | 11 ++++++++++ valuable/src/enumerable.rs | 1 + valuable/src/listable.rs | 1 + valuable/src/mappable.rs | 1 + valuable/src/structable.rs | 1 + valuable/src/valuable.rs | 1 + 9 files changed, 99 insertions(+), 3 deletions(-) create mode 100755 ci/no_atomic_cas.sh create mode 100644 valuable/build.rs create mode 100644 valuable/no_atomic_cas.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a640f38..2c72ea8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,9 +26,7 @@ jobs: strategy: matrix: target: - # TODO: Currently, valuable cannot build with targets that do not have atomic CAS. - # We should port https://github.com/rust-lang/futures-rs/pull/2400. - # - thumbv6m-none-eabi + - thumbv6m-none-eabi - thumbv7m-none-eabi runs-on: ubuntu-latest steps: @@ -59,6 +57,19 @@ jobs: # run: cargo install cargo-hack # - run: cargo hack build --workspace --feature-powerset --no-dev-deps + # When this job failed, run ci/no_atomic_cas.sh and commit result changes. + # TODO(taiki-e): Ideally, this should be automated using a bot that creates + # PR when failed, but there is no bandwidth to implement it + # right now... + codegen: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Install Rust + run: rustup update nightly && rustup default nightly + - run: ci/no_atomic_cas.sh + - run: git diff --exit-code + fmt: runs-on: ubuntu-latest steps: diff --git a/ci/no_atomic_cas.sh b/ci/no_atomic_cas.sh new file mode 100755 index 0000000..761e1c2 --- /dev/null +++ b/ci/no_atomic_cas.sh @@ -0,0 +1,28 @@ +#!/bin/bash + +# Update the list of targets that do not support atomic CAS operations. +# +# Usage: +# ./ci/no_atomic_cas.sh + +set -euo pipefail +IFS=$'\n\t' + +cd "$(cd "$(dirname "$0")" && pwd)"/.. + +file="valuable/no_atomic_cas.rs" + +{ + echo "// This file is @generated by $(basename "$0")." + echo "// It is not intended for manual editing." + echo "" + echo "const NO_ATOMIC_CAS_TARGETS: &[&str] = &[" +} >"$file" + +for target in $(rustc --print target-list); do + res=$(rustc --print target-spec-json -Z unstable-options --target "$target" \ + | jq -r "select(.\"atomic-cas\" == false)") + [[ -z "$res" ]] || echo " \"$target\"," >>"$file" +done + +echo "];" >>"$file" diff --git a/valuable/build.rs b/valuable/build.rs new file mode 100644 index 0000000..8e1357e --- /dev/null +++ b/valuable/build.rs @@ -0,0 +1,41 @@ +#![warn(rust_2018_idioms, single_use_lifetimes)] + +use std::env; + +include!("no_atomic_cas.rs"); + +// The rustc-cfg listed below are considered public API, but it is *unstable* +// and outside of the normal semver guarantees: +// +// - `valuable_no_atomic_cas` +// Assume the target does *not* support atomic CAS operations. +// This is usually detected automatically by the build script, but you may +// need to enable it manually when building for custom targets or using +// non-cargo build systems that don't run the build script. +// +// With the exceptions mentioned above, the rustc-cfg strings below are +// *not* public API. Please let us know by opening a GitHub issue if your build +// environment requires some way to enable these cfgs other than by executing +// our build script. +fn main() { + let target = match env::var("TARGET") { + Ok(target) => target, + Err(e) => { + println!( + "cargo:warning=valuable: unable to get TARGET environment variable: {}", + e + ); + return; + } + }; + + // Note that this is `no_*`, not `has_*`. This allows treating + // `cfg(target_has_atomic = "ptr")` as true when the build script doesn't + // run. This is needed for compatibility with non-cargo build systems that + // don't run the build script. + if NO_ATOMIC_CAS_TARGETS.contains(&&*target) { + println!("cargo:rustc-cfg=valuable_no_atomic_cas"); + } + + println!("cargo:rerun-if-changed=no_atomic_cas.rs"); +} diff --git a/valuable/no_atomic_cas.rs b/valuable/no_atomic_cas.rs new file mode 100644 index 0000000..0819af1 --- /dev/null +++ b/valuable/no_atomic_cas.rs @@ -0,0 +1,11 @@ +// This file is @generated by no_atomic_cas.sh. +// It is not intended for manual editing. + +const NO_ATOMIC_CAS_TARGETS: &[&str] = &[ + "avr-unknown-gnu-atmega328", + "msp430-none-elf", + "riscv32i-unknown-none-elf", + "riscv32imc-unknown-none-elf", + "thumbv4t-none-eabi", + "thumbv6m-none-eabi", +]; diff --git a/valuable/src/enumerable.rs b/valuable/src/enumerable.rs index 5980bfb..736e89d 100644 --- a/valuable/src/enumerable.rs +++ b/valuable/src/enumerable.rs @@ -213,6 +213,7 @@ impl Enumerable for alloc::rc::Rc { } } +#[cfg(not(valuable_no_atomic_cas))] #[cfg(feature = "alloc")] impl Enumerable for alloc::sync::Arc { fn definition(&self) -> EnumDef<'_> { diff --git a/valuable/src/listable.rs b/valuable/src/listable.rs index 011340e..b21e43a 100644 --- a/valuable/src/listable.rs +++ b/valuable/src/listable.rs @@ -26,6 +26,7 @@ impl Listable for alloc::rc::Rc { } } +#[cfg(not(valuable_no_atomic_cas))] #[cfg(feature = "alloc")] impl Listable for alloc::sync::Arc { fn size_hint(&self) -> (usize, Option) { diff --git a/valuable/src/mappable.rs b/valuable/src/mappable.rs index e7cfe6f..9754bc7 100644 --- a/valuable/src/mappable.rs +++ b/valuable/src/mappable.rs @@ -26,6 +26,7 @@ impl Mappable for alloc::rc::Rc { } } +#[cfg(not(valuable_no_atomic_cas))] #[cfg(feature = "alloc")] impl Mappable for alloc::sync::Arc { fn size_hint(&self) -> (usize, Option) { diff --git a/valuable/src/structable.rs b/valuable/src/structable.rs index c6ede7f..c4bd204 100644 --- a/valuable/src/structable.rs +++ b/valuable/src/structable.rs @@ -108,6 +108,7 @@ impl Structable for alloc::rc::Rc { } } +#[cfg(not(valuable_no_atomic_cas))] #[cfg(feature = "alloc")] impl Structable for alloc::sync::Arc { fn definition(&self) -> StructDef<'_> { diff --git a/valuable/src/valuable.rs b/valuable/src/valuable.rs index fb596e1..8fc711d 100644 --- a/valuable/src/valuable.rs +++ b/valuable/src/valuable.rs @@ -69,6 +69,7 @@ impl Valuable for alloc::rc::Rc { } } +#[cfg(not(valuable_no_atomic_cas))] #[cfg(feature = "alloc")] impl Valuable for alloc::sync::Arc { fn as_value(&self) -> Value<'_> { From f7f1b959d9290195c92cf9c92891e587bd16a69f Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 12 May 2021 07:11:00 +0900 Subject: [PATCH 2/2] consider cfg as private --- valuable/build.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/valuable/build.rs b/valuable/build.rs index 8e1357e..029d644 100644 --- a/valuable/build.rs +++ b/valuable/build.rs @@ -4,19 +4,9 @@ use std::env; include!("no_atomic_cas.rs"); -// The rustc-cfg listed below are considered public API, but it is *unstable* -// and outside of the normal semver guarantees: -// -// - `valuable_no_atomic_cas` -// Assume the target does *not* support atomic CAS operations. -// This is usually detected automatically by the build script, but you may -// need to enable it manually when building for custom targets or using -// non-cargo build systems that don't run the build script. -// -// With the exceptions mentioned above, the rustc-cfg strings below are -// *not* public API. Please let us know by opening a GitHub issue if your build -// environment requires some way to enable these cfgs other than by executing -// our build script. +// The rustc-cfg strings below are *not* public API. Please let us know by +// opening a GitHub issue if your build environment requires some way to enable +// these cfgs other than by executing our build script. fn main() { let target = match env::var("TARGET") { Ok(target) => target,