Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 42 additions & 10 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,9 @@ independently edited pin would let the two disagree.
enforces all four callers. For each one it asserts:

- the job uses the expected shared-action reference — path *and* pinned
revision (see "Workflow pins and Dependabot" below for why the exact
revision is asserted here);
revision, the latter derived from the checked workflows themselves rather
than restated in the test (see "Workflow pins and Dependabot" below for why
the revision is asserted here);
- the `with.rustflags` value matches the table above in full, not merely that
it contains `-Zpolonius=next`, so a dropped `-D warnings` is caught too;
- the job declares no `env.RUSTFLAGS`;
Expand Down Expand Up @@ -530,14 +531,22 @@ an unrecognized `with:` key on a composite action is a warning, not an error —
it simply never exports the flag, so the build fails later as a borrow-check
error rather than as a configuration error.

`tests/polonius_toolchain_contract.rs` therefore asserts each of those
workflows' exact shared-action path *and* pinned revision, held in the
`SETUP_RUST_ACTION` and `RUST_BUILD_RELEASE_ACTION` constants. A Dependabot
bump of these four references is expected to fail the test until someone
updates the constants, and that failure is the point: it forces a human to
confirm the new revision still implements the `rustflags` input contract before
the bump lands. Restrict this exception to callers with a genuine
revision-level dependency; everywhere else, the shape-only policy applies.
`tests/polonius_toolchain_contract.rs` therefore requires the four workflows'
shared-action references to agree, rather than restating the expected pin as a
constant. It extracts every `leynos/shared-actions` reference from the checked
workflows with the shared YAML-parsing helper in
`tests/support/shared_actions.rs`, validates that each is a full
40-character lowercase-hex commit SHA, and derives the pin the workflows must
share from that set. A complete bump — Dependabot's or a manual one — moves
every reference together and passes with no test edit. A partial bump, where
some workflows move and others are left behind, fails on the disagreement
between references: the same failure that previously broke `main` when a bump
missed the hand-maintained constants this contract used to hold. The
revision-level dependency on the `rustflags` input is now protected by that
agreement requirement together with `shared-actions`' own contract tests
upstream, rather than by a constant edited by hand here. Restrict this
exception to callers with a genuine revision-level dependency; everywhere
else, the shape-only policy applies.

If a workflow's behaviour does not depend on a feature from a particular commit
onwards, do not assert its SHA — express any advisory note as a comment or a
Expand Down Expand Up @@ -1816,6 +1825,29 @@ regressing to a doc-comment promise. `tests/locale_stub_strictness_tests.rs`
covers the panic, the trichotomy, and the last-declaration-wins rule with
both example-based and property tests.

#### Locale-stub UI harness and split build directories

`tests/locale_stub_ui_tests.rs` builds `test_support` with `cargo build
--message-format=json` and parses the resulting Cargo JSON messages rather
than assuming its dependencies sit beside the uplifted `test_support` rlib.
For every `compiler-artifact` message it records the parent directory of each
rlib the message names, and passes the whole set to `rustc` as `-L
dependency=` directories when compiling the UI fixtures. This keeps the
harness correct when Cargo's `build.build-dir` setting splits intermediate
artefacts — where dependency rlibs live — from the final, uplifted ones;
deriving the directories from what Cargo actually reports, rather than from a
single assumed location, means the harness does not need to special-case that
split.

`harness_compiles_under_a_split_build_dir` is the regression test for this:
it forces a split layout with its own private `CARGO_TARGET_DIR` and
`CARGO_BUILD_BUILD_DIR` roots, confirms the collected dependency directories
span the split, and then compiles a fixture against them. The roots are
private to the test rather than the ambient target directory because the
`#[once]` `test_support_rlib` fixture builds concurrently in the other test;
sharing a target directory between the two races on the uplifted rlibs and
fails with version-skew errors (`E0460`).

### Manifest `env()` reader

The `env()` Jinja helper reads through an injected [`EnvReader`], a shared
Expand Down
204 changes: 168 additions & 36 deletions tests/locale_stub_ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//! flags — and the fixtures are compiled directly with the workspace `rustc`
//! against that rlib.

