Address feedback

src/constants.rs

@@ -505,11 +505,11 @@ bitflags! {
         const PREUNTRANS	= 1 << 20;	// %preuntrans dependency
         const POSTUNTRANS = 1 << 21;	// %postuntrans dependency
         // bits 22-23 unused
-        const RPMLIB = 1 << 24;	      // rpmlib(feature) dependency.
+        const RPMLIB = 1 << 24;	      // rpmlib(feature) dependency
         const TRIGGERPREIN = 1 << 25;  // %triggerprein dependency
         const KEYRING	= 1 << 26;
         // bit 27 unused
-        const CONFIG	= 1 << 28;    // config() dependency: TODO: wtf is a config dependency?
+        const CONFIG	= 1 << 28;    // config() dependency
         const META	= 1 << 29;	      // meta dependency
     }
 }
@@ -589,63 +589,55 @@ pub enum DigestAlgorithm {
 }
 
 /// Index tag values for the %prein scriptlet,
-///
 pub(crate) const PREIN_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_PREIN,
     IndexTag::RPMTAG_PREINFLAGS,
     IndexTag::RPMTAG_PREINPROG,
 );
 
 /// Index tag values for the %postin scriptlet,
-///
 pub(crate) const POSTIN_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_POSTIN,
     IndexTag::RPMTAG_POSTINFLAGS,
     IndexTag::RPMTAG_POSTINPROG,
 );
 
 /// Index tag values for the %preun scriptlet,
-///
 pub(crate) const PREUN_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_PREUN,
     IndexTag::RPMTAG_PREUNFLAGS,
     IndexTag::RPMTAG_PREUNPROG,
 );
 
 /// Index tag values for the %postun scriptlet,
-///
 pub(crate) const POSTUN_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_POSTUN,
     IndexTag::RPMTAG_POSTUNFLAGS,
     IndexTag::RPMTAG_POSTUNPROG,
 );
 
 /// Index tag values for the %pretrans scriptlet,
-///
 pub(crate) const PRETRANS_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_PRETRANS,
     IndexTag::RPMTAG_PRETRANSFLAGS,
     IndexTag::RPMTAG_PRETRANSPROG,
 );
 
 /// Index tag values for the %posttrans scriptlet,
-///
 pub(crate) const POSTTRANS_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_POSTTRANS,
     IndexTag::RPMTAG_POSTTRANSFLAGS,
     IndexTag::RPMTAG_POSTTRANSPROG,
 );
 
 /// Index tag values for the %preuntrans scriptlet,
-///
 pub(crate) const PREUNTRANS_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_PREUNTRANS,
     IndexTag::RPMTAG_PREUNTRANSFLAGS,
     IndexTag::RPMTAG_PREUNTRANSPROG,
 );
 
 /// Index tag values for the %postuntrans scriptlet,
