Skip to content
Open
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
19 changes: 18 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,14 @@ struct InputAction {
*/
force_compile: bool,

/**
Does the manifest declare path dependencies?

Their sources are invisible to the mtime freshness check below, so cargo
must decide whether a rebuild is needed (issue #122).
*/
has_path_dependencies: bool,

/// Execute the compiled binary?
execute: bool,

Expand Down Expand Up @@ -409,7 +417,13 @@ impl InputAction {
}
};

if matches!(self.build_kind, BuildKind::Normal) && !self.force_compile {
// A script with path dependencies must not be served from the binary
// cache without consulting cargo: this mtime comparison cannot see
// changes in the dependency sources (issue #122).
if matches!(self.build_kind, BuildKind::Normal)
&& !self.force_compile
&& !self.has_path_dependencies
{
match fs::File::open(&built_binary_path) {
Ok(built_binary_file) => {
// When possible, use creation time instead of modified time as cargo may copy
Expand Down Expand Up @@ -538,6 +552,8 @@ fn decide_action_for(
toolchain_version.clone(),
)?;

let has_path_dependencies = manifest::has_path_dependencies(&mani_str)?;

// Forcibly override some flags based on build kind.
let debug = match args.build_kind {
BuildKind::Normal => args.debug,
Expand All @@ -548,6 +564,7 @@ fn decide_action_for(
Ok(InputAction {
cargo_output: args.cargo_output,
force_compile: args.force,
has_path_dependencies,
execute: !args.gen_pkg_only,
pkg_path,
script_path,
Expand Down
48 changes: 48 additions & 0 deletions src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1294,6 +1294,54 @@ fn merge_manifest(
/**
Given a Cargo manifest, attempts to rewrite relative file paths to absolute ones, allowing the manifest to be relocated.
*/
/// Dependency keys whose `path` values point at a local directory.
const DEPENDENCY_PATH_KEYS: &[&[&str]] = &[
&["build-dependencies", "*", "path"],
&["dependencies", "*", "path"],
&["dev-dependencies", "*", "path"],
&["target", "*", "dependencies", "*", "path"],
];

/**
Does the manifest declare any path dependencies?

The sources of a path dependency never invalidate a cached binary: the cache
freshness check compares only the script file and generated manifest mtimes.
Builds with path dependencies must therefore let cargo decide whether a
rebuild is needed (issue #122).
*/
pub fn has_path_dependencies(mani: &str) -> MainResult<bool> {
let mani: toml::value::Table = toml::from_str(mani).map_err(|e| {
MainError::Tag(
"could not parse embedded manifest".into(),
Box::new(MainError::Other(Box::new(e))),
)
})?;

let mut mani = toml::Value::Table(mani);
let mut has_paths = false;
for path in DEPENDENCY_PATH_KEYS {
iterate_toml_mut_path(&mut mani, path, &mut |v| {
if v.is_str() {
has_paths = true;
}
Ok(())
})?;
}
Ok(has_paths)
}

#[test]
fn test_has_path_dependencies() {
assert!(!has_path_dependencies("[package]\nname = \"x\"\n").unwrap());
assert!(!has_path_dependencies("[dependencies]\nserde = \"1\"\n").unwrap());
assert!(has_path_dependencies("[dependencies]\nfoo = { path = \"../foo\" }\n").unwrap());
assert!(has_path_dependencies(
"[target.'cfg(unix)'.dependencies]\nfoo = { path = \"../foo\" }\n"
)
.unwrap());
}

fn fix_manifest_paths(mani: toml::value::Table, base: &Path) -> MainResult<toml::value::Table> {
// Values that need to be rewritten:
let paths: &[&[&str]] = &[
Expand Down
2 changes: 2 additions & 0 deletions tests/scripts/path-dependency-cache.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
value = v1
value = v2
37 changes: 37 additions & 0 deletions tests/scripts/path-dependency-cache.script
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/bin/sh
set -e -u

# https://unix.stackexchange.com/questions/30091/fix-or-alternative-for-mktemp-in-os-x
mytmpdir=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')

cd "$mytmpdir"

mkdir -p dep/src
cat > dep/Cargo.toml <<'EOF'
[package]
name = "rspathdep"
version = "0.1.0"
edition = "2021"
EOF

printf 'pub const VALUE: &str = "v1";\n' > dep/src/lib.rs

cat > script.rs <<'EOF'
#!/usr/bin/env rust-script
//! ```cargo
//! [dependencies]
//! rspathdep = { path = "dep" }
//! ```
fn main() {
println!("value = {}", rspathdep::VALUE);
}
EOF

rust-script script.rs

# Modify the path dependency: the next run has to notice and rebuild it.
# The sleep guards against coarse filesystem mtime granularity.
sleep 1
printf 'pub const VALUE: &str = "v2";\n' > dep/src/lib.rs

rust-script script.rs