[antlir2][features] use same traits in individual feature plugins

Summary:
Now that all features are loadable `.so`s, we can get more safety by using the
same traits as defined in `antlir2_compile` and `antlir2_depgraph` instead of
the hackery required when these were standalone binaries talking over stdout/in.

Test Plan:
```
❯ buck2 test fbcode//antlir/antlir2/test_images/...
Buck UI: https://www.internalfb.com/buck2/13a7a8da-5a5a-4401-9e1d-a0efd093b036
Test UI: https://www.internalfb.com/intern/testinfra/testrun/9570149213315935
Network: Up: 63MiB  Down: 2.0GiB  (reSessionID-197a814a-7d63-4eb0-b14e-a71fd1dd1616)
Jobs completed: 1020610. Time elapsed: 6:01.6s.
Cache hits: 96%. Commands: 44012 (cached: 42152, remote: 85, local: 1775). Fallback: 23/1860
Tests finished: Pass 424. Fail 0. Fatal 0. Skip 1. Build failure 0
```

Reviewed By: sergeyfd

Differential Revision: D48046540

fbshipit-source-id: 3f6cf203c3548e08ab58c89762ebedf0f96f823c

antlir/antlir2/antlir2_compile/BUCK

@@ -16,6 +16,7 @@ rust_library(
         "tracing",
         "xattr",
         "//antlir/antlir2/antlir2_features:antlir2_features",
+        "//antlir/antlir2/antlir2_isolate:antlir2_isolate",
         "//antlir/antlir2/antlir2_users:antlir2_users",
         "//antlir/buck/buck_label:buck_label",
     ],

antlir/antlir2/antlir2_compile/src/lib.rs

