Richterrettich / rpm-rs

A pure rust library for building and parsing RPM's
Other
39 stars 18 forks source link

RPMFileOptionsBuilder#mode need to set file type bits #31

Closed cat-in-136 closed 2 years ago

cat-in-136 commented 3 years ago

The argument of https://docs.rs/rpm-rs/0.6.0/rpm/struct.RPMFileOptionsBuilder.html#method.mode shall be same value as st_mode and the file type bit field (12-15bit) is mandatory. If these bit omitted, it generates a corrupted RPM file where these files will not be installed.

The file type bit field description is written in linux programmer's manual (man) of inode(7):

       S_IFMT     0170000   bit mask for the file type bit field

       S_IFSOCK   0140000   socket
       S_IFLNK    0120000   symbolic link
       S_IFREG    0100000   regular file
       S_IFBLK    0060000   block device
       S_IFDIR    0040000   directory
       S_IFCHR    0020000   character device
       S_IFIFO    0010000   FIFO

For example, 0o0100755 should be set to typical executable file.

This specification confused me, although it seems to be in line with the RPM specification. In my software, if no bits set, it is filled from the actual file type (cat-in-136/cargo-generate-rpm#2).

drahnr commented 3 years ago

So a good solution would be to have a more restrictive builder pattern I guess.

Richterrettich commented 2 years ago

Correct me if I'm wrong, but it seems that we only need two modes in the context of RPM:

  1. files
  2. directories

I think we could solve this by adding an enum for for mode, something like this:

#[derive(Copy,Clone)]
pub enum FileMode{
  Regular(i32),
  Dir(i32),
  Invalid(i32)
}

we could make this backwards compatible by implementing From<i32> for this enum.

let a: FileMode = 0o0100_755.into(); // Regular(0o755)
a =  0o0100_664.into(); // Regular(0o644)
a = 0o5_664.into(); // Invalid(0o5_664)
a = 0o0040_664.into(); // Dir(0o644)

By implementing TryFrom we can turn this into aResult:

0o5_664.try_into()?; // Err("Invalid mode")

Finally, the signature of mode would look like this:

pub fn mode<T: Into<FileMode>>(mut self, mode: T) -> Self 

IMO this has three advantages:

  1. Downstream can reasonably react on invalid file modes.
  2. We do not break existing code.
  3. It is still possible to create entries with file modes that we declare invalid now, but may be valid at some point in time by some implementation of RPM.