[guppy-summaries] make metadata a dynamic toml value

This means that the same library can parse summaries created by both old
and new versions of guppy. We don't use the summary for much anyway,
currently.

I also looked at switching to `toml_edit`, which has a much better
design overall. Unfortunately, I ran into
toml-rs/toml#192. It makes sense to switch
to `toml_edit` at some point in the future, and I've filed facebookarchive#492 to keep
track of that.

guppy-summaries/README.md

@@ -13,15 +13,12 @@ and allow for tracking results of builds over time.
 ## Examples
 
 ```rust
-use guppy_summaries::{SummaryWithMetadata, SummaryId, SummarySource, PackageStatus};
+use guppy_summaries::{Summary, SummaryId, SummarySource, PackageStatus};
 use pretty_assertions::assert_eq;
 use semver::Version;
 use std::collections::BTreeSet;
 use toml::Value;
 
-// Type alias for use in this example.
-type Summary = SummaryWithMetadata<Value>;
-
 // A summary is a TOML file that has this format:
 static SUMMARY: &str = r#"
 [[target-package]]

guppy-summaries/src/diff.rs

@@ -9,9 +9,7 @@
 //! summaries or through `SummaryDiff::new`.
 
 pub use crate::report::SummaryReport;
-use crate::{
-    PackageInfo, PackageMap, PackageStatus, SummaryId, SummarySource, SummaryWithMetadata,
-};
+use crate::{PackageInfo, PackageMap, PackageStatus, Summary, SummaryId, SummarySource};
 use diffus::{edit, Diffable};
 use semver::Version;
 use serde::{ser::SerializeStruct, Serialize};
@@ -82,7 +80,7 @@ pub struct SummaryDiff<'a> {
 
 impl<'a> SummaryDiff<'a> {
     /// Computes a diff between two summaries.
-    pub fn new<M1, M2>(old: &'a SummaryWithMetadata<M1>, new: &'a SummaryWithMetadata<M2>) -> Self {
+    pub fn new(old: &'a Summary, new: &'a Summary) -> Self {
         Self {
             target_packages: PackageDiff::new(&old.target_packages, &new.target_packages),
             host_packages: PackageDiff::new(&old.host_packages, &new.host_packages),

guppy-summaries/src/lib.rs

@@ -12,15 +12,12 @@
 //! # Examples
 //!
 //! ```rust
-//! use guppy_summaries::{SummaryWithMetadata, SummaryId, SummarySource, PackageStatus};
+//! use guppy_summaries::{Summary, SummaryId, SummarySource, PackageStatus};
 //! use pretty_assertions::assert_eq;
 //! use semver::Version;
 //! use std::collections::BTreeSet;
 //! use toml::Value;
 //!
-//! // Type alias for use in this example.
-//! type Summary = SummaryWithMetadata<Value>;
-//!
 //! // A summary is a TOML file that has this format:
 //! static SUMMARY: &str = r#"
 //! [[target-package]]

guppy-summaries/src/summary.rs

@@ -9,7 +9,7 @@ use std::{
     collections::{BTreeMap, BTreeSet},
     fmt,
 };
-use toml::{Serializer, Value};
+use toml::{value::Table, Serializer};
 
 /// A type representing a package map as used in `Summary` instances.
 pub type PackageMap = BTreeMap<SummaryId, PackageInfo>;
@@ -19,16 +19,16 @@ pub type PackageMap = BTreeMap<SummaryId, PackageInfo>;
 /// The metadata parameter is customizable.
 ///
 /// For more, see the crate-level documentation.
-#[derive(Clone, Debug, Deserialize, Eq, Hash, Serialize, PartialEq)]
+#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq)]
 #[serde(rename_all = "kebab-case")]
