Skip to content

Commit

Permalink
Auto merge of #12995 - linyihai:issue_11383, r=weihanglo
Browse files Browse the repository at this point in the history
Exited with hard error when custom build file no existence or not in package

## What does this PR try to resolve?
Fixed #11383

## How should we test and review this PR?
Add test `build_script_outside_pkg_root`,  this will check `custom_build.rs` existence and whether in the package root, if not then exited with a hard error

## Additional information
The code just handle the `custom build` target that i know how to test it.  Other target type is skipped.
  • Loading branch information
bors committed Nov 24, 2023
2 parents 527b35e + 75aaa40 commit 22bbc95
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 0 deletions.
43 changes: 43 additions & 0 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::sync::Arc;
use std::task::Poll;

use crate::core::compiler::{BuildConfig, CompileMode, DefaultExecutor, Executor};
use crate::core::manifest::Target;
use crate::core::resolver::CliFeatures;
use crate::core::{registry::PackageRegistry, resolver::HasDevUnits};
use crate::core::{Feature, Shell, Verbosity, Workspace};
Expand Down Expand Up @@ -331,6 +332,23 @@ fn build_ar_list(
warn_on_nonexistent_file(&pkg, &readme_path, "readme", &ws)?;
}
}

for t in pkg
.manifest()
.targets()
.iter()
.filter(|t| t.is_custom_build())
{
if let Some(custome_build_path) = t.src_path().path() {
let abs_custome_build_path =
paths::normalize_path(&pkg.root().join(custome_build_path));
if !abs_custome_build_path.is_file() || !abs_custome_build_path.starts_with(pkg.root())
{
error_custom_build_file_not_in_package(pkg, &abs_custome_build_path, t)?;
}
}
}

result.sort_unstable_by(|a, b| a.rel_path.cmp(&b.rel_path));

Ok(result)
Expand Down Expand Up @@ -405,6 +423,31 @@ fn warn_on_nonexistent_file(
))
}

fn error_custom_build_file_not_in_package(
pkg: &Package,
path: &Path,
target: &Target,
) -> CargoResult<Vec<ArchiveFile>> {
let tip = {
let description_name = target.description_named();
if path.is_file() {
format!("the source file of {description_name} doesn't appear to be a path inside of the package.\n\
It is at `{}`, whereas the root the package is `{}`.\n",
path.display(), pkg.root().display()
)
} else {
format!("the source file of {description_name} doesn't appear to exist.\n",)
}
};
let msg = format!(
"{}\
This may cause issue during packaging, as modules resolution and resources included via macros are often relative to the path of source files.\n\
Please update the `build` setting in the manifest at `{}` and point to a path inside the root of the package.",
tip, pkg.manifest_path().display()
);
anyhow::bail!(msg)
}

/// Construct `Cargo.lock` for the package to be published.
fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult<String> {
let config = ws.config();
Expand Down
49 changes: 49 additions & 0 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3304,3 +3304,52 @@ fn init_and_add_inner_target(p: ProjectBuilder) -> ProjectBuilder {
.file("derp/target/foo.txt", "")
.file("derp/not_target/foo.txt", "")
}

#[cargo_test]
fn build_script_outside_pkg_root() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
license = "MIT"
description = "foo"
authors = []
build = "../t_custom_build/custom_build.rs"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();
let mut expect_msg = String::from("\
warning: manifest has no documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
error: the source file of build script doesn't appear to exist.
This may cause issue during packaging, as modules resolution and resources included via macros are often relative to the path of source files.
Please update the `build` setting in the manifest at `[CWD]/Cargo.toml` and point to a path inside the root of the package.
");
// custom_build.rs does not exist
p.cargo("package -l")
.with_status(101)
.with_stderr(&expect_msg)
.run();

// custom_build.rs outside the package root
let custom_build_root = paths::root().join("t_custom_build");
_ = fs::create_dir(&custom_build_root).unwrap();
_ = fs::write(&custom_build_root.join("custom_build.rs"), "fn main() {}");
expect_msg = format!(
"\
warning: manifest has no documentation, homepage or repository.
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
error: the source file of build script doesn't appear to be a path inside of the package.
It is at `{}/t_custom_build/custom_build.rs`, whereas the root the package is `[CWD]`.
This may cause issue during packaging, as modules resolution and resources included via macros are often relative to the path of source files.
Please update the `build` setting in the manifest at `[CWD]/Cargo.toml` and point to a path inside the root of the package.
", paths::root().display());
p.cargo("package -l")
.with_status(101)
.with_stderr(&expect_msg)
.run();
}

0 comments on commit 22bbc95

Please sign in to comment.