refactoring: Change the way non-specified sandbox kind is handled

Also, make SandboxConfig be non-optional, but take default values.

src/checker.rs

@@ -480,7 +480,7 @@ impl Checker {
             }
         }
         for (perm_sel, config) in &self.config.permissions_no_inheritance.packages {
-            if config.sandbox.is_some()
+            if config.sandbox.kind.is_some()
                 && !matches!(
                     perm_sel.scope,
                     PermissionScope::Build | PermissionScope::Test

src/config.rs

@@ -79,7 +79,7 @@ pub(crate) struct CommonConfig {
 #[serde(deny_unknown_fields)]
 pub(crate) struct SandboxConfig {
     #[serde(default)]
-    pub(crate) kind: SandboxKind,
+    pub(crate) kind: Option<SandboxKind>,
 
     #[serde(default)]
     pub(crate) extra_args: Vec<String>,
@@ -119,10 +119,8 @@ pub(crate) struct ApiPath {
     pub(crate) prefix: Arc<str>,
 }
 
-#[derive(Deserialize, Serialize, Debug, Default, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Deserialize, Serialize, Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub(crate) enum SandboxKind {
-    #[default]
-    Inherit,
     Disabled,
     Bubblewrap,
 }
@@ -150,8 +148,8 @@ pub(crate) struct PackageConfig {
     #[serde()]
     pub(crate) from: Option<FromConfig>,
 
-    #[serde()]
-    pub(crate) sandbox: Option<SandboxConfig>,
+    #[serde(default)]
+    pub(crate) sandbox: SandboxConfig,
 
     #[serde(default)]
     pub(crate) import: Option<Vec<String>>,
@@ -303,9 +301,7 @@ impl RawConfig {
 
 impl PackageConfig {
     fn make_paths_absolute(&mut self, workspace_root: Option<&Path>) -> Result<()> {
-        if let Some(sandbox_config) = self.sandbox.as_mut() {
-            sandbox_config.make_paths_absolute(workspace_root)?;
-        }
+        self.sandbox.make_paths_absolute(workspace_root)?;
         if let Some(sub_config) = self.build.as_mut() {
             sub_config.make_paths_absolute(workspace_root)?;
         }
@@ -553,13 +549,13 @@ mod tests {
         let sandbox_a = config
             .permissions
             .sandbox_config_for_package(&PermSel::for_build_script("a"));
-        assert_eq!(sandbox_a.kind, SandboxKind::Bubblewrap);
+        assert_eq!(sandbox_a.kind, Some(SandboxKind::Bubblewrap));
         assert_eq!(sandbox_a.extra_args, vec!["--extra1", "--extra2"]);
 
         let sandbox_b = config
             .permissions
             .sandbox_config_for_package(&PermSel::for_build_script("b"));
-        assert_eq!(sandbox_b.kind, SandboxKind::Disabled);
+        assert_eq!(sandbox_b.kind, Some(SandboxKind::Disabled));
     }
 
     #[test]

src/config/permissions.rs

@@ -2,7 +2,6 @@ use super::PackageConfig;
 use super::PackageName;
 use super::RawConfig;
 use super::SandboxConfig;
-use super::SandboxKind;
 use crate::crate_index::CrateIndex;
 use crate::crate_index::CrateKind;
 use crate::crate_index::CrateSel;
@@ -112,7 +111,7 @@ impl Permissions {
     pub(crate) fn sandbox_config_for_package(&self, perm_sel: &PermSel) -> SandboxConfig {
         self.packages
             .get(perm_sel)
-            .and_then(|c| c.sandbox.clone())
+            .map(|c| c.sandbox.clone())
             .unwrap_or_default()
     }
 
@@ -132,7 +131,7 @@ fn apply_inheritance(packages: &mut FxHashMap<PermSel, PackageConfig>, config: &
     // Determine a global config. We may eventually make this an actual thing in our configuration
     // file.
     let global_config = PackageConfig {
-        sandbox: Some(config.sandbox.clone()),
+        sandbox: config.sandbox.clone(),
         ..Default::default()
     };
 
@@ -186,19 +185,13 @@ impl PackageConfig {
         );
         self.allow_proc_macro |= other.allow_proc_macro;
         self.allow_unsafe |= other.allow_unsafe;
-        if let Some(other_sandbox) = other.sandbox.as_ref() {
-            if let Some(sandbox) = self.sandbox.as_mut() {
-                sandbox.inherit(other_sandbox);
-            } else {
-                self.sandbox = other.sandbox.clone();
-            }
-        }
+        self.sandbox.inherit(&other.sandbox);
     }
 }
 
 impl SandboxConfig {
     fn inherit(&mut self, other: &SandboxConfig) {
-        if self.kind == SandboxKind::Inherit {
+        if self.kind.is_none() {
             self.kind = other.kind;
         }
         merge_string_vec(&mut self.extra_args, &other.extra_args);

src/config_editor.rs

@@ -94,7 +94,7 @@ pub(crate) fn fixes_for_problem(problem: &Problem, config: &Config) -> Vec<Box<d
             }));
         }
         Problem::BuildScriptFailed(failure) => {
-            if failure.output.sandbox_config.kind != SandboxKind::Disabled {
+            if failure.output.sandbox_config.kind != Some(SandboxKind::Disabled) {
                 let perm_sel = PermSel::for_build_script(failure.crate_sel.pkg_name());
                 if !failure.output.sandbox_config.allow_network.unwrap_or(false) {
                     edits.push(Box::new(SandboxAllowNetwork {
@@ -261,7 +261,6 @@ impl ConfigEditor {
     pub(crate) fn set_sandbox_kind(&mut self, sandbox_kind: SandboxKind) -> Result<()> {
         crate::sandbox::verify_kind(sandbox_kind)?;
         let sandbox_kind = match sandbox_kind {
-            SandboxKind::Inherit => "Inherit",
             SandboxKind::Disabled => "Disabled",
             SandboxKind::Bubblewrap => "Bubblewrap",
         };
@@ -1303,7 +1302,7 @@ mod tests {
                 stderr: Vec::new(),
                 crate_sel: crate_sel.clone(),
                 sandbox_config: SandboxConfig {
-                    kind: crate::config::SandboxKind::Bubblewrap,
+                    kind: Some(crate::config::SandboxKind::Bubblewrap),
                     extra_args: vec![],
                     allow_network: None,
                     bind_writable: vec![],

src/sandbox.rs

@@ -62,8 +62,8 @@ pub(crate) fn from_config(
     perm_sel: &PermSel,
 ) -> Result<Option<Box<dyn Sandbox>>> {
     let mut sandbox = match &config.kind {
-        SandboxKind::Disabled | SandboxKind::Inherit => return Ok(None),
-        SandboxKind::Bubblewrap => Box::<bubblewrap::Bubblewrap>::default(),
+        None | Some(SandboxKind::Disabled) => return Ok(None),
+        Some(SandboxKind::Bubblewrap) => Box::<bubblewrap::Bubblewrap>::default(),
     };
     let home = PathBuf::from(std::env::var("HOME").context("Couldn't get HOME env var")?);
     // We allow access to the root of the filesystem, but only selected parts of the user's home