Disable annotations when using Legacy Encoding (#1035)

* Stops spk adding annotations when spfs is not using the flatbuffers encoding format.

When flatbuffers encoding is enabled, spk will add an annotation to the spfs runtimes it make.

This separate the digest and encoding usage of legacy_encode methods into two methods. Add digest_encode method to spfs graph objects. Change Layer's legacy_encode method to error if the Layer has annotation date. But its digest_encode method remains the same. size_for_legacy_encoding methods also remain unchanged.

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>

crates/spfs/src/graph/entry.rs

@@ -180,6 +180,15 @@ impl<'buf> encoding::Digestible for Entry<'buf> {
 }
 
 impl<'buf> Entry<'buf> {
+    pub(super) fn digest_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
+        encoding::write_digest(&mut writer, self.object())?;
+        self.kind().encode(&mut writer)?;
+        encoding::write_uint64(&mut writer, self.mode() as u64)?;
+        encoding::write_uint64(&mut writer, self.size_for_legacy_encode())?;
+        encoding::write_string(writer, self.name())?;
+        Ok(())
+    }
+
     pub(super) fn legacy_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
         encoding::write_digest(&mut writer, self.object())?;
         self.kind().encode(&mut writer)?;

crates/spfs/src/graph/layer.rs

@@ -119,7 +119,8 @@ impl Layer {
         children
     }
 
-    pub(super) fn legacy_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
+    pub(super) fn digest_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
+        // Includes any annotations regardless of the EncodingFormat setting
         let annotations = self.annotations();
         let result = if let Some(manifest_digest) = self.manifest() {
             let manifest_result =
@@ -144,6 +145,24 @@ impl Layer {
 
         result
     }
+
+    pub(super) fn legacy_encode(&self, writer: &mut impl std::io::Write) -> Result<()> {
+        // Legacy encoded layers do not support writing annotations
+        if !self.annotations().is_empty() {
+            return Err(Error::String(
+                "Invalid Layer object for legacy encoding, it has annotation data. Annotations are not supported with legacy encoding".to_string(),
+            ));
+        }
+        let result = if let Some(manifest_digest) = self.manifest() {
+            encoding::write_digest(writer, manifest_digest).map_err(Error::Encoding)
+        } else {
+            Err(Error::String(
+                "Invalid Layer object for legacy encoding, it has no manifest data".to_string(),
+            ))
+        };
+
+        result
+    }
 }
 
 impl std::hash::Hash for Layer {

crates/spfs/src/graph/manifest.rs

@@ -119,6 +119,19 @@ impl Manifest {
         manifest
     }
 
+    pub(super) fn digest_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
+        self.root().digest_encode(&mut writer)?;
+        // this method encodes the root tree first, and does not
+        // include it in the count of remaining trees since at least
+        // one root is always required
+        encoding::write_uint64(&mut writer, self.proto().trees().len() as u64 - 1)?;
+        // skip the root tree when saving the rest
+        for tree in self.iter_trees().skip(1) {
+            tree.digest_encode(writer)?;
+        }
+        Ok(())
+    }
+
     pub(super) fn legacy_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
         self.root().legacy_encode(&mut writer)?;
         // this method encodes the root tree first, and does not

crates/spfs/src/graph/object.rs

@@ -219,9 +219,9 @@ impl<T: ObjectProto> encoding::Digestible for FlatObject<T> {
             }
         }
         match variant {
-            Enum::Platform(obj) => obj.legacy_encode(&mut hasher)?,
-            Enum::Layer(obj) => obj.legacy_encode(&mut hasher)?,
-            Enum::Manifest(obj) => obj.legacy_encode(&mut hasher)?,
+            Enum::Platform(obj) => obj.digest_encode(&mut hasher)?,
+            Enum::Layer(obj) => obj.digest_encode(&mut hasher)?,
+            Enum::Manifest(obj) => obj.digest_encode(&mut hasher)?,
             Enum::Blob(_obj) => unreachable!("handled above"),
         }
         Ok(hasher.digest())

