Using strum to denoise `Capability` enum

saschagrunert commented 3 years ago

The idea is to use the strum crate to be able to make the Capability enum more compact and Rusty. This means that we could reduce this:

https://github.com/lucab/caps-rs/blob/cb54844125d9dd6de51d6c8c8a951aefbd0d3904/src/lib.rs#L75-L77

to

#[strum(serialize_all = "shouty_snake_case")]
/// All available capabilities.
pub enum Capability {
    #[strum(serialize = "CHOWN", serialize = "CAP_CHOWN")]
    // In a system with the [_POSIX_CHOWN_RESTRICTED] option defined, this
    // overrides the restriction of changing file ownership and group ownership.
    Chown,

We already use this method in containrs, which is a WIP project: https://github.com/cri-o/containrs/blob/master/src/capability.rs

Do you think this makes sense?

lucab commented 3 years ago

Thanks for the report. I did use strum previously in some other projects and, at the time, decided against using it here for several reasons:

the variants in this enum change very slowly, and only in additive mode, so human mistakes in keeping the list update aren't really a concern
strum is a proc-macro, bringing in a good amount of transitive deps and increasing the build-time in a noticeable way (I don't have current numbers at hand for either, and I'm aware the cost of both gets amortized on larger projects)

So overall, unless this is solving some specific problem, I'd keep my original position and say it is not worth it.

Additionally, here there could be an implicit request of having multiple canonical names for a variant (i.e. CAP_-prefix and non-prefixed). Let me know if you further need to touch on that or if was just an example, but for that one too I'd say it is a wrong direction at the level of this library (security-wise, for sane input validation - but can be freely done by consumers).

saschagrunert commented 3 years ago

Alright, thank you for the input, closing this one now.

lucab / caps-rs

Using strum to denoise `Capability` enum #75