Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error on absolute src paths #2853

Merged
merged 1 commit into from
Sep 10, 2024
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
8 changes: 4 additions & 4 deletions crate_universe/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl Context {
Ok(serde_json::from_str(&data)?)
}

pub(crate) fn new(annotations: Annotations, sources_are_present: bool) -> Result<Self> {
pub(crate) fn new(annotations: Annotations, sources_are_present: bool) -> anyhow::Result<Self> {
// Build a map of crate contexts
let crates: BTreeMap<CrateId, CrateContext> = annotations
.metadata
Expand All @@ -68,11 +68,11 @@ impl Context {
annotations.config.generate_binaries,
annotations.config.generate_build_scripts,
sources_are_present,
);
)?;
let id = CrateId::new(context.name.clone(), context.version.clone());
(id, context)
Ok::<_, anyhow::Error>((id, context))
})
.collect();
.collect::<Result<_, _>>()?;

// Filter for any crate that contains a binary
let binary_crates: BTreeSet<CrateId> = crates
Expand Down
83 changes: 63 additions & 20 deletions crate_universe/src/context/crate_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ impl CrateContext {
include_binaries: bool,
include_build_scripts: bool,
sources_are_present: bool,
) -> Self {
) -> anyhow::Result<Self> {
let package: &Package = &packages[&annotation.node.id];
let current_crate_id = CrateId::new(package.name.clone(), package.version.clone());

Expand Down Expand Up @@ -430,7 +430,7 @@ impl CrateContext {
gen_binaries,
include_build_scripts,
sources_are_present,
);
)?;

// Parse the library crate name from the set of included targets
let library_target_name = {
Expand Down Expand Up @@ -511,7 +511,7 @@ impl CrateContext {
};

// Create the crate's context and apply extra settings
CrateContext {
Ok(CrateContext {
name: package.name.clone(),
version: package.version.clone(),
license: package.license.clone(),
Expand All @@ -529,7 +529,7 @@ impl CrateContext {
alias_rule: None,
override_targets: BTreeMap::new(),
}
.with_overrides(extras)
.with_overrides(extras))
}

fn with_overrides(mut self, extras: &BTreeMap<CrateId, PairedExtras>) -> Self {
Expand Down Expand Up @@ -755,7 +755,7 @@ impl CrateContext {
gen_binaries: &GenBinaries,
include_build_scripts: bool,
sources_are_present: bool,
) -> BTreeSet<Rule> {
) -> anyhow::Result<BTreeSet<Rule>> {
let package = &packages[&node.id];

let package_root = package
Expand All @@ -774,6 +774,10 @@ impl CrateContext {
// content to align when rendering, the package target names are always sanitized.
let crate_name = sanitize_module_name(&target.name);

if !target.src_path.starts_with(package_root) {
return Some(Err(anyhow::anyhow!("Package {:?} target {:?} had an absolute source path {:?}, which is not supported", package.name, target.name, target.src_path)));
}

// Locate the crate's root source file relative to the package root normalized for unix
let crate_root = pathdiff::diff_paths(&target.src_path, package_root).map(
// Normalize the path so that it always renders the same regardless of platform
Expand All @@ -782,29 +786,29 @@ impl CrateContext {

// Conditionally check to see if the dependencies is a build-script target
if include_build_scripts && kind == "custom-build" {
return Some(Rule::BuildScript(TargetAttributes {
return Some(Ok(Rule::BuildScript(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(!sources_are_present),
}));
})));
}

// Check to see if the dependencies is a proc-macro target
if kind == "proc-macro" {
return Some(Rule::ProcMacro(TargetAttributes {
return Some(Ok(Rule::ProcMacro(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(!sources_are_present),
}));
})));
}

// Check to see if the dependencies is a library target
if ["lib", "rlib"].contains(&kind.as_str()) {
return Some(Rule::Library(TargetAttributes {
return Some(Ok(Rule::Library(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(!sources_are_present),
}));
})));
}

// Check if the target kind is binary and is one of the ones included in gen_binaries
Expand All @@ -814,11 +818,11 @@ impl CrateContext {
GenBinaries::Some(set) => set.contains(&target.name),
}
{
return Some(Rule::Binary(TargetAttributes {
return Some(Ok(Rule::Binary(TargetAttributes {
crate_name: target.name.clone(),
crate_root,
srcs: Glob::new_rust_srcs(!sources_are_present),
}));
})));
}

None
Expand Down Expand Up @@ -866,7 +870,8 @@ mod test {
include_binaries,
include_build_scripts,
are_sources_present,
);
)
.unwrap();

assert_eq!(context.name, "common");
assert_eq!(
Expand Down Expand Up @@ -914,7 +919,8 @@ mod test {
include_binaries,
include_build_scripts,
are_sources_present,
);
)
.unwrap();

assert_eq!(context.name, "common");
assert_eq!(
Expand Down Expand Up @@ -979,7 +985,8 @@ mod test {
include_binaries,
include_build_scripts,
are_sources_present,
);
)
.unwrap();

assert_eq!(context.name, "openssl-sys");
assert!(context.build_script_attrs.is_some());
Expand Down Expand Up @@ -1026,7 +1033,8 @@ mod test {
include_binaries,
include_build_scripts,
are_sources_present,
);
)
.unwrap();

assert_eq!(context.name, "openssl-sys");
assert!(context.build_script_attrs.is_none());
Expand Down Expand Up @@ -1062,7 +1070,8 @@ mod test {
include_binaries,
include_build_scripts,
are_sources_present,
);
)
.unwrap();

assert_eq!(context.name, "sysinfo");
assert!(context.build_script_attrs.is_none());
Expand Down Expand Up @@ -1104,7 +1113,8 @@ mod test {
include_binaries,
include_build_scripts,
are_sources_present,
);
)
.unwrap();