-pub struct SummaryWithMetadata<M = Value> {
+pub struct Summary {
     /// Extra metadata associated with the summary.
     ///
     /// This may be used for storing extra information about the summary.
     ///
     /// The type defaults to `toml::Value` but is customizable.
-    #[serde(default = "Option::default")]
-    pub metadata: Option<M>,
+    #[serde(default, skip_serializing_if = "Table::is_empty")]
+    pub metadata: Table,
 
     /// The packages and features built on the target platform.
     #[serde(
@@ -49,53 +49,46 @@ pub struct SummaryWithMetadata<M = Value> {
     pub host_packages: PackageMap,
 }
 
-impl<M> SummaryWithMetadata<M> {
+impl Summary {
+    /// Constructs a new summary with the provided metadata, and an empty `target_packages` and
+    /// `host_packages`.
+    pub fn with_metadata(metadata: &impl Serialize) -> Result<Self, toml::ser::Error> {
+        let toml_str = toml::to_string(metadata)?;
+        let metadata =
+            toml::from_str(&toml_str).expect("toml::to_string creates a valid TOML string");
+        Ok(Self {
+            metadata,
+            ..Self::default()
+        })
+    }
+
     /// Deserializes a summary from the given string, with optional custom metadata.
-    pub fn parse<'de>(s: &'de str) -> Result<Self, toml::de::Error>
-    where
-        M: Deserialize<'de>,
-    {
+    pub fn parse(s: &str) -> Result<Self, toml::de::Error> {
         toml::from_str(s)
     }
 
     /// Perform a diff of this summary against another.
     ///
     /// This doesn't diff the metadata, just the initials and packages.
-    pub fn diff<'a, M2>(&'a self, other: &'a SummaryWithMetadata<M2>) -> SummaryDiff<'a> {
+    pub fn diff<'a>(&'a self, other: &'a Summary) -> SummaryDiff<'a> {
         SummaryDiff::new(self, other)
     }
 
     /// Serializes this summary to a TOML string.
-    pub fn to_string(&self) -> Result<String, toml::ser::Error>
-    where
-        M: Serialize,
-    {
+    pub fn to_string(&self) -> Result<String, toml::ser::Error> {
         let mut dst = String::new();
         self.write_to_string(&mut dst)?;
         Ok(dst)
     }
 
     /// Serializes this summary into the given TOML string, using pretty TOML syntax.
-    pub fn write_to_string(&self, dst: &mut String) -> Result<(), toml::ser::Error>
-    where
-        M: Serialize,
-    {
+    pub fn write_to_string(&self, dst: &mut String) -> Result<(), toml::ser::Error> {
         let mut serializer = Serializer::pretty(dst);
         serializer.pretty_array(false);
         self.serialize(&mut serializer)
     }
 }
 
-impl<M> Default for SummaryWithMetadata<M> {
-    fn default() -> Self {
-        Self {
-            metadata: None,
-            target_packages: PackageMap::new(),
-            host_packages: PackageMap::new(),
-        }
-    }
-}
-
 /// A unique identifier for a package in a build summary.
 #[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, Serialize, PartialEq, PartialOrd)]
 #[serde(rename_all = "kebab-case")]

guppy-summaries/src/unit_tests/basic_tests.rs

@@ -2,15 +2,13 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use crate::{
-    diff::SummaryDiffStatus, PackageInfo, PackageMap, PackageStatus, SummaryId, SummarySource,
-    SummaryWithMetadata,
+    diff::SummaryDiffStatus, PackageInfo, PackageMap, PackageStatus, Summary, SummaryId,
+    SummarySource,
 };
 use pretty_assertions::assert_eq;
 use semver::Version;
 use std::collections::BTreeSet;
 
-type Summary = SummaryWithMetadata;
-
 static SERIALIZED_SUMMARY: &str = r#"# This is a test @generated summary.
 
 [[target-package]]
@@ -168,7 +166,7 @@ fn basic_roundtrip() {
     ];
 
     let summary = Summary {
-        metadata: None,
+        metadata: Default::default(),
         target_packages: make_summary(target_packages),
         host_packages: make_summary(host_packages),
     };

guppy/src/errors.rs

@@ -68,6 +68,9 @@ pub enum Error {
         /// The registry name that wasn't recognized.
         registry_name: String,
     },
+    /// An error occurred while serializing to TOML.
+    #[cfg(feature = "summaries")]
+    TomlSerializeError(toml::ser::Error),
 }
 
 impl Error {
@@ -140,6 +143,8 @@ impl fmt::Display for Error {
                     message, summary, registry_name
                 )
             }
+            #[cfg(feature = "summaries")]
+            TomlSerializeError(_) => write!(f, "failed to serialize to TOML"),
         }
     }
 }
@@ -164,6 +169,8 @@ impl error::Error for Error {
             UnknownPackageSetSummary { .. } => None,
             #[cfg(feature = "summaries")]
             UnknownRegistryName { .. } => None,
+            #[cfg(feature = "summaries")]
+            TomlSerializeError(err) => Some(err),
         }
     }
 }

guppy/src/graph/summaries.rs

@@ -21,9 +21,6 @@ pub use package_set::*;
 use serde::{Deserialize, Serialize};
 use std::collections::BTreeSet;
 