crates/spfs/src/graph/platform.rs

@@ -55,6 +55,19 @@ impl Platform {
         self.iter_bottom_up().copied().collect()
     }
 
+    pub(super) fn digest_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
+        // use a vec to know the name ahead of time and
+        // avoid iterating the stack twice
+        let digests = self.iter_bottom_up().collect::<Vec<_>>();
+        encoding::write_uint64(&mut writer, digests.len() as u64)?;
+        // for historical reasons, and to remain backward-compatible, platform
+        // stacks are stored in reverse (top-down) order
+        for digest in digests.into_iter().rev() {
+            encoding::write_digest(&mut writer, digest)?;
+        }
+        Ok(())
+    }
+
     pub(super) fn legacy_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
         // use a vec to know the name ahead of time and
         // avoid iterating the stack twice

crates/spfs/src/graph/tree.rs

@@ -37,6 +37,18 @@ impl<'buf> Tree<'buf> {
         self.entries().find(|entry| entry.name() == name.as_ref())
     }
 
+    pub(super) fn digest_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
+        let mut entries: Vec<_> = self.entries().collect();
+        encoding::write_uint64(&mut writer, entries.len() as u64)?;
+        // this is not the default sort mode for entries but
+        // matches the existing compatible encoding order
+        entries.sort_unstable_by_key(|e| e.name());
+        for entry in entries.into_iter() {
+            entry.digest_encode(writer)?;
+        }
+        Ok(())
+    }
+
     pub(super) fn legacy_encode(&self, mut writer: &mut impl std::io::Write) -> Result<()> {
         let mut entries: Vec<_> = self.entries().collect();
         encoding::write_uint64(&mut writer, entries.len() as u64)?;

crates/spk-exec/src/exec.rs

@@ -9,6 +9,7 @@ use async_stream::try_stream;
 use futures::{Stream, StreamExt};
 use relative_path::RelativePathBuf;
 use spfs::encoding::Digest;
+use spfs::graph::object::EncodingFormat;
 use spfs::prelude::*;
 use spfs::tracking::{Entry, EntryKind};
 use spk_schema::foundation::format::{FormatIdent, FormatOptionMap};
@@ -298,18 +299,21 @@ pub async fn setup_runtime(rt: &mut spfs::runtime::Runtime, solution: &Solution)
         resolve_runtime_layers(rt.config.mount_backend.requires_localization(), solution).await?;
     rt.status.stack = spfs::graph::Stack::from_iter(stack);
 
-    // Store additional solve data all the resolved packages as extra
-    // data in the spfs runtime so future spk commands run inside the
-    // runtime can access it.
-    let solve_data = serde_json::to_string(&solution.packages_to_solve_data())
-        .map_err(|err| Error::String(err.to_string()))?;
     let spfs_config = spfs::Config::current()?;
-    rt.add_annotation(
-        SPK_SOLVE_EXTRA_DATA_KEY.to_string(),
-        solve_data,
-        spfs_config.filesystem.annotation_size_limit,
-    )
-    .await?;
+    // Annotations are only supported with FlatFileBuffers
+    if spfs_config.storage.encoding_format == EncodingFormat::FlatBuffers {
+        // Store additional solve data all the resolved packages as extra
+        // data in the spfs runtime so future spk commands run inside the
+        // runtime can access it.
+        let solve_data = serde_json::to_string(&solution.packages_to_solve_data())
+            .map_err(|err| Error::String(err.to_string()))?;
+        rt.add_annotation(
+            SPK_SOLVE_EXTRA_DATA_KEY.to_string(),
+            solve_data,
+            spfs_config.filesystem.annotation_size_limit,
+        )
+        .await?;
+    }
 
     rt.save_state_to_storage().await?;
     spfs::remount_runtime(rt).await?;