Fix single package features in a workspace (#1593)

fix #1516 #1599

extensions/scarb-doc/tests/features.rs

@@ -112,7 +112,7 @@ fn test_workspace_without_features_in_manifest() {
         error: metadata command failed: `scarb metadata` exited with error
 
         stdout:
-        error: no features in manifest
+        error: none of the selected packages contains `test_feature` feature
         note: to use features, you need to define [features] section in Scarb.toml
 
         stderr:
@@ -125,7 +125,7 @@ fn test_workspace_without_features_in_manifest() {
         error: metadata command failed: `scarb metadata` exited with error
 
         stdout:
-        error: no features in manifest
+        error: none of the selected packages contains `test_feature` feature
         note: to use features, you need to define [features] section in Scarb.toml
 
         stderr:
@@ -225,7 +225,7 @@ fn test_workspace_with_working_feature_in_root_only() {
         .lib_cairo(COMMON_CODE_WITH_FEATURE)
         .build(&child_dir);
 
-    let snapbox = Scarb::quick_snapbox()
+    Scarb::quick_snapbox()
         .env("RUST_BACKTRACE", "0")
         .arg("doc")
         .args([
@@ -237,32 +237,7 @@ fn test_workspace_with_working_feature_in_root_only() {
         ])
         .current_dir(&root_dir)
         .assert()
-        .failure();
-
-    #[cfg(windows)]
-    snapbox.stdout_matches(indoc! {r#"
-        error: metadata command failed: `scarb metadata` exited with error
-
-        stdout:
-        error: no features in manifest
-        note: to use features, you need to define [features] section in Scarb.toml
-
-        stderr:
-        
-        error: process did not exit successfully: exit code: 1
-        "#});
-
-    #[cfg(not(windows))]
-    snapbox.stdout_matches(indoc! {r#"
-        error: metadata command failed: `scarb metadata` exited with error
-
-        stdout:
-        error: no features in manifest
-        note: to use features, you need to define [features] section in Scarb.toml
-
-        stderr:
-
-        "#});
+        .success();
 }
 
 #[test]
@@ -290,7 +265,7 @@ fn test_workspace_with_working_feature_in_sub_package_only() {
         .lib_cairo(COMMON_CODE_WITH_FEATURE)
         .build(&child_dir);
 
-    let snapbox = Scarb::quick_snapbox()
+    Scarb::quick_snapbox()
         .env("RUST_BACKTRACE", "0")
         .arg("doc")
         .args([
@@ -302,32 +277,7 @@ fn test_workspace_with_working_feature_in_sub_package_only() {
         ])
         .current_dir(&root_dir)
         .assert()
-        .failure();
-
-    #[cfg(windows)]
-    snapbox.stdout_matches(indoc! {r#"
-        error: metadata command failed: `scarb metadata` exited with error
-
-        stdout:
-        error: no features in manifest
-        note: to use features, you need to define [features] section in Scarb.toml
-
-        stderr:
-        
-        error: process did not exit successfully: exit code: 1
-        "#});
-
-    #[cfg(not(windows))]
-    snapbox.stdout_matches(indoc! {r#"
-        error: metadata command failed: `scarb metadata` exited with error
-
-        stdout:
-        error: no features in manifest
-        note: to use features, you need to define [features] section in Scarb.toml
-        
-        stderr:
-
-        "#});
+        .success();
 }
 
 #[test]
@@ -367,7 +317,7 @@ fn test_workspace_without_features_in_manifest_and_present_in_sub_package_code()
             error: metadata command failed: `scarb metadata` exited with error
     
             stdout:
-            error: no features in manifest
+            error: none of the selected packages contains `test_feature` feature
             note: to use features, you need to define [features] section in Scarb.toml
     
             stderr:
@@ -380,7 +330,7 @@ fn test_workspace_without_features_in_manifest_and_present_in_sub_package_code()
             error: metadata command failed: `scarb metadata` exited with error
     
             stdout:
-            error: no features in manifest
+            error: none of the selected packages contains `test_feature` feature
             note: to use features, you need to define [features] section in Scarb.toml
     
             stderr:
@@ -426,7 +376,7 @@ fn test_workspace_without_features_in_manifest_and_present_in_root_package_code(
         error: metadata command failed: `scarb metadata` exited with error
 
         stdout:
-        error: no features in manifest
+        error: none of the selected packages contains `test_feature` feature
         note: to use features, you need to define [features] section in Scarb.toml
 
         stderr:
@@ -439,7 +389,7 @@ fn test_workspace_without_features_in_manifest_and_present_in_root_package_code(
         error: metadata command failed: `scarb metadata` exited with error
 
         stdout:
-        error: no features in manifest
+        error: none of the selected packages contains `test_feature` feature
         note: to use features, you need to define [features] section in Scarb.toml
 
         stderr:

scarb/src/bin/scarb/commands/package.rs

@@ -2,11 +2,12 @@ use std::collections::BTreeMap;
 
 use anyhow::Result;
 use camino::Utf8PathBuf;
+use itertools::Itertools;
 use serde::Serializer;
 
 use scarb::core::{Config, PackageName};
 use scarb::ops;
-use scarb::ops::PackageOpts;
+use scarb::ops::{validate_features, PackageOpts};
 use scarb_ui::Message;
 
 use crate::args::PackageArgs;
@@ -18,17 +19,20 @@ pub fn run(args: PackageArgs, config: &Config) -> Result<()> {
         .packages_filter
         .match_many(&ws)?
         .into_iter()
-        .map(|p| p.id)
-        .collect::<Vec<_>>();
+        .collect_vec();
 
+    let features_opts = args.features.try_into()?;
+    validate_features(&packages, &features_opts)?;
     let opts = PackageOpts {
         // Disable dirty repository checks when printing package files.
         allow_dirty: args.list || args.shared_args.allow_dirty,
         verify: !args.shared_args.no_verify,
         check_metadata: !args.no_metadata,
-        features: args.features.try_into()?,
+        features: features_opts,
     };
 
+    let packages = packages.into_iter().map(|p| p.id).collect_vec();
+
     if args.list {
         let result = ops::package_list(&packages, &opts, &ws)?;
         ws.config().ui().print(ListMessage(result));

scarb/src/bin/scarb/commands/publish.rs

@@ -4,7 +4,7 @@ use url::Url;
 
 use scarb::core::registry::DEFAULT_REGISTRY_INDEX;
 use scarb::core::Config;
-use scarb::ops::{self, PackageOpts, PublishOpts};
+use scarb::ops::{self, validate_features, PackageOpts, PublishOpts};
 
 use crate::args::PublishArgs;
 
@@ -17,13 +17,15 @@ pub fn run(args: PublishArgs, config: &Config) -> Result<()> {
         None => Url::from_str(DEFAULT_REGISTRY_INDEX)?,
     };
 
+    let features_opts = args.features.try_into()?;
+    validate_features(&[package.clone()], &features_opts)?;
     let ops = PublishOpts {
         index_url: index,
         package_opts: PackageOpts {
             allow_dirty: args.shared_args.allow_dirty,
             verify: !args.shared_args.no_verify,
             check_metadata: true,
-            features: args.features.try_into()?,
+            features: features_opts,
         },
     };
 

scarb/src/bin/scarb/commands/test.rs

@@ -1,18 +1,21 @@
+use crate::args::TestArgs;
 use anyhow::Result;
-
+use itertools::Itertools;
 use scarb::core::Config;
 use scarb::ops;
-
-use crate::args::TestArgs;
+use scarb::ops::{validate_features, FeaturesOpts};
 
 #[tracing::instrument(skip_all, level = "info")]
 pub fn run(args: TestArgs, config: &Config) -> Result<()> {
     let ws = ops::read_workspace(config.manifest_path(), config)?;
-    args.packages_filter
+    let packages = args
+        .packages_filter
         .match_many(&ws)?
-        .iter()
-        .try_for_each(|package| {
-            ops::execute_test_subcommand(package, &args.args, &ws, args.features.clone())
-                .map(|_| ())
-        })
+        .into_iter()
+        .collect_vec();
+    let features_opts: FeaturesOpts = args.features.clone().try_into()?;
+    validate_features(&packages, &features_opts)?;
+    packages.iter().try_for_each(|package| {
+        ops::execute_test_subcommand(package, &args.args, &ws, args.features.clone()).map(|_| ())
+    })
 }

scarb/src/ops/compile.rs

@@ -19,7 +19,7 @@ use crate::core::{
     FeatureName, PackageId, PackageName, TargetKind, Utf8PathWorkspaceExt, Workspace,
 };
 use crate::ops;
-use crate::ops::get_test_package_ids;
+use crate::ops::{get_test_package_ids, validate_features};
 
 #[derive(Debug, Clone)]
 pub enum FeaturesSelector {
@@ -115,7 +115,11 @@ where
     F: FnMut(CompilationUnit, &Workspace<'_>) -> Result<()>,
 {
     let resolve = ops::resolve_workspace(ws)?;
-
+    let packages_to_process = ws
+        .members()
+        .filter(|p| packages.contains(&p.id))
+        .collect_vec();
+    validate_features(&packages_to_process, &opts.features)?;
     // Add test compilation units to build
     let packages = get_test_package_ids(packages, ws);
     let compilation_units = ops::generate_compilation_units(&resolve, &opts.features, ws)?

scarb/src/ops/expand.rs

@@ -3,7 +3,7 @@ use crate::compiler::helpers::{build_compiler_config, write_string};
 use crate::compiler::{CairoCompilationUnit, CompilationUnit, CompilationUnitAttributes};
 use crate::core::{Package, PackageId, TargetKind, Workspace};
 use crate::ops;
-use crate::ops::{get_test_package_ids, FeaturesOpts};
+use crate::ops::{get_test_package_ids, validate_features, FeaturesOpts};
 use anyhow::{bail, Context, Result};
 use cairo_lang_defs::db::DefsGroup;
 use cairo_lang_defs::ids::{LanguageElementId, ModuleId, ModuleItemId};
@@ -38,6 +38,8 @@ pub struct ExpandOpts {
 }
 
 pub fn expand(package: Package, opts: ExpandOpts, ws: &Workspace<'_>) -> Result<()> {
+    validate_features(&[package.clone()], &opts.features)?;
+
     let package_name = package.id.name.to_string();
     let resolve = ops::resolve_workspace(ws)?;
     let compilation_units = ops::generate_compilation_units(&resolve, &opts.features, ws)?;

scarb/src/ops/resolve.rs

@@ -173,7 +173,12 @@ pub fn generate_compilation_units(
     ws: &Workspace<'_>,
 ) -> Result<Vec<CompilationUnit>> {
     let mut units = Vec::with_capacity(ws.members().size_hint().0);
-    for member in ws.members().filter(|member| !member.is_cairo_plugin()) {
+    let members = ws
+        .members()
+        .filter(|member| !member.is_cairo_plugin())
+        .collect_vec();
+    validate_features(&members, enabled_features)?;
+    for member in members {
         units.extend(generate_cairo_compilation_units(
             &member,
             resolve,
@@ -207,6 +212,25 @@ pub fn generate_compilation_units(
     Ok(units)
 }
 
+pub fn validate_features(members: &[Package], enabled_features: &FeaturesOpts) -> Result<()> {
+    // Check if any member has features defined.
+    if let FeaturesSelector::Features(features) = &enabled_features.features {
+        for feature in features {
+            if !members
+                .iter()
+                .any(|member| member.manifest.features.contains_key(feature))
+            {
+                bail!(
+                    "none of the selected packages contains `{}` feature\n\
+                    note: to use features, you need to define [features] section in Scarb.toml",
+                    feature
+                );
+            }
+        }
+    }
+    Ok(())
+}
+
 fn generate_cairo_compilation_units(
     member: &Package,
     resolve: &WorkspaceResolve,
@@ -377,25 +401,15 @@ fn get_cfg_with_features(
     features_manifest: &BTreeMap<FeatureName, Vec<FeatureName>>,
     enabled_features: &FeaturesOpts,
 ) -> Result<Option<CfgSet>> {
-    if features_manifest.is_empty() {
-        match &enabled_features.features {
-            FeaturesSelector::Features(features) if !features.is_empty() => {
-                bail!(
-                    "no features in manifest\n\
-                    note: to use features, you need to define [features] section in Scarb.toml",
-                )
-            }
-            _ => {
-                return Ok(None);
-            }
-        }
-    }
     let available_features: HashSet<FeatureName> = features_manifest.keys().cloned().collect();
-
     let mut selected_features: HashSet<FeatureName> = match &enabled_features.features {
         FeaturesSelector::AllFeatures => available_features.clone(),
         FeaturesSelector::Features(features) => {
-            let mut features: HashSet<FeatureName> = features.iter().cloned().collect();
+            let features: HashSet<FeatureName> = features.iter().cloned().collect();
+            let mut features: HashSet<FeatureName> = features
+                .intersection(&available_features)
+                .cloned()
+                .collect();
             if !enabled_features.no_default_features {
                 features.extend(
                     features_manifest

scarb/src/ops/subcommands.rs

@@ -13,7 +13,7 @@ use scarb_ui::components::Status;
 
 use crate::core::{Config, Package, ScriptDefinition, Workspace};
 use crate::internal::fsx::is_executable;
-use crate::ops::{self, FeaturesOpts};
+use crate::ops;
 use crate::process::exec_replace;
 use crate::subcommands::{get_env_vars, EXTERNAL_CMD_PREFIX, SCARB_MANIFEST_PATH_ENV};
 
@@ -57,8 +57,6 @@ pub fn execute_test_subcommand(
         SCARB_MANIFEST_PATH_ENV.into(),
         package.manifest_path().to_string(),
     )]);
-    // Validate features opts.
-    let _: FeaturesOpts = features.clone().try_into()?;
     env.extend(features.to_env_vars());
     if let Some(script_definition) = package.manifest.scripts.get("test") {
         debug!("using `test` script: {script_definition}");