@@ -47,6 +47,8 @@ pub enum Error {
     #[error(transparent)]
     Feature(#[from] antlir2_features::Error),
     #[error(transparent)]
+    Isolate(#[from] antlir2_isolate::Error),
+    #[error(transparent)]
     Other(#[from] anyhow::Error),
 }
 

antlir/antlir2/antlir2_compile/src/plan.rs

@@ -6,6 +6,7 @@
  */
 
 use std::collections::BTreeSet;
+use std::path::PathBuf;
 
 use serde::Deserialize;
 use serde::Serialize;
@@ -100,7 +101,7 @@ pub struct DnfTransaction {
 pub struct InstallFbpkg {
     pub name: String,
     pub tag: String,
-    pub dst: Option<String>,
+    pub dst: Option<PathBuf>,
     pub organize: bool,
     pub organize_alias: Option<String>,
 }

antlir/antlir2/features/antlir1_no_equivalent.rs

@@ -9,8 +9,7 @@ use antlir2_compile::CompilerContext;
 use antlir2_depgraph::item::Item;
 use antlir2_depgraph::requires_provides::Requirement;
 use antlir2_features as _;
-use anyhow::bail;
-use anyhow::Result;
+use anyhow as _;
 use serde::Deserialize;
 use serde::Serialize;
 use tracing as _;
@@ -20,16 +19,18 @@ pub type Feature = Antlir1NoEquivalent;
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)]
 pub struct Antlir1NoEquivalent();
 
-impl<'f> antlir2_feature_impl::Feature<'f> for Antlir1NoEquivalent {
-    fn provides(&self) -> Result<Vec<Item<'f>>> {
-        bail!("Antlir1NoEquivalent")
+impl antlir2_depgraph::requires_provides::RequiresProvides for Antlir1NoEquivalent {
+    fn provides(&self) -> Result<Vec<Item<'static>>, String> {
+        unimplemented!("Antlir1NoEquivalent")
     }
 
-    fn requires(&self) -> Result<Vec<Requirement<'f>>> {
-        bail!("Antlir1NoEquivalent")
+    fn requires(&self) -> Result<Vec<Requirement<'static>>, String> {
+        unimplemented!("Antlir1NoEquivalent")
     }
+}
 
-    fn compile(&self, _ctx: &CompilerContext) -> Result<()> {
-        bail!("Antlir1NoEquivalent")
+impl antlir2_compile::CompileFeature for Antlir1NoEquivalent {
+    fn compile(&self, _ctx: &CompilerContext) -> antlir2_compile::Result<()> {
+        unimplemented!("Antlir1NoEquivalent")
     }
 }

antlir/antlir2/features/clone.rs

@@ -51,8 +51,8 @@ pub struct CloneUserGroup {
     pub group: GroupName,
 }
 
-impl<'f> antlir2_feature_impl::Feature<'f> for Clone {
-    fn requires(&self) -> Result<Vec<Requirement<'f>>> {
+impl antlir2_depgraph::requires_provides::RequiresProvides for Clone {
+    fn requires(&self) -> Result<Vec<Requirement<'static>>, String> {
         let mut v = vec![Requirement::ordered(
             ItemKey::Layer(self.src_layer.label.to_owned()),
             Validator::ItemInLayer {
@@ -146,12 +146,14 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Clone {
         Ok(v)
     }
 
-    fn provides(&self) -> Result<Vec<Item<'f>>> {
+    fn provides(&self) -> Result<Vec<Item<'static>>, String> {
         let src_layer_depgraph_path: &Path = self.src_layer.depgraph.as_ref();
-        let src_layer =
-            std::fs::read(src_layer_depgraph_path).context("while reading src_layer depgraph")?;
-        let src_depgraph: Graph<'_> =
-            serde_json::from_slice(&src_layer).context("while parsing src_layer depgraph")?;
+        let src_layer = std::fs::read(src_layer_depgraph_path)
+            .context("while reading src_layer depgraph")
+            .map_err(|e| e.to_string())?;
+        let src_depgraph: Graph<'_> = serde_json::from_slice(&src_layer)
+            .context("while parsing src_layer depgraph")
+            .map_err(|e| e.to_string())?;
         let mut v = Vec::new();
         // if this is creating the top-level dest, we need to produce that now
         if !self.pre_existing_dest {
@@ -207,9 +209,11 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Clone {
 
         Ok(v)
     }
+}
 
+impl antlir2_compile::CompileFeature for Clone {
     #[tracing::instrument(name = "clone", skip(ctx), ret, err)]
-    fn compile(&self, ctx: &CompilerContext) -> Result<()> {
+    fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> {
         // antlir2_depgraph has already done all the safety validation, so we
         // can just go ahead and blindly copy everything here
         let src_root = self

antlir/antlir2/features/defs.bzl

@@ -81,7 +81,6 @@ def feature_impl(
             "//antlir/antlir2/antlir2_compile:antlir2_compile",
             "//antlir/antlir2/antlir2_depgraph:antlir2_depgraph",
             "//antlir/antlir2/antlir2_features:antlir2_features",
-            "//antlir/antlir2/features/antlir2_feature_impl:antlir2_feature_impl",
         ],
     )
     rust_library(
@@ -104,14 +103,17 @@ def feature_impl(
         ],
         visibility = [":" + name],
         deps = [
+            "//antlir/antlir2/antlir2_features:antlir2_features",
+            "//antlir/antlir2/antlir2_depgraph:antlir2_depgraph",
+            "//antlir/antlir2/antlir2_compile:antlir2_compile",
+            "static_assertions",
             "anyhow",
             "serde_json",
             "tracing",
             "tracing-core",
             "//antlir/antlir2/antlir2_compile:antlir2_compile",
             "//antlir/antlir2/antlir2_depgraph:antlir2_depgraph",
             "//antlir/antlir2/antlir2_features:antlir2_features",
-            "//antlir/antlir2/features/antlir2_feature_impl:antlir2_feature_impl",
         ],
     )
 

antlir/antlir2/features/dot_meta.rs

@@ -30,17 +30,19 @@ struct BuildInfo {
     package: Option<String>,
 }
 
-impl<'f> antlir2_feature_impl::Feature<'f> for DotMeta {
-    fn provides(&self) -> Result<Vec<Item<'f>>> {
+impl antlir2_depgraph::requires_provides::RequiresProvides for DotMeta {
+    fn provides(&self) -> Result<Vec<Item<'static>>, String> {
         Ok(Default::default())
     }
 
-    fn requires(&self) -> Result<Vec<Requirement<'f>>> {
+    fn requires(&self) -> Result<Vec<Requirement<'static>>, String> {
         Ok(Default::default())
     }
+}
 
+impl antlir2_compile::CompileFeature for DotMeta {
     #[tracing::instrument(name = "dot_meta", skip(ctx), ret, err)]
-    fn compile(&self, ctx: &CompilerContext) -> Result<()> {
+    fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> {
         std::fs::create_dir_all(ctx.dst_path("/.meta")).context("while creating /.meta")?;
         std::fs::write(
             ctx.dst_path("/.meta/target"),

antlir/antlir2/features/ensure_dir_exists.rs

@@ -35,16 +35,16 @@ pub struct EnsureDirExists {
     pub user: UserName,
 }
 
-impl<'f> antlir2_feature_impl::Feature<'f> for EnsureDirExists {
-    fn provides(&self) -> Result<Vec<Item<'f>>> {
+impl antlir2_depgraph::requires_provides::RequiresProvides for EnsureDirExists {
+    fn provides(&self) -> Result<Vec<Item<'static>>, String> {
         Ok(vec![Item::Path(Path::Entry(FsEntry {
             path: self.dir.clone().into(),
             file_type: FileType::Directory,
             mode: self.mode.0,
         }))])
     }
 
-    fn requires(&self) -> Result<Vec<Requirement<'f>>> {
+    fn requires(&self) -> Result<Vec<Requirement<'static>>, String> {
         let mut v = vec![
             Requirement::ordered(ItemKey::User(self.user.clone().into()), Validator::Exists),
             Requirement::ordered(ItemKey::Group(self.group.clone().into()), Validator::Exists),
@@ -57,9 +57,11 @@ impl<'f> antlir2_feature_impl::Feature<'f> for EnsureDirExists {
         }
         Ok(v)
     }
+}
 
+impl antlir2_compile::CompileFeature for EnsureDirExists {
     #[tracing::instrument(name = "ensure_dir_exists", skip(ctx), ret, err)]
-    fn compile(&self, ctx: &CompilerContext) -> Result<()> {
+    fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> {
         let dst = ctx.dst_path(&self.dir);
         tracing::trace!("creating {}", dst.display());
         match std::fs::create_dir(&dst) {

antlir/antlir2/features/extract.rs

@@ -28,7 +28,6 @@ use antlir2_features::types::LayerInfo;
 use antlir2_features::types::PathInLayer;
 use antlir2_isolate::isolate;
 use antlir2_isolate::IsolationContext;
-use anyhow::ensure;
 use anyhow::Context;
 use anyhow::Result;
 use goblin::elf::Elf;
@@ -86,8 +85,8 @@ pub struct ExtractLayerBinaries {
     pub binaries: Vec<PathInLayer>,
 }
 
-impl<'f> antlir2_feature_impl::Feature<'f> for Extract {
-    fn provides(&self) -> Result<Vec<Item<'f>>> {
+impl antlir2_depgraph::requires_provides::RequiresProvides for Extract {
+    fn provides(&self) -> Result<Vec<Item<'static>>, String> {
         // Intentionally provide only the direct files the user asked for,
         // because we don't want to produce conflicts with all the transitive
         // dependencies. However, we will check that any duplicated items are in
@@ -115,7 +114,7 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Extract {
         })
     }
 
-    fn requires(&self) -> Result<Vec<Requirement<'f>>> {
+    fn requires(&self) -> Result<Vec<Requirement<'static>>, String> {
         Ok(match self {
             Self::Layer(l) => l
                 .binaries
@@ -157,9 +156,11 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Extract {
             )],
         })
     }
+}
 
+impl antlir2_compile::CompileFeature for Extract {
     #[tracing::instrument(name = "extract", skip(ctx), ret, err)]
-    fn compile(&self, ctx: &CompilerContext) -> Result<()> {
+    fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> {
         let default_interpreter = Path::new(match ctx.target_arch() {
             Arch::X86_64 => "/usr/lib64/ld-linux-x86-64.so.2",
             Arch::Aarch64 => "/lib/ld-linux-aarch64.so.1",
@@ -233,7 +234,7 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Extract {
                                 canonical_target.display(),
                             );
                             Self::Buck(ExtractBuckBinary {
-                                src: canonical_target.to_owned().into(),
+                                src: canonical_target.clone(),
                                 dst: binary.to_owned(),
                             })
                             .compile(ctx)
@@ -254,12 +255,14 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Extract {
                                 .strip_prefix("/")
                                 .unwrap_or(canonical_target.as_path()),
                         );
-                        ensure!(
-                            target_under_src.exists(),
-                            "symlink target {} ({} under src_layer) does not actually exist",
-                            canonical_target.display(),
-                            target_under_src.display()
-                        );
+                        if !target_under_src.exists() {
+                            return Err(anyhow::anyhow!(
+                                "symlink target {} ({} under src_layer) does not actually exist",
+                                canonical_target.display(),
+                                target_under_src.display()
+                            )
+                            .into());
+                        }
 
                         copy_with_metadata(
                             &target_under_src,
@@ -316,7 +319,8 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Extract {
                             return Err(anyhow::anyhow!(
                                 "'{}' exists but it seems like we should get it from the host",
                                 path_in_src_layer.display()
-                            ));
+                            )
+                            .into());
                         }
                         dep.clone()
                     } else {
@@ -503,11 +507,12 @@ fn copy_dep(dep: &Path, dst: &Path) -> Result<()> {
             new_src_hash
         );
 
-        ensure!(
-            pre_existing_hash == new_src_hash,
-            "extract conflicts with existing file at {}",
-            dst.display()
-        );
+        if pre_existing_hash != new_src_hash {
+            return Err(anyhow::anyhow!(
+                "extract conflicts with existing file at {}",
+                dst.display()
+            ));
+        }
     } else {
         copy_with_metadata(&dep, dst, None, None)?;
     }

antlir/antlir2/features/genrule.rs

@@ -14,7 +14,6 @@ use antlir2_features::types::UserName;
 use antlir2_isolate::isolate;
 use antlir2_isolate::InvocationType;
 use antlir2_isolate::IsolationContext;
-use anyhow::ensure;
 use anyhow::Context;
 use anyhow::Result;
 use derivative::Derivative;
@@ -53,17 +52,19 @@ impl Genrule {
     }
 }
 
-impl<'f> antlir2_feature_impl::Feature<'f> for Genrule {
-    fn provides(&self) -> Result<Vec<Item<'f>>> {
+impl antlir2_depgraph::requires_provides::RequiresProvides for Genrule {
+    fn provides(&self) -> Result<Vec<Item<'static>>, String> {
         Ok(Default::default())
     }
 
-    fn requires(&self) -> Result<Vec<Requirement<'f>>> {
+    fn requires(&self) -> Result<Vec<Requirement<'static>>, String> {
         Ok(Default::default())
     }
+}
 
+impl antlir2_compile::CompileFeature for Genrule {
     #[tracing::instrument(name = "genrule", skip(ctx), ret, err)]
-    fn compile(&self, ctx: &CompilerContext) -> Result<()> {
+    fn compile(&self, ctx: &CompilerContext) -> antlir2_compile::Result<()> {
         if self.boot {
             unimplemented!("boot is not yet implemented");
         }
@@ -92,16 +93,18 @@ impl<'f> antlir2_feature_impl::Feature<'f> for Genrule {
         cmd.args(inner_cmd);
         tracing::trace!("executing genrule with isolated command: {cmd:?}");
         let res = cmd.output().context("while running cmd")?;
-        ensure!(
-            res.status.success(),
-            "genrule {self:?} {}. {}\n{}",
-            match res.status.code() {
-                Some(code) => format!("exited with code {code}"),
-                None => "was terminated by a signal".to_owned(),
-            },
-            std::str::from_utf8(&res.stdout).unwrap_or("<invalid utf8>"),
-            std::str::from_utf8(&res.stderr).unwrap_or("<invalid utf8>"),
-        );
+        if !res.status.success() {
+            return Err(anyhow::anyhow!(
+                "genrule {self:?} {}. {}\n{}",
+                match res.status.code() {
+                    Some(code) => format!("exited with code {code}"),
+                    None => "was terminated by a signal".to_owned(),
+                },
+                std::str::from_utf8(&res.stdout).unwrap_or("<invalid utf8>"),
+                std::str::from_utf8(&res.stderr).unwrap_or("<invalid utf8>"),
+            )
+            .into());
+        }
         Ok(())
     }
 }