Partly remove the notion of entry sizes for non-blobs

This kind of change is desired to eliminate a problem of `spfs diff`
reporting changes to directory sizes.

However, the size value associated with non-blob entries is still needed
in order to preserve the same on-disk representation and digest
calculated for manifests.

Signed-off-by: J Robert Ray <jrray@jrray.org>

crates/spfs-cli/main/src/cmd_ls.rs

@@ -166,7 +166,7 @@ impl TotalSize for Entry {
         if self.is_dir() {
             self.entries.values().map(|e| e.total_size()).sum()
         } else {
-            self.size
+            self.size()
         }
     }
 }

crates/spfs-proto/schema/spfs.fbs

@@ -54,6 +54,7 @@ table Entry {
     kind:EntryKind;
     object:Digest (required);
     mode:uint32;
+    // Size should only be present for blob entries
     size:uint64;
     name:string (required);
 }

crates/spfs-vfs/src/fuse.rs

@@ -115,9 +115,9 @@ impl Filesystem {
             kind,
             object,
             mode,
-            size,
             entries,
             user_data: _,
+            legacy_size,
         } = entry;
 
         let inode = self.allocate_inode();
@@ -129,9 +129,9 @@ impl Filesystem {
             kind,
             object,
             mode,
-            size,
             entries,
             user_data: inode,
+            legacy_size,
         });
         self.inodes.insert(inode, Arc::clone(&entry));
         entry
