fix(toml): Warn, rather than fail publish, if build.rs is excluded

This could offer a minor performance gain when reading this manifest
since the target doesn't need to be discovered.

Author: epage

crates/cargo-util-schemas/src/manifest/mod.rs

@@ -215,6 +215,15 @@ impl TomlPackage {
         self.authors.as_ref().map(|v| v.resolved()).transpose()
     }
 
+    pub fn resolved_build(&self) -> Result<Option<&String>, UnresolvedError> {
+        let readme = self.build.as_ref().ok_or(UnresolvedError)?;
+        match readme {
+            StringOrBool::Bool(false) => Ok(None),
+            StringOrBool::Bool(true) => Err(UnresolvedError),
+            StringOrBool::String(value) => Ok(Some(value)),
+        }
+    }
+
     pub fn resolved_exclude(&self) -> Result<Option<&Vec<String>>, UnresolvedError> {
         self.exclude.as_ref().map(|v| v.resolved()).transpose()
     }

src/cargo/ops/cargo_package.rs

@@ -689,7 +689,11 @@ fn tar(
 
     let base_name = format!("{}-{}", pkg.name(), pkg.version());
     let base_path = Path::new(&base_name);
-    let publish_pkg = prepare_for_publish(pkg, ws)?;
+    let included = ar_files
+        .iter()
+        .map(|ar_file| ar_file.rel_path.clone())
+        .collect::<Vec<_>>();
+    let publish_pkg = prepare_for_publish(pkg, ws, &included)?;
 
     let mut uncompressed_size = 0;
     for ar_file in ar_files {

src/cargo/util/toml/mod.rs

@@ -494,7 +494,7 @@ fn resolve_package_toml<'a>(
             .map(|value| field_inherit_with(value, "authors", || inherit()?.authors()))
             .transpose()?
             .map(manifest::InheritableField::Value),
-        build: original_package.build.clone(),
+        build: targets::resolve_build(original_package.build.as_ref(), package_root),
         metabuild: original_package.metabuild.clone(),
         default_target: original_package.default_target.clone(),
         forced_target: original_package.forced_target.clone(),
@@ -1153,7 +1153,6 @@ fn to_real_manifest(
         package_name,
         package_root,
         edition,
-        &resolved_package.build,
         &resolved_package.metabuild,
         warnings,
         errors,
@@ -2360,10 +2359,15 @@ fn unused_dep_keys(
     }
 }
 
-pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult<Package> {
+pub fn prepare_for_publish(
+    me: &Package,
+    ws: &Workspace<'_>,
+    included: &[PathBuf],
+) -> CargoResult<Package> {
     let contents = me.manifest().contents();
     let document = me.manifest().document();
-    let original_toml = prepare_toml_for_publish(me.manifest().resolved_toml(), ws, me.root())?;
+    let original_toml =
+        prepare_toml_for_publish(me.manifest().resolved_toml(), ws, me.root(), included)?;
     let resolved_toml = original_toml.clone();
     let features = me.manifest().unstable_features().clone();
     let workspace_config = me.manifest().workspace_config().clone();
@@ -2395,6 +2399,7 @@ fn prepare_toml_for_publish(
     me: &manifest::TomlManifest,
     ws: &Workspace<'_>,
     package_root: &Path,
+    included: &[PathBuf],
 ) -> CargoResult<manifest::TomlManifest> {
     let gctx = ws.gctx();
 
@@ -2411,11 +2416,21 @@ fn prepare_toml_for_publish(
     package.workspace = None;
     if let Some(StringOrBool::String(path)) = &package.build {
         let path = paths::normalize_path(Path::new(path));
-        let path = path
-            .into_os_string()
-            .into_string()
-            .map_err(|_err| anyhow::format_err!("non-UTF8 `package.build`"))?;
-        package.build = Some(StringOrBool::String(normalize_path_string_sep(path)));
+        let build = if included.contains(&path) {
+            let path = path
+                .into_os_string()
+                .into_string()
+                .map_err(|_err| anyhow::format_err!("non-UTF8 `package.build`"))?;
+            let path = normalize_path_string_sep(path);
+            StringOrBool::String(path)
+        } else {
+            ws.gctx().shell().warn(format!(
+                "ignoring `package.build` as `{}` is not included in the published package",
+                path.display()
+            ))?;
+            StringOrBool::Bool(false)
+        };
+        package.build = Some(build);
     }
     let current_resolver = package
         .resolver

src/cargo/util/toml/targets.rs

@@ -38,7 +38,6 @@ pub(super) fn to_targets(
     package_name: &str,
     package_root: &Path,
     edition: Edition,
-    custom_build: &Option<StringOrBool>,
     metabuild: &Option<StringOrVec>,
     warnings: &mut Vec<String>,
     errors: &mut Vec<String>,
@@ -120,10 +119,11 @@ pub(super) fn to_targets(
     targets.extend(to_bench_targets(&toml_benches, package_root, edition)?);
 
     // processing the custom build script
-    if let Some(custom_build) = maybe_custom_build(custom_build, package_root) {
+    if let Some(custom_build) = package.resolved_build().expect("should be resolved") {
         if metabuild.is_some() {
             anyhow::bail!("cannot specify both `metabuild` and `build`");
         }
+        let custom_build = Path::new(custom_build);
         let name = format!(
             "build-script-{}",
             custom_build
@@ -1072,22 +1072,22 @@ Cargo doesn't know which to use because multiple target files found at `{}` and
 }
 
 /// Returns the path to the build script if one exists for this crate.
-fn maybe_custom_build(build: &Option<StringOrBool>, package_root: &Path) -> Option<PathBuf> {
-    let build_rs = package_root.join("build.rs");
-    match *build {
-        // Explicitly no build script.
-        Some(StringOrBool::Bool(false)) => None,
-        Some(StringOrBool::Bool(true)) => Some(build_rs),
-        Some(StringOrBool::String(ref s)) => Some(PathBuf::from(s)),
+pub fn resolve_build(build: Option<&StringOrBool>, package_root: &Path) -> Option<StringOrBool> {
+    const BUILD_RS: &str = "build.rs";
+    match build {
         None => {
             // If there is a `build.rs` file next to the `Cargo.toml`, assume it is
             // a build script.
+            let build_rs = package_root.join(BUILD_RS);
             if build_rs.is_file() {
-                Some(build_rs)
+                Some(StringOrBool::String(BUILD_RS.to_owned()))
             } else {
-                None
+                Some(StringOrBool::Bool(false))
             }
         }
+        // Explicitly no build script.
+        Some(StringOrBool::Bool(false)) | Some(StringOrBool::String(_)) => build.cloned(),
+        Some(StringOrBool::Bool(true)) => Some(StringOrBool::String(BUILD_RS.to_owned())),
     }
 }
 

tests/testsuite/artifact_dep.rs

@@ -2214,6 +2214,7 @@ edition = "2015"
 name = "foo"
 version = "0.1.0"
 authors = []
+build = false
 description = "foo"
 homepage = "foo"
 documentation = "foo"

tests/testsuite/features2.rs

@@ -1703,6 +1703,7 @@ edition = "2015"
 name = "a"
 version = "0.1.0"
 authors = ["Zzz"]
+build = false
 description = "foo"
 homepage = "https://example.com/"
 readme = false

tests/testsuite/features_namespaced.rs

@@ -985,6 +985,7 @@ You may press ctrl-c [..]
 edition = "2015"
 name = "foo"
 version = "0.1.0"
+build = false
 description = "foo"
 homepage = "https://example.com/"
 readme = false
@@ -1104,6 +1105,7 @@ You may press ctrl-c [..]
 edition = "2015"
 name = "foo"
 version = "0.1.0"
+build = false
 description = "foo"
 homepage = "https://example.com/"
 readme = false

tests/testsuite/inheritable_workspace_fields.rs

@@ -217,6 +217,7 @@ rust-version = "1.60"
 name = "foo"
 version = "1.2.3"
 authors = ["Rustaceans"]
+build = false
 exclude = ["foo.txt"]
 include = [
     "bar.txt",
@@ -384,6 +385,7 @@ edition = "2015"
 name = "bar"
 version = "0.2.0"
 authors = []
+build = false
 readme = false
 
 [dependencies.dep]
@@ -516,6 +518,7 @@ edition = "2015"
 name = "bar"
 version = "0.2.0"
 authors = []
+build = false
 readme = false
 
 [dependencies.dep]
@@ -758,6 +761,7 @@ rust-version = "1.60"
 name = "bar"
 version = "1.2.3"
 authors = ["Rustaceans"]
+build = false
 exclude = ["foo.txt"]
 include = [
     "bar.txt",
@@ -930,6 +934,7 @@ edition = "2015"
 name = "bar"
 version = "0.2.0"
 authors = []
+build = false
 readme = false
 
 [dependencies.dep]