-/// A type alias for build summaries generated by `guppy`.
-pub type Summary = SummaryWithMetadata<CargoOptionsSummary>;
-
 impl<'g> CargoSet<'g> {
     /// Creates a build summary with the given options.
     ///
@@ -35,11 +32,12 @@ impl<'g> CargoSet<'g> {
         let target_features = self.target_features();
         let host_features = self.host_features();
 
-        Ok(Summary {
-            metadata: Some(metadata),
-            target_packages: target_features.to_package_map(initials, self.target_direct_deps()),
-            host_packages: host_features.to_package_map(initials, self.host_direct_deps()),
-        })
+        let mut summary = Summary::with_metadata(&metadata).map_err(Error::TomlSerializeError)?;
+        summary.target_packages =
+            target_features.to_package_map(initials, self.target_direct_deps());
+        summary.host_packages = host_features.to_package_map(initials, self.host_direct_deps());
+
+        Ok(summary)
     }
 }
 
@@ -320,17 +318,17 @@ mod tests {
     #[test]
     fn parse_old_metadata() {
         // Ensure that previous versions of the metadata parse correctly.
+        // TODO: note that there have been some compatibility breaks, particularly for
+        // omitted-packages. Probably don't need to retain too much backwards compatibility.
         let metadata = "\
-[metadata]
 version = 'v1'
 include-dev = true
 proc-macros-on-target = false
 ";
 
-        let parsed = Summary::parse(metadata).expect("parsed correctly");
-        let metadata = parsed.metadata.expect("metadata is present");
+        let summary: CargoOptionsSummary = toml::from_str(metadata).expect("parsed correctly");
         assert_eq!(
-            InitialsPlatform::from(metadata.initials_platform),
+            InitialsPlatform::from(summary.initials_platform),
             InitialsPlatform::Standard
         );
     }

workspace-hack/Cargo.toml

@@ -50,9 +50,9 @@ libc = { version = "0.2.103", features = ["extra_traits", "std"] }
 libc = { version = "0.2.103", features = ["extra_traits", "std"] }
 
 [target.x86_64-pc-windows-msvc.dependencies]
-winapi = { version = "0.3.9", default-features = false, features = ["basetsd", "consoleapi", "errhandlingapi", "fileapi", "handleapi", "ioapiset", "jobapi", "jobapi2", "libloaderapi", "lmcons", "memoryapi", "minschannel", "minwinbase", "minwindef", "namedpipeapi", "ntdef", "ntstatus", "processenv", "processthreadsapi", "profileapi", "psapi", "schannel", "securitybaseapi", "shellapi", "shlobj", "sspi", "std", "synchapi", "sysinfoapi", "timezoneapi", "winbase", "wincon", "wincrypt", "winerror", "winnt", "winsock2", "winuser", "ws2def", "ws2ipdef", "ws2tcpip"] }
+winapi = { version = "0.3.9", default-features = false, features = ["basetsd", "consoleapi", "errhandlingapi", "fileapi", "handleapi", "ioapiset", "jobapi", "jobapi2", "libloaderapi", "lmcons", "memoryapi", "minschannel", "minwinbase", "minwindef", "namedpipeapi", "ntdef", "ntstatus", "processenv", "processthreadsapi", "psapi", "schannel", "securitybaseapi", "shellapi", "shlobj", "sspi", "std", "synchapi", "sysinfoapi", "timezoneapi", "winbase", "wincon", "wincrypt", "winerror", "winnt", "winsock2", "winuser", "ws2def", "ws2ipdef", "ws2tcpip"] }
 
 [target.x86_64-pc-windows-msvc.build-dependencies]
-winapi = { version = "0.3.9", default-features = false, features = ["basetsd", "consoleapi", "errhandlingapi", "fileapi", "handleapi", "ioapiset", "jobapi", "jobapi2", "libloaderapi", "lmcons", "memoryapi", "minschannel", "minwinbase", "minwindef", "namedpipeapi", "ntdef", "ntstatus", "processenv", "processthreadsapi", "profileapi", "psapi", "schannel", "securitybaseapi", "shellapi", "shlobj", "sspi", "std", "synchapi", "sysinfoapi", "timezoneapi", "winbase", "wincon", "wincrypt", "winerror", "winnt", "winsock2", "winuser", "ws2def", "ws2ipdef", "ws2tcpip"] }
+winapi = { version = "0.3.9", default-features = false, features = ["basetsd", "consoleapi", "errhandlingapi", "fileapi", "handleapi", "ioapiset", "jobapi", "jobapi2", "libloaderapi", "lmcons", "memoryapi", "minschannel", "minwinbase", "minwindef", "namedpipeapi", "ntdef", "ntstatus", "processenv", "processthreadsapi", "psapi", "schannel", "securitybaseapi", "shellapi", "shlobj", "sspi", "std", "synchapi", "sysinfoapi", "timezoneapi", "winbase", "wincon", "wincrypt", "winerror", "winnt", "winsock2", "winuser", "ws2def", "ws2ipdef", "ws2tcpip"] }
 
 ### END HAKARI SECTION