assert_eq!(context.name, "common");
check_context(context);
Expand Down Expand Up @@ -1236,11 +1246,44 @@ mod test {
include_binaries,
include_build_scripts,
are_sources_present,
);
)
.unwrap();

let mut expected = Select::new();
expected.insert("unique_feature".to_owned(), None);

assert_eq!(context.common_attrs.crate_features, expected);
}

#[test]
fn absolute_paths_for_srcs_are_errors() {
let annotations = Annotations::new(
crate::test::metadata::abspath(),
crate::test::lockfile::abspath(),
crate::config::Config::default(),
)
.unwrap();

let crate_annotation = &annotations.metadata.crates[&PackageId {
repr: "path+file://{TEMP_DIR}/common#0.1.0".to_owned(),
}];

let include_binaries = false;
let include_build_scripts = false;
let are_sources_present = false;
let err = CrateContext::new(
crate_annotation,
&annotations.metadata.packages,
&annotations.lockfile.crates,
&annotations.pairred_extras,
&annotations.metadata.workspace_metadata.tree_metadata,
include_binaries,
include_build_scripts,
are_sources_present,
)
.unwrap_err()
.to_string();

assert_eq!(err, "Package \"common\" target \"common\" had an absolute source path \"/dev/null\", which is not supported");
}
}
16 changes: 16 additions & 0 deletions crate_universe/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,14 @@ pub(crate) mod metadata {
)))
.unwrap()
}

pub(crate) fn abspath() -> cargo_metadata::Metadata {
serde_json::from_str(include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/test_data/metadata/abspath/metadata.json"
)))
.unwrap()
}
}

pub(crate) mod lockfile {
Expand Down Expand Up @@ -238,4 +246,12 @@ pub(crate) mod lockfile {
)))
.unwrap()
}

pub(crate) fn abspath() -> cargo_lock::Lockfile {
cargo_lock::Lockfile::from_str(include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/test_data/metadata/abspath/Cargo.lock"
)))
.unwrap()
}
}
7 changes: 7 additions & 0 deletions crate_universe/test_data/metadata/abspath/Cargo.lock

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

7 changes: 7 additions & 0 deletions crate_universe/test_data/metadata/abspath/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "common"
version = "0.1.0"
edition = "2018"

[lib]
path = "fake.rs"
66 changes: 66 additions & 0 deletions crate_universe/test_data/metadata/abspath/metadata.json

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

Loading