use camino::{Utf8Path, Utf8PathBuf};
use rstest::{fixture, rstest};
use std::{
io,
Expand Down Expand Up @@ -76,10 +77,10 @@ fn stub_env_builders_compile_under_the_same_harness(
Ok(())
}

/// The `test_support` rlib and the deps directory holding its dependencies.
/// The `test_support` rlib and the directories holding its dependencies.
struct TestSupportRlib {
rlib: PathBuf,
deps_dir: PathBuf,
rlib: Utf8PathBuf,
deps_dirs: Vec<Utf8PathBuf>,
}

impl TestSupportRlib {
Expand All @@ -89,12 +90,24 @@ impl TestSupportRlib {
/// with the same Polonius flags as the rest of the suite — the property
/// trybuild could not preserve.
fn build() -> io::Result<Self> {
let output = Command::new(cargo())
Self::build_with(&[])
}

/// Build `test_support` with additional environment variables applied.
///
/// The split-layout regression test uses this to force Cargo's
/// `build.build-dir` into a separate directory.
fn build_with(env: &[(&str, &Path)]) -> io::Result<Self> {
let mut command = Command::new(cargo());
command
.arg("build")
.arg("--manifest-path")
.arg(manifest_dir().join("test_support/Cargo.toml"))
.arg("--message-format=json")
.output()?;
.arg("--message-format=json");
for (key, value) in env {
command.env(key, value);
}
let output = command.output()?;
if !output.status.success() {
return Err(io::Error::other(format!(
"building test_support failed:\n{}",
Expand All @@ -108,18 +121,23 @@ impl TestSupportRlib {
.filter_map(test_support_rlib_in_message)
.next_back()
.ok_or_else(|| io::Error::other("cargo reported no test_support rlib artefact"))?;
// Cargo uplifts the top-level package's rlib out of `deps/` into the
// profile directory, so the rlib's own parent is not where the
// dependency rlibs live.
let parent = rlib
.parent()
.ok_or_else(|| io::Error::other("the rlib path should have a parent"))?;
let deps_dir = if parent.file_name() == Some(std::ffi::OsStr::new("deps")) {
parent.to_path_buf()
} else {
parent.join("deps")
};
Ok(Self { rlib, deps_dir })
// Dependency rlibs do not necessarily sit beside the uplifted
// `test_support` rlib: Cargo's `build.build-dir` setting splits
// intermediate artefacts (where dependencies live) from final ones.
// Every compiler-artifact message names its rlib's real location, so
// collect each artefact's parent directory for `-L dependency=`.
let mut deps_dirs: Vec<Utf8PathBuf> = Vec::new();
for parent in stdout.lines().flat_map(rlib_parents_in_message) {
if !deps_dirs.contains(&parent) {
deps_dirs.push(parent);
}
}
if deps_dirs.is_empty() {
return Err(io::Error::other(
"cargo reported no rlib artefacts to derive dependency dirs from",
));
}
Ok(Self { rlib, deps_dirs })
}

/// Type-check `source` against the rlib without linking a binary.
Expand All @@ -134,39 +152,67 @@ impl TestSupportRlib {
.arg("--emit=metadata")
.arg(manifest_dir().join(source))
.arg("--extern")
.arg(format!("test_support={}", self.rlib.display()))
.arg("-L")
.arg(format!("dependency={}", self.deps_dir.display()))
.arg(format!("test_support={}", self.rlib))
.args(
self.deps_dirs
.iter()
.flat_map(|dir| [String::from("-L"), format!("dependency={dir}")]),
)
.arg("-o")
.arg(output_dir.path().join("stub-env-ui.rmeta"))
.output()
}
}

/// Extract the `test_support` rlib path from one Cargo JSON message, if any.
fn test_support_rlib_in_message(line: &str) -> Option<PathBuf> {
/// Extract a compiler-artifact message's target name and rlib paths.
///
/// Returns `None` for lines that are not valid JSON, not compiler-artifact
/// messages, or that lack a target name; the rlib list may be empty for
/// artefacts that emit no rlib.
fn compiler_artifact_rlibs(line: &str) -> Option<(String, Vec<Utf8PathBuf>)> {
let message: serde_json::Value = serde_json::from_str(line).ok()?;
if message.get("reason")? != "compiler-artifact"
|| message.get("target")?.get("name")? != "test_support"
{
if message.get("reason")? != "compiler-artifact" {
return None;
}
message
.get("filenames")?
.as_array()?
.iter()
.filter_map(|filename| filename.as_str())
let name = message.get("target")?.get("name")?.as_str()?.to_owned();
let rlibs = message
.get("filenames")
.and_then(serde_json::Value::as_array)
.into_iter()
.flatten()
.filter_map(serde_json::Value::as_str)
.filter(|filename| {
Path::new(filename)
Utf8Path::new(filename)
.extension()
.is_some_and(|extension| extension.eq_ignore_ascii_case("rlib"))
})
.map(PathBuf::from)
.next_back()
.map(Utf8PathBuf::from)
.collect();
Some((name, rlibs))
}

/// Extract the parent directories of every rlib in one Cargo JSON message.
fn rlib_parents_in_message(line: &str) -> Vec<Utf8PathBuf> {
compiler_artifact_rlibs(line)
.map(|(_name, rlibs)| {
rlibs
.iter()
.filter_map(|rlib| rlib.parent().map(Utf8Path::to_path_buf))
.collect()
})
.unwrap_or_default()
}

/// Extract the `test_support` rlib path from one Cargo JSON message, if any.
fn test_support_rlib_in_message(line: &str) -> Option<Utf8PathBuf> {
let (name, rlibs) = compiler_artifact_rlibs(line)?;
(name == "test_support")
.then(|| rlibs.into_iter().next_back())
.flatten()
}

fn manifest_dir() -> PathBuf {
PathBuf::from(env!("CARGO_MANIFEST_DIR"))
fn manifest_dir() -> Utf8PathBuf {
Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR"))
}

#[expect(
Expand All @@ -188,3 +234,89 @@ fn rustc() -> PathBuf {
fn stderr(output: &Output) -> String {
String::from_utf8_lossy(&output.stderr).into_owned()
}

/// A synthetic Cargo message with two rlibs in different directories,
/// mirroring a split `build.build-dir` layout.
const SPLIT_LAYOUT_MESSAGE: &str = r#"{"reason":"compiler-artifact","target":{"name":"anyhow"},"filenames":["/build/debug/deps/libanyhow-1.rlib","/target/debug/libanyhow-1.rlib"]}"#;

#[rstest]
fn parser_collects_every_rlib_directory_from_a_message() {
let parents = rlib_parents_in_message(SPLIT_LAYOUT_MESSAGE);
assert_eq!(
parents,
vec![
Utf8PathBuf::from("/build/debug/deps"),
Utf8PathBuf::from("/target/debug"),
],
"both rlib directories should be collected in message order"
);
}

#[rstest]
#[case::malformed_json("not json at all")]
#[case::other_reason(r#"{"reason":"build-script-executed","target":{"name":"anyhow"}}"#)]
#[case::missing_target(r#"{"reason":"compiler-artifact","filenames":["/a/lib.rlib"]}"#)]
fn parser_ignores_non_artifact_messages(#[case] line: &str) {
assert!(
rlib_parents_in_message(line).is_empty(),
"non-artifact input should yield no directories: {line:?}"
);
assert!(
test_support_rlib_in_message(line).is_none(),
"non-artifact input should yield no test_support rlib: {line:?}"
);
}

#[rstest]
fn parser_selects_the_test_support_rlib_by_target_name() {
let message = r#"{"reason":"compiler-artifact","target":{"name":"test_support"},"filenames":["/deps/libtest_support-1.rlib","/final/libtest_support.rlib"]}"#;
assert_eq!(
test_support_rlib_in_message(message),
Some(Utf8PathBuf::from("/final/libtest_support.rlib")),
"the last-listed rlib should win, matching Cargo's uplift ordering"
);
assert!(
test_support_rlib_in_message(SPLIT_LAYOUT_MESSAGE).is_none(),
"other targets' artefacts should not be mistaken for test_support"
);
}

/// Forcing a split `build.build-dir` must still yield a working harness:
/// the dependency rlibs land apart from the uplifted `test_support` rlib, so
/// the collected `-L dependency=` set has to span the split for the control
/// fixture to compile. This pins the regression where a single derived
/// directory missed the dependencies entirely.
#[rstest]
fn harness_compiles_under_a_split_build_dir() -> io::Result<()> {
// Both roots are private to this test: sharing the ambient target dir
// with the concurrently building `#[once]` fixture races on the
// uplifted rlibs and fails with version-skew errors (E0460).
let target_dir = tempfile::tempdir()?;
let build_dir = tempfile::tempdir()?;
let harness = TestSupportRlib::build_with(&[
("CARGO_TARGET_DIR", target_dir.path()),
("CARGO_BUILD_BUILD_DIR", build_dir.path()),
])?;

let spans_split_dir = harness
.deps_dirs
.iter()
.any(|dir| dir.starts_with(build_dir.path()));
if !spans_split_dir {
return Err(io::Error::other(format!(
"the dependency directories should include the split build dir {}; found {:?}",
build_dir.path().display(),
harness.deps_dirs,
)));
}

let output = harness.compile("tests/ui/stub_env_strict_compile_pass.rs")?;
if !output.status.success() {
return Err(io::Error::other(format!(
"the control fixture should compile under a split build dir:
{}",
stderr(&output),
)));
}
Ok(())
}
Loading
Loading