-///
 pub(crate) const POSTUNTRANS_TAGS: ScriptletIndexTags = (
     IndexTag::RPMTAG_POSTUNTRANS,
     IndexTag::RPMTAG_POSTUNTRANSFLAGS,

src/rpm/builder.rs

@@ -667,7 +667,7 @@ impl PackageBuilder {
         let mut groups_to_create = HashSet::new();
 
         let mut combined_file_sizes: u64 = 0;
-        let mut has_caps = false;
+        let mut uses_file_capabilities = false;
 
         for (cpio_path, entry) in self.files.iter() {
             combined_file_sizes += entry.size;
@@ -678,7 +678,7 @@ impl PackageBuilder {
                 groups_to_create.insert(entry.group.clone());
             }
             if entry.caps.is_some() {
-                has_caps = true;
+                uses_file_capabilities = true;
             }
             file_sizes.push(entry.size);
             file_modes.push(entry.mode.into());
@@ -754,7 +754,7 @@ impl PackageBuilder {
             ));
         }
 
-        if has_caps {
+        if uses_file_capabilities {
             self.requires.push(Dependency::rpmlib(
                 "rpmlib(FileCaps)".to_owned(),
                 "4.6.1-1".to_owned(),

src/rpm/headers/types.rs

@@ -52,7 +52,6 @@ pub const REGULAR_FILE_TYPE: u16 = 0o100000; //  bit representation = "100000000
 pub const DIR_FILE_TYPE: u16 = 0o040000; //      bit representation = "0100000000000000"
 pub const SYMBOLIC_LINK_FILE_TYPE: u16 = 0o120000; // bit representation = "1010000000000000"
 
-// @todo: <https://github.com/rpm-rs/rpm/issues/52>
 impl From<u16> for FileMode {
     fn from(raw_mode: u16) -> Self {
         // example
@@ -258,11 +257,13 @@ impl FileOptionsBuilder {
         Ok(self)
     }
 
+    /// Marks the file as a documentation file. Documentation files are tracked in the RPM database for easy access.
     pub fn is_doc(mut self) -> Self {
         self.inner.flag.insert(FileFlags::DOC);
         self
     }
 
+    /// Marks the file as a config file - such files are not automatically overwritten by package upgrades
     pub fn is_config(mut self) -> Self {
         self.inner.flag.insert(FileFlags::CONFIG);
         self
@@ -273,16 +274,19 @@ impl FileOptionsBuilder {
         self
     }
 
+    /// Marks the file as owned by the package, but not "installed" by it, e.g. log files.
     pub fn is_ghost(mut self) -> Self {
         self.inner.flag.insert(FileFlags::GHOST);
         self
     }
 
+    /// Marks the file as a license file
     pub fn is_license(mut self) -> Self {
         self.inner.flag.insert(FileFlags::LICENSE);
         self
     }
 
+    /// Marks the file as a README file which will be copied into the documentation directory
     pub fn is_readme(mut self) -> Self {
         self.inner.flag.insert(FileFlags::README);
         self
@@ -306,7 +310,7 @@ pub struct Dependency {
 impl Dependency {
     /// Create a dependency on any version of some package or file (or string in general).
     pub fn any(dep_name: impl Into<String>) -> Self {
-        Self::new(dep_name.into(), DependencyFlags::ANY, "".to_string())
+        Self::new(dep_name.into(), DependencyFlags::ANY, String::new())
     }
 
     /// Create a dependency on an exact version of some package.
@@ -342,28 +346,24 @@ impl Dependency {
         )
     }
 
-    // TODO: does version matter here?  probably not the common case though
+    // TODO: does version matter here for scripts? I think I've seen it before, it's probably not the common case though
 
     /// Create a dependency on a package or file required for a pre-install script.
     pub fn script_pre(dep_name: impl Into<String>) -> Self {
-        Self::new(dep_name.into(), DependencyFlags::SCRIPT_PRE, "".to_string())
+        Self::new(dep_name.into(), DependencyFlags::SCRIPT_PRE, String::new())
     }
 
     /// Create a dependency on a package or file required for a post-install script.
     pub fn script_post(dep_name: impl Into<String>) -> Self {
-        Self::new(
-            dep_name.into(),
-            DependencyFlags::SCRIPT_POST,
-            "".to_string(),
-        )
+        Self::new(dep_name.into(), DependencyFlags::SCRIPT_POST, String::new())
     }
 
     /// Create a dependency on a package or file required for a pre-un-install script.
     pub fn script_preun(dep_name: impl Into<String>) -> Self {
         Self::new(
             dep_name.into(),
             DependencyFlags::SCRIPT_PREUN,
-            "".to_string(),
+            String::new(),
         )
     }
 
@@ -385,8 +385,7 @@ impl Dependency {
         )
     }
 
-    // TODO: wtf is a config dependency?
-    /// Add a config dependency
+    /// Create a config dependency, typically a dependency on some configuration file provided by a package
     pub fn config(dep_name: impl Into<String>, version: impl Into<String>) -> Self {
         Self::new(
             dep_name.into(),

tests/building.rs

@@ -15,7 +15,7 @@ fn test_empty_package_equivalent() -> Result<(), Box<dyn std::error::Error>> {
         .compression(CompressionType::None)
         .build()?;
 
-    // @TODO: currently failing due to missing tags, different checksums, offsets etc.
+    // TODO: currently failing due to missing tags, different checksums, offsets etc.
     // empty_rpm.canonicalize()?;
     // built_empty_rpm.canonicalize()?;
     // pretty_assertions::assert_eq!(empty_rpm.metadata, built_empty_rpm.metadata);
@@ -37,7 +37,7 @@ fn test_empty_source_package_equivalent() -> Result<(), Box<dyn std::error::Erro
     let empty_rpm = Package::open(common::rpm_empty_source_path())?;
     let built_empty_rpm = PackageBuilder::new("rpm-empty", "0", "LGPL", "x86-64", "").build()?;
 
-    // @TODO: currently failing due to missing tags, different checksums, offsets etc.
+    // TODO: currently failing due to missing tags, different checksums, offsets etc.
     // empty_rpm.canonicalize()?;
     // built_empty_rpm.canonicalize()?;
     // pretty_assertions::assert_eq!(empty_rpm.metadata, built_empty_rpm.metadata);
@@ -51,6 +51,35 @@ fn test_empty_source_package_equivalent() -> Result<(), Box<dyn std::error::Erro
     Ok(())
 }
 
+#[ignore = "Finish once basic RPM equivalence test passes"]
+#[test]
+fn test_basic_package_equivalent() -> Result<(), Box<dyn std::error::Error>> {
+    let basic_rpm = Package::open(common::rpm_basic_pkg_path())?;
+    let built_basic_rpm = PackageBuilder::new(
+        "rpm-basic",
+        "2.3.4",
+        "MPL-2.0",
+        "x86-64",
+        "A package for exercising basic features of RPM",
+    )
+    .description("A package for exercising basic features of RPM")
+    .compression(CompressionType::None)
+    .build()?;
+
+    // TODO: currently failing due to missing tags, different checksums, offsets etc.
+    // empty_rpm.canonicalize()?;
+    // built_empty_rpm.canonicalize()?;
+    // pretty_assertions::assert_eq!(empty_rpm.metadata, built_empty_rpm.metadata);
+
+    // Test that the payload generated is equivalent
+    pretty_assertions::assert_str_eq!(
+        format!("{:?}", &basic_rpm.content.as_bstr()),
+        format!("{:?}", &built_basic_rpm.content.as_bstr()),
+    );
+
+    Ok(())
+}
+
 #[test]
 fn test_rpm_builder() -> Result<(), Box<dyn std::error::Error>> {
     let mut buff = std::io::Cursor::new(Vec::<u8>::new());

tests/signatures.rs

@@ -77,7 +77,6 @@ fn test_rpm_file_signatures_resign() -> Result<(), Box<dyn std::error::Error>> {
     )
 }
 
-// @todo: we could really just use a fixture for this, better than rebuilding?
 /// Test verifying the signature of a package that has been signed
 #[test]
 fn parse_externally_signed_rpm_and_verify() -> Result<(), Box<dyn std::error::Error>> {