@@ -159,15 +159,15 @@ impl Filesystem {
 
     fn attr_from_entry(&self, entry: &Entry<u64>) -> FileAttr {
         let kind = match entry.kind {
-            EntryKind::Blob if entry.is_symlink() => FileType::Symlink,
-            EntryKind::Blob => FileType::RegularFile,
+            EntryKind::Blob(_) if entry.is_symlink() => FileType::Symlink,
+            EntryKind::Blob(_) => FileType::RegularFile,
             EntryKind::Tree => FileType::Directory,
             EntryKind::Mask => unreachable!(),
         };
         let size = if entry.is_dir() {
             entry.entries.len() as u64
         } else {
-            entry.size
+            entry.size()
         };
         FileAttr {
             ino: entry.user_data,
@@ -222,7 +222,7 @@ impl Filesystem {
         let blocks = self
             .inodes
             .iter()
-            .map(|i| (i.value().size / Self::BLOCK_SIZE as u64) + 1)
+            .map(|i| (i.value().size() / Self::BLOCK_SIZE as u64) + 1)
             .sum();
         let files = self
             .inodes
@@ -255,7 +255,7 @@ impl Filesystem {
         tracing::trace!("lookup {name} in {}", parent.key());
 
         match parent.kind {
-            EntryKind::Blob => {
+            EntryKind::Blob(_) => {
                 reply.error(libc::ENOTDIR);
                 return;
             }
@@ -349,7 +349,7 @@ impl Filesystem {
                 reply.error(libc::ENOENT);
                 return;
             }
-            EntryKind::Blob => &entry.object,
+            EntryKind::Blob(_) => &entry.object,
         };
 
         let mut handle = None;
@@ -503,7 +503,7 @@ impl Filesystem {
         };
 
         let handle = match entry.kind {
-            EntryKind::Blob => {
+            EntryKind::Blob(_) => {
                 reply.error(libc::ENOTDIR);
                 return;
             }
@@ -548,8 +548,8 @@ impl Filesystem {
         }
         for (name, entry) in remaining {
             let kind = match entry.kind {
-                EntryKind::Blob if entry.is_symlink() => FileType::Symlink,
-                EntryKind::Blob => FileType::RegularFile,
+                EntryKind::Blob(_) if entry.is_symlink() => FileType::Symlink,
+                EntryKind::Blob(_) => FileType::RegularFile,
                 EntryKind::Tree => FileType::Directory,
                 EntryKind::Mask => continue,
             };

crates/spfs-vfs/src/winfsp/mount.rs

@@ -128,9 +128,9 @@ impl Mount {
             kind,
             object,
             mode,
-            size,
             entries,
             user_data: _,
+            legacy_size,
         } = entry;
 
         let inode = self.allocate_inode();
@@ -142,18 +142,18 @@ impl Mount {
             kind,
             object,
             mode,
-            size,
             entries,
             user_data: inode,
+            legacy_size,
         });
         self.inodes.insert(inode, Arc::clone(&entry));
         entry
     }
 
     fn attr_from_entry(&self, entry: &Entry<u64>) -> u32 {
         let mut attrs = match entry.kind {
-            EntryKind::Blob if entry.is_symlink() => FILE_ATTRIBUTE_REPARSE_POINT.0,
-            EntryKind::Blob => FILE_ATTRIBUTE_NORMAL.0,
+            EntryKind::Blob(_) if entry.is_symlink() => FILE_ATTRIBUTE_REPARSE_POINT.0,
+            EntryKind::Blob(_) => FILE_ATTRIBUTE_NORMAL.0,
             EntryKind::Tree => FILE_ATTRIBUTE_DIRECTORY.0,
             // we do not allocate nodes for mask files
             EntryKind::Mask => unreachable!(),
@@ -270,7 +270,7 @@ impl winfsp::filesystem::FileSystemContext for Mount {
         let info = file_info.as_mut();
         info.file_attributes = attributes;
         info.index_number = entry.user_data;
-        info.file_size = entry.size;
+        info.file_size = entry.size();
         info.ea_size = 0;
         info.creation_time = now;
         info.change_time = now;
@@ -365,7 +365,7 @@ impl winfsp::filesystem::FileSystemContext for Mount {
                 let attributes = self.attr_from_entry(entry);
                 info.file_attributes = attributes;
                 info.index_number = entry.user_data;
-                info.file_size = entry.size;
+                info.file_size = entry.size();
                 info.ea_size = 0;
                 info.creation_time = now;
                 info.change_time = now;

crates/spfs/src/commit.rs

@@ -395,16 +395,21 @@ impl tracking::ComputeManifestReporter for ConsoleCommitReporter {
         bars.entries.inc(1);
         if entry.kind.is_blob() {
             bars.blobs.inc_length(1);
-            bars.bytes.inc_length(entry.size);
+            bars.bytes.inc_length(entry.size());
         }
     }
 }
 
 impl CommitReporter for ConsoleCommitReporter {
     fn committed_blob(&self, result: &CommitBlobResult) {
         let bars = self.get_bars();
-        bars.bytes.inc(result.node().entry.size);
-        bars.blobs.inc(1);
+        if result.node().entry.kind.is_blob() {
+            bars.bytes.inc(result.node().entry.size());
+            bars.blobs.inc(1);
+        } else {
+            debug_assert!(false, "committed_blob called with non-blob entry");
+            bars.blobs.inc(1);
+        }
     }
 }
 

crates/spfs/src/graph/entry.rs

@@ -70,7 +70,7 @@ impl<'buf> Entry<'buf> {
             name,
             entry.kind,
             entry.mode,
-            entry.size,
+            entry.size_for_legacy_encode(),
             &entry.object,
         )
     }
@@ -82,7 +82,7 @@ impl<'buf> Entry<'buf> {
 
     pub fn kind(&self) -> tracking::EntryKind {
         match self.0.kind() {
-            spfs_proto::EntryKind::Blob => tracking::EntryKind::Blob,
+            spfs_proto::EntryKind::Blob => tracking::EntryKind::Blob(self.0.size_()),
             spfs_proto::EntryKind::Tree => tracking::EntryKind::Tree,
             spfs_proto::EntryKind::Mask => tracking::EntryKind::Mask,
             _ => unreachable!("internally valid entry buffer"),
@@ -96,7 +96,10 @@ impl<'buf> Entry<'buf> {
 
     #[inline]
     pub fn size(&self) -> u64 {
-        self.0.size_()
+        match self.0.kind() {
+            spfs_proto::EntryKind::Blob => self.0.size_(),
+            _ => 0,
+        }
     }
 
     #[inline]
@@ -118,6 +121,15 @@ impl<'buf> Entry<'buf> {
     pub fn is_regular_file(&self) -> bool {
         unix_mode::is_file(self.mode())
     }
+
+    /// Return the size of the blob or the legacy size for non-blobs.
+    ///
+    /// This is required to preserve backwards compatibility with how digests
+    /// are calculated for non-blob entries.
+    #[inline]
+    pub fn size_for_legacy_encode(&self) -> u64 {
+        self.0.size_()
+    }
 }
 
 impl<'buf> std::fmt::Display for Entry<'buf> {
@@ -148,7 +160,8 @@ impl<'buf1, 'buf2> PartialOrd<Entry<'buf2>> for Entry<'buf1> {
 
 impl<'buf> Ord for Entry<'buf> {
     fn cmp(&self, other: &Entry<'buf>) -> std::cmp::Ordering {
-        if self.kind() == other.kind() {
+        // Note that the Entry's size does not factor into the comparison.
+        if self.0.kind() == other.0.kind() {
             self.name().cmp(other.name())
         } else {
             self.kind().cmp(&other.kind())
@@ -171,7 +184,7 @@ impl<'buf> Entry<'buf> {
         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())?;
+        encoding::write_uint64(&mut writer, self.size_for_legacy_encode())?;
         encoding::write_string(writer, self.name())?;
         Ok(())
     }
@@ -182,10 +195,13 @@ impl<'buf> Entry<'buf> {
     ) -> Result<flatbuffers::WIPOffset<spfs_proto::Entry<'builder>>> {
         // fields in the same order as above
         let object = encoding::read_digest(&mut reader)?;
-        let kind = tracking::EntryKind::decode(&mut reader)?;
+        let mut kind = tracking::EntryKind::decode(&mut reader)?;
         let mode = encoding::read_uint64(&mut reader)? as u32;
         let size = encoding::read_uint64(&mut reader)?;
         let name = encoding::read_string(reader)?;
+        if kind.is_blob() {
+            kind = tracking::EntryKind::Blob(size);
+        }
         Ok(Self::build(builder, &name, kind, mode, size, &object))
     }
 }
@@ -201,6 +217,36 @@ pub struct EntryBuf(Box<[u8]>);
 #[cfg(test)]
 impl EntryBuf {
     pub fn build(
+        name: &str,
+        kind: tracking::EntryKind,
+        mode: u32,
+        object: &encoding::Digest,
+    ) -> Self {
+        crate::graph::BUILDER.with_borrow_mut(|builder| {
+            let name = builder.create_string(name);
+            let e = spfs_proto::Entry::create(
+                builder,
+                &EntryArgs {
+                    kind: kind.into(),
+                    object: Some(object),
+                    mode,
+                    size_: {
+                        match kind {
+                            tracking::EntryKind::Blob(size) => size,
+                            _ => 0,
+                        }
+                    },
+                    name: Some(name),
+                },
+            );
+            builder.finish_minimal(e);
+            let bytes = builder.finished_data().into();
+            builder.reset();
+            Self(bytes)
+        })
+    }
+
+    pub fn build_with_legacy_size(
         name: &str,
         kind: tracking::EntryKind,
         mode: u32,
@@ -215,7 +261,12 @@ impl EntryBuf {
                     kind: kind.into(),
                     object: Some(object),
                     mode,
-                    size_: size,
+                    size_: {
+                        match kind {
+                            tracking::EntryKind::Blob(size) => size,
+                            _ => size,
+                        }
+                    },
                     name: Some(name),
                 },
             );

crates/spfs/src/graph/entry_test.rs

@@ -12,7 +12,7 @@ use crate::tracking::EntryKind;
 
 #[rstest(entry, digest,
     case(
-        EntryBuf::build(
+        EntryBuf::build_with_legacy_size(
             "testcase",
             EntryKind::Tree,
             0o40755,
@@ -24,9 +24,8 @@ use crate::tracking::EntryKind;
     case(
         EntryBuf::build(
             "swig_full_names.xsl",
-            EntryKind::Blob,
+            EntryKind::Blob(3293),
             0o100644,
-            3293,
             &"ZD25L3AN5E3LTZ6MDQOIZUV6KRV5Y4SSXRE4YMYZJJ3PXCQ3FMQA====".parse().unwrap(),
         ),
         "GP7DYE22DYLH3I5MB33PW5Z3AZXZIBGOND7MX65KECBMHVMXBUHQ====".parse().unwrap(),

crates/spfs/src/graph/manifest.rs

@@ -92,10 +92,10 @@ impl Manifest {
                 let mut new_entry = tracking::Entry {
                     kind: entry.kind(),
                     mode: entry.mode(),
-                    size: entry.size(),
                     entries: Default::default(),
                     object: *entry.object(),
                     user_data: (),
+                    legacy_size: entry.size_for_legacy_encode(),
                 };
                 if entry.kind().is_tree() {
                     new_entry.object = encoding::NULL_DIGEST.into();
@@ -275,7 +275,7 @@ impl ManifestBuilder {
                         node.path.as_str(),
                         node.entry.kind,
                         node.entry.mode,
-                        node.entry.size,
+                        node.entry.size_for_legacy_encode(),
                         &sub_root_digest,
                     )
                 }