add PackageBuilder::with_files too allows batch addition of files to a rpm

[Lunarequest]

This pr address #193 and adds a with_files method which allows adding multiple files in batches to the rpm. There is a test based of with_file and documentation as well.

📜 Checklist

• Commits are cleanly separated and have useful messages
• A changelog entry or entries has been added to CHANGELOG.md
• Documentation is thorough
• Test coverage is excellent and passes
• Works when tests are run --all-features enabled

[drahnr]

Could we make these explicit generics? It allows the user to specify the types in case of ambiguities.

[drahnr]

Note: I am not a particular fan of tuples, in this particular case, adding a type just for the sake of a public function input might be overkill.

[drahnr]

A few small nits, generally looks good. Thank you for your contribution!

[dralley]

I've changed my thinking a bit on the whole API for building RPMs.

The trouble is that if you look at building an RPM of even average complexity, the builder approach with pass-by-value just doesn't scale very well. By that I mean mostly that it isn't readable.

 let built_rpm = PackageBuilder::new(
        "rpm-basic",
        "2.3.4",
        "MPL-2.0",
        "noarch",
        "A package for exercising basic features of RPM",
    )
    .epoch(1)
    .release("5")
    .description("This package attempts to exercise basic features of RPM packages.")
    .vendor("Los Pollos Hermanos")
    .url("http://www.savewalterwhite.com/")
    .vcs("https://github.com/rpm-rs/rpm")
    .group("Development/Tools")
    .packager("Walter White")
    .compression(CompressionType::None)
    .build_host("localhost")
    .source_date(1681068559)
    .provides(Dependency::any("/usr/bin/ls"))
    .provides(Dependency::any("aaronpaul"))
    .provides(Dependency::any("breaking(bad)"))
    .provides(Dependency::config("rpm-basic", "1:2.3.4-5.el9"))
    .provides(Dependency::eq("rpm-basic", "1:2.3.4-5.el9"))
    .provides(Dependency::eq("shock", "33"))
    .requires(Dependency::script_pre("/usr/sbin/ego"))
    .requires(Dependency::config("rpm-basic", "1:2.3.4-5.el9"))
    .requires(Dependency::greater_eq("methylamine", "1.0.0-1"))
    .requires(Dependency::less_eq("morality", "2"))
    .requires(Dependency::script_post("regret"))
    .conflicts(Dependency::greater("hank", "35"))
    .obsoletes(Dependency::less("gusfring", "32.1-0"))
    .obsoletes(Dependency::less("tucosalamanca", "444"))
    .supplements(Dependency::eq("comedy", "0:11.1-4"))
    .suggests(Dependency::any("chilipowder"))
    .enhances(Dependency::greater("purity", "9000"))
    .recommends(Dependency::any("SaulGoodman(CriminalLawyer)"))
    .recommends(Dependency::greater("huel", "9:11.0-0"))
    .with_file(
        "./tests/assets/SOURCES/example_config.toml",
        FileOptions::new("/etc/rpm-basic/example_config.toml").is_config(),
    )?
    .with_file(
        "./tests/assets/SOURCES/multiplication_tables.py",
        FileOptions::new("/usr/bin/rpm-basic"),
    )?
    .with_file(
        // I didn't bother filling all of these in yet, obviously it's not complete
        "",
        FileOptions::new("/usr/lib/rpm-basic").mode(FileMode::Dir { permissions: 0o644 }),
    )?
    .with_file(
        "",
        FileOptions::new("/usr/lib/rpm-basic/module").mode(FileMode::Dir { permissions: 0o755 }),
    )?
    .with_file(
        "./tests/assets/SOURCES/module/__init__.py",
        FileOptions::new("/usr/lib/rpm-basic/module/__init__.py"),
    )?
    .with_file(
        "./tests/assets/SOURCES/module/hello.py",
        FileOptions::new("/usr/lib/rpm-basic/module/hello.py"),
    )?
    .with_file(
        "",
        FileOptions::new("/usr/lib/rpm-basic/module").mode(FileMode::Dir { permissions: 0o755 }),
    )?
    .with_file(
        "",
        FileOptions::new("/usr/share/doc/rpm-basic").mode(FileMode::Regular { permissions: 0o644 }),
    )?
    .with_file(
        "",
        FileOptions::new("/usr/share/doc/rpm-basic/README").is_doc(),
    )?
    .with_file(
        "./tests/assets/SOURCES/example_data.xml",
        FileOptions::new("/usr/share/rpm-basic/example_data.xml"),
    )?
    .with_file(
        "",
        FileOptions::new("/var/log/rpm-basic/basic.log").is_ghost(),
    )?
    .with_file(
        "",
        FileOptions::new("/var/tmp/rpm-basic").mode(FileMode::Dir { permissions: 0o755 }),
    )?
    .add_changelog_entry(
        "Walter White <[email protected]> - 3.3.3-3",
        "- I'm not in the meth business. I'm in the empire business.",
        1623672000,
    )
    .add_changelog_entry(
        "Gustavo Fring <[email protected]> - 2.2.2-2",
        "- Never Make The Same Mistake Twice.",
        1619352000,
    )
    .add_changelog_entry(
        "Mike Ehrmantraut <[email protected]> - 1.1.1-1",
        "- Just because you shot Jesse James, don't make you Jesse James.",
        1619352000,
    )
    .build()?;

Maybe it would be better if we passed by reference instead. This would mean you have to assign ownership to a variable for the builder before proceeding with setting new fields, but it would make it a lot easier to split things up into sections and use loops like:

let mut builder = PackageBuilder::new(
        "rpm-basic",
        "2.3.4",
        "MPL-2.0",
        "noarch",
        "A package for exercising basic features of RPM",
    );

builder
    .epoch(1)
    .release("5")
    .description("This package attempts to exercise basic features of RPM packages.")
    .vendor("Los Pollos Hermanos")
    .url("http://www.savewalterwhite.com/")
    .vcs("https://github.com/rpm-rs/rpm")
    .group("Development/Tools")
    .packager("Walter White");

builder
    .provides(Dependency::any("/usr/bin/ls"))
    .provides(Dependency::any("aaronpaul"))
    .provides(Dependency::any("breaking(bad)"))
    .provides(Dependency::config("rpm-basic", "1:2.3.4-5.el9"))
    .provides(Dependency::eq("rpm-basic", "1:2.3.4-5.el9"))
    .provides(Dependency::eq("shock", "33"));

for req in requirements {
    builder.requires(req);
}

You could do this previously using assignments (as noted in #193 (comment)), but this way is cleaner and doesn't lean so hard on the compiler to remove a bunch of memcpys.

This is just an idea, though. I'm curious what others think.

@Lunarequest @cat-in-136

[Lunarequest]

I quite prefer the api you've proposed to the current one. The current one feels rather clunky and obtuse compared to the proposed one

[cat-in-136]

Your suggested new design is more friendly for new users.

I'm glad the suggested code works with my existing coding style of splitting processes like builder = builder.xxx(); builder = builder.yyy(); as used in my project cargo-generate-rpm. I'm accepting your suggestion because it can be applied through simple find-and-replace is a good one.

This would mean you have to assign ownership to a variable for the builder before proceeding with setting new fields

I consider it is possible to write the chained-style code without the need for a builder variable as like std::process::Command.

[dralley]

I consider it is possible to write the chained-style code without the need for a builder variable as like std::process::Command.

You're probably right, I was just thinking that if .build() consumed the builder then you'd definitely need to assign it to variable first, but actually I'm not sure there's necessarily particular reason to do that.

[dralley]

Ok, I've filed #220, we can continue this discussion there.