Gankra / cargo-mommy

Mommy's here to support you when running cargo~
https://faultlore.com/cargo-mommy/
Apache License 2.0
754 stars 51 forks source link

[mood] ability to strip moods from the binary at build time #30

Closed Gankra closed 11 months ago

Gankra commented 11 months ago

I'm not 100% sure how i'd want it to be impl the interface for it, but some people are concerned about the more spicy mood strings being in the binary, so it would be nice to have a way to maybe do like --no-default-features and have only chill mode.

The only place that really needs to change is here:

https://github.com/Gankra/cargo-mommy/blob/aa7801b3a69d789d52a72436c94cfdd8c6fe6e12/build.rs#L40C15-L56

break up the string formatting into several iterative steps, then disable the ones you don't want in the binary (forget if build.rs makes reading features messy).

5225225 commented 11 months ago

I'll look into this, from what I can tell you can just use cfg! inside a build.rs (https://stackoverflow.com/a/66843072), so I'm thinking having

[features]
# This affects if a given response is *supported* in the built binary,
# not if it's enabled by default
default = ["chill", "thirsty", "yikes"]

chill = []
thirsty = []
yikes = []

Haven't tested if cfg! works to read features, but they're definitely at least exposed as envars, which wouldn't be too bad to read, and the installation method would be like cargo install cargo-mommy --no-default-features -F chill. (nixpkgs could probably just expose it as overridable options? not entirely sure how package config works there).

Gankra commented 11 months ago

I think we should just always have chill on (it's the default mode, stuff assumes it exists) although "mommy has no chill build" is extremely funny as a premise.

5225225 commented 11 months ago

Makes sense, yeah. Simply not emitting the yikes/thirsty moods into the binary works except for DENIGRATING_TERMS_DEFAULT/MOMMYS_PARTS_DEFAULT, which are only used in thirsty/yikes (and show up in the binary).

Two options I can see are to either just have them exist but be blank strings, so anything that tries to use them should just panic (but that would be a responses.json or library bug), or cfg out the parsing code. I'm leaning towards the latter, so DENIGRATING_TERM showing up in a chill template string would simply not be replaced, as if it wasn't a placeholder.

Though it does seem somewhat fragile, better tests to make sure that placeholders aren't used in the wrong moods can be future work IMO.

I'll make the error message for an unknown mood say which moods cargo-mommy was installed with, to help track down "why isn't thirsty working, did I typo it or was it just not packaged with it?" Maybe stick it in --version too.