Refactor forc_pkg::BuildConfig -> BuildProfile, fix CLI arg handling (#2094)

Previously, if any of the `print` args were set, the rest of the
selected build profile was ignored. This changes the behaviour so that
the command line arguments only override their associated build profile
fields.

Also renames `BuildConfig` to `BuildProfile` and moves it from
`forc_pkg::pkg` to `forc_pkg::manifest` along with the rest of the
serializable manifest types.

Author: mitchmindtree

forc-pkg/src/lib.rs

@@ -9,6 +9,6 @@ pub mod manifest;
 mod pkg;
 
 pub use lock::Lock;
-pub use manifest::{Manifest, ManifestFile};
+pub use manifest::{BuildProfile, Manifest, ManifestFile};
 #[doc(inline)]
 pub use pkg::*;

forc-pkg/src/manifest.rs

@@ -1,7 +1,4 @@
-use crate::{
-    pkg::{manifest_file_missing, parsing_failed, wrong_program_type},
-    BuildConfig,
-};
+use crate::pkg::{manifest_file_missing, parsing_failed, wrong_program_type};
 use anyhow::{anyhow, bail, Result};
 use forc_util::{find_manifest_dir, println_yellow_err, validate_name};
 use serde::{Deserialize, Serialize};
@@ -30,7 +27,7 @@ pub struct Manifest {
     pub project: Project,
     pub network: Option<Network>,
     pub dependencies: Option<BTreeMap<String, Dependency>>,
-    pub build_profile: Option<BTreeMap<String, BuildConfig>>,
+    build_profile: Option<BTreeMap<String, BuildProfile>>,
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -76,6 +73,16 @@ pub struct DependencyDetails {
     pub(crate) rev: Option<String>,
 }
 
+/// Parameters to pass through to the `sway_core::BuildConfig` during compilation.
+#[derive(Serialize, Deserialize, Debug, Clone)]
+#[serde(rename_all = "kebab-case")]
+pub struct BuildProfile {
+    pub print_ir: bool,
+    pub print_finalized_asm: bool,
+    pub print_intermediate_asm: bool,
+    pub silent: bool,
+}
+
 impl Dependency {
     /// The string of the `package` field if specified.
     pub fn package(&self) -> Option<&str> {
@@ -182,6 +189,13 @@ impl ManifestFile {
             Ok(())
         }
     }
+
+    /// Access the build profile associated with the given profile name.
+    pub fn build_profile(&self, profile_name: &str) -> Option<&BuildProfile> {
+        self.build_profile
+            .as_ref()
+            .and_then(|profiles| profiles.get(profile_name))
+    }
 }
 
 impl Manifest {
@@ -241,7 +255,7 @@ impl Manifest {
     }
 
     /// Produce an iterator yielding all listed build profiles.
-    pub fn build_profiles(&self) -> impl Iterator<Item = (&String, &BuildConfig)> {
+    pub fn build_profiles(&self) -> impl Iterator<Item = (&String, &BuildProfile)> {
         self.build_profile
             .as_ref()
             .into_iter()
@@ -291,32 +305,13 @@ impl Manifest {
     /// If they are provided, use the provided `debug` or `release` so that they override the default `debug`
     /// and `release`.
     fn implicitly_include_default_build_profiles_if_missing(&mut self) {
-        const DEBUG: &str = "debug";
-        const RELEASE: &str = "release";
-
         let build_profiles = self.build_profile.get_or_insert_with(Default::default);
 
-        if build_profiles.get(DEBUG).is_none() {
-            build_profiles.insert(
-                DEBUG.to_string(),
-                BuildConfig {
-                    print_ir: false,
-                    print_finalized_asm: false,
-                    print_intermediate_asm: false,
-                    silent: false,
-                },
-            );
+        if build_profiles.get(BuildProfile::DEBUG).is_none() {
+            build_profiles.insert(BuildProfile::DEBUG.into(), BuildProfile::debug());
         }
-        if build_profiles.get(RELEASE).is_none() {
-            build_profiles.insert(
-                RELEASE.to_string(),
-                BuildConfig {
-                    print_ir: false,
-                    print_finalized_asm: false,
-                    print_intermediate_asm: false,
-                    silent: false,
-                },
-            );
+        if build_profiles.get(BuildProfile::RELEASE).is_none() {
+            build_profiles.insert(BuildProfile::RELEASE.into(), BuildProfile::release());
         }
     }
 
@@ -341,13 +336,43 @@ impl Manifest {
     }
 }
 
+impl BuildProfile {
+    pub const DEBUG: &'static str = "debug";
+    pub const RELEASE: &'static str = "release";
+    pub const DEFAULT: &'static str = Self::DEBUG;
+
+    pub fn debug() -> Self {
+        Self {
+            print_ir: false,
+            print_finalized_asm: false,
+            print_intermediate_asm: false,
+            silent: false,
+        }
+    }
+
+    pub fn release() -> Self {
+        Self {
+            print_ir: false,
+            print_finalized_asm: false,
+            print_intermediate_asm: false,
+            silent: false,
+        }
+    }
+}
+
 impl std::ops::Deref for ManifestFile {
     type Target = Manifest;
     fn deref(&self) -> &Self::Target {
         &self.manifest
     }
 }
 
+impl Default for BuildProfile {
+    fn default() -> Self {
+        Self::debug()
+    }
+}
+
 /// The definition for the implicit `std` dependency.
 fn implicit_std_dep(sway_git_tag: String) -> Dependency {
     const SWAY_GIT_REPO_URL: &str = "https://github.com/fuellabs/sway";

forc-pkg/src/pkg.rs

@@ -1,6 +1,6 @@
 use crate::{
     lock::Lock,
-    manifest::{Dependency, Manifest, ManifestFile},
+    manifest::{BuildProfile, Dependency, Manifest, ManifestFile},
 };
 use anyhow::{anyhow, bail, Context, Error, Result};
 use forc_util::{
@@ -166,16 +166,6 @@ pub struct BuildPlan {
     compilation_order: Vec<NodeIx>,
 }
 
-/// Parameters to pass through to the `sway_core::BuildConfig` during compilation.
-#[derive(Serialize, Deserialize, Debug)]
-#[serde(rename_all = "kebab-case")]
-pub struct BuildConfig {
-    pub print_ir: bool,
-    pub print_finalized_asm: bool,
-    pub print_intermediate_asm: bool,
-    pub silent: bool,
-}
-
 /// Error returned upon failed parsing of `PinnedId::from_str`.
 #[derive(Clone, Debug)]
 pub struct PinnedIdParseError;
@@ -1327,22 +1317,22 @@ fn dep_to_source(pkg_path: &Path, dep: &Dependency) -> Result<Source> {
     Ok(source)
 }
 
-/// Given a `forc_pkg::BuildConfig`, produce the necessary `sway_core::BuildConfig` required for
+/// Given a `forc_pkg::BuildProfile`, produce the necessary `sway_core::BuildConfig` required for
 /// compilation.
 pub fn sway_build_config(
     manifest_dir: &Path,
     entry_path: &Path,
-    build_conf: &BuildConfig,
+    build_profile: &BuildProfile,
 ) -> Result<sway_core::BuildConfig> {
     // Prepare the build config to pass through to the compiler.
     let file_name = find_file_name(manifest_dir, entry_path)?;
     let build_config = sway_core::BuildConfig::root_from_file_name_and_manifest_path(
         file_name.to_path_buf(),
         manifest_dir.to_path_buf(),
     )
-    .print_finalized_asm(build_conf.print_finalized_asm)
-    .print_intermediate_asm(build_conf.print_intermediate_asm)
-    .print_ir(build_conf.print_ir);
+    .print_finalized_asm(build_profile.print_finalized_asm)
+    .print_intermediate_asm(build_profile.print_intermediate_asm)
+    .print_ir(build_profile.print_ir);
     Ok(build_config)
 }
 
@@ -1384,12 +1374,12 @@ pub fn dependency_namespace(
 /// Compiles the package to an AST.
 pub fn compile_ast(
     manifest: &ManifestFile,
-    build_config: &BuildConfig,
+    build_profile: &BuildProfile,
     namespace: namespace::Module,
 ) -> Result<CompileAstResult> {
     let source = manifest.entry_string()?;
     let sway_build_config =
-        sway_build_config(manifest.dir(), &manifest.entry_path(), build_config)?;
+        sway_build_config(manifest.dir(), &manifest.entry_path(), build_profile)?;
     let ast_res = sway_core::compile_to_ast(source, namespace, Some(&sway_build_config));
     Ok(ast_res)
 }
@@ -1415,16 +1405,16 @@ pub fn compile_ast(
 pub fn compile(
     pkg: &Pinned,
     manifest: &ManifestFile,
-    build_config: &BuildConfig,
+    build_profile: &BuildProfile,
     namespace: namespace::Module,
     source_map: &mut SourceMap,
 ) -> Result<(Compiled, Option<namespace::Root>)> {
     let entry_path = manifest.entry_path();
-    let sway_build_config = sway_build_config(manifest.dir(), &entry_path, build_config)?;
-    let silent_mode = build_config.silent;
+    let sway_build_config = sway_build_config(manifest.dir(), &entry_path, build_profile)?;
+    let silent_mode = build_profile.silent;
 
     // First, compile to an AST. We'll update the namespace and check for JSON ABI output.
-    let ast_res = compile_ast(manifest, build_config, namespace)?;
+    let ast_res = compile_ast(manifest, build_profile, namespace)?;
     match &ast_res {
         CompileAstResult::Failure { warnings, errors } => {
             print_on_failure(silent_mode, warnings, errors);
@@ -1487,7 +1477,7 @@ pub fn compile(
 /// Also returns the resulting `sway_core::SourceMap` which may be useful for debugging purposes.
 pub fn build(
     plan: &BuildPlan,
-    conf: &BuildConfig,
+    profile: &BuildProfile,
     sway_git_tag: &str,
 ) -> anyhow::Result<(Compiled, SourceMap)> {
     let mut namespace_map = Default::default();
@@ -1501,7 +1491,7 @@ pub fn build(
         let pkg = &plan.graph[node];
         let path = &plan.path_map[&pkg.id()];
         let manifest = ManifestFile::from_dir(path, sway_git_tag)?;
-        let res = compile(pkg, &manifest, conf, dep_namespace, &mut source_map)?;
+        let res = compile(pkg, &manifest, profile, dep_namespace, &mut source_map)?;
         let (compiled, maybe_namespace) = res;
         if let Some(namespace) = maybe_namespace {
             namespace_map.insert(node, namespace.into());
@@ -1527,11 +1517,9 @@ pub fn check(
     silent_mode: bool,
     sway_git_tag: &str,
 ) -> anyhow::Result<CompileAstResult> {
-    let conf = &BuildConfig {
-        print_ir: false,
-        print_finalized_asm: false,
-        print_intermediate_asm: false,
+    let profile = BuildProfile {
         silent: silent_mode,
+        ..BuildProfile::debug()
     };
 
     let mut namespace_map = Default::default();
@@ -1542,7 +1530,7 @@ pub fn check(
         let pkg = &plan.graph[node];
         let path = &plan.path_map[&pkg.id()];
         let manifest = ManifestFile::from_dir(path, sway_git_tag)?;
-        let ast_res = compile_ast(&manifest, conf, dep_namespace)?;
+        let ast_res = compile_ast(&manifest, &profile, dep_namespace)?;
         if let CompileAstResult::Success { typed_program, .. } = &ast_res {
             if let TreeType::Library { .. } = typed_program.kind.tree_type() {
                 namespace_map.insert(node, typed_program.root.namespace.clone());

forc/src/ops/forc_build.rs

@@ -58,32 +58,25 @@ pub fn build(command: BuildCommand) -> Result<pkg::Compiled> {
 
     let plan = pkg::BuildPlan::load_from_manifest(&manifest, locked, offline, SWAY_GIT_TAG)?;
 
-    // If any cli parameter is passed by the user it overrides the selected build profile.
-    let mut config = &pkg::BuildConfig {
-        print_ir,
-        print_finalized_asm,
-        print_intermediate_asm,
-        silent: silent_mode,
-    };
-
-    // Check if any cli parameter is passed by the user if not fetch the build profile from manifest.
-    if !print_ir && !print_intermediate_asm && !print_finalized_asm && !silent_mode {
-        config = manifest
-            .build_profile
-            .as_ref()
-            .and_then(|profiles| profiles.get(&selected_build_profile))
-            .unwrap_or_else(|| {
-                warn!(
-                    "provided profile option {} is not present in the manifest file. \
-                Using default config.",
-                    selected_build_profile
-                );
-                config
-            });
-    }
+    // Retrieve the specified build profile
+    let mut profile = manifest
+        .build_profile(&selected_build_profile)
+        .cloned()
+        .unwrap_or_else(|| {
+            warn!(
+                "provided profile option {} is not present in the manifest file. \
+            Using default profile.",
+                selected_build_profile
+            );
+            Default::default()
+        });
+    profile.print_ir |= print_ir;
+    profile.print_finalized_asm |= print_finalized_asm;
+    profile.print_intermediate_asm |= print_intermediate_asm;
+    profile.silent |= silent_mode;
 
     // Build it!
-    let (compiled, source_map) = pkg::build(&plan, config, SWAY_GIT_TAG)?;
+    let (compiled, source_map) = pkg::build(&plan, &profile, SWAY_GIT_TAG)?;
 
     if let Some(outfile) = binary_outfile {
         fs::write(&outfile, &compiled.bytecode)?;