search

ipetkov / crane

A Nix library for building cargo projects. Never build twice thanks to incremental artifact caching.
https://crane.dev
MIT License
959 stars 92 forks source link

Add `cargoDenyCheckExtraArgs` #747

Closed dpc closed 4 days ago

dpc commented 5 days ago

Is your feature request related to a problem? Please describe.

I want to run cargo check --hide-inclusion-graph.

cargoDenyExtraArgs is passed before check in cargo check, so it doesn't work. cargoDenyChecks is about selecting checks, so adding --hide-inclusion-graph there works, but requires duplicating default checks.

Describe the solution you'd like cargoDenyCheckExtraArgs that goes after check next to cargoDenyChecks would be ideal.

ipetkov commented 4 days ago

Hi @dpc thanks for the report!

Generally I've tried to keep these APIs from growing too many options, especially ones with subtle differences that require the caller to understand the implementation details (ordering of arguments) if they're mostly doing the same thing. Generally we've stuck with cargoExtraArgs so that various common argument inheritance can work, and cargoDenyExtraArgs is following the pattern of per-command extensions.

Given that we also have a third cargoDenyChecks for tweaking the actual check parameters, I'd say it would be overkill to have a fourth. Yes, specifying an additional parameter would require duplicating the default value, but IMO that's not such a bad thing: given that you are diverging from the defaults, it's probably worth being a bit more explicit in re-specifying exactly what checks should be included (the default value we currently have is a nice default, but definitely not the case that we should be authoritative on a default everyone sticks to)

Going to close this out since I don't think we need to go in this direction, but let me know if you disagree.