c-blake / cligen

Nim library to infer/generate command-line-interfaces / option / argument parsing; Docs at
https://c-blake.github.io/cligen/
ISC License
509 stars 24 forks source link

Switch magic from header to dynlib #115

Closed SolitudeSF closed 5 years ago

SolitudeSF commented 5 years ago

This requires no changes in lc. cligenMagic is enabled by default, which is very safe bet, and can be optionally disabled by -d:cligenMagic:false.

c-blake commented 5 years ago

Ok. I see what you did there. Anything doing import magic wouldn't be able to statically link, but that might be ok.

Hmm. So, how safe a bet is it? Is file a required dependency for some major package or is just part of all the base installs? I always install it, myself. I just don't know.

Also, for a client package like lc where people might not want to enable libmagic support even if it's possible on their build system, maybe you know of a way to pass options from nimble install down through the chain. Would it be possible for us to say nimble install --with:magic lc for people that want the dependency? (or we could reverse it to --with:nomagic or --without:magic or something).

SolitudeSF commented 5 years ago

what distro are you on? arch and void (and i'm gonna bet every derivative of debian) have file in base installation.

nimble install has a restriction that it doesnt pass compiler flags. maybe i should open nimble issue for that. https://github.com/nim-lang/nimble/issues/599

c-blake commented 5 years ago

I use Gentoo myself. I have a pile of /etc/portage/patches.

I agree that nimble install xyz -d:pdq (or the -d: anywhere, really) would be broadly useful to let installers manage optional dependencies.

With lc we could then return to something more like -d:lcMagic.

c-blake commented 5 years ago

Well, I just pushed something that seemed a reasonable compromise between various in-play ideas - take out the magic.h dependencies (that c2nim had put in automatically, but are not needed), but leave the passl compile-time linking to enable static linking or ldd on client executables to see if magic support/dependency is there.

Not trying to make too big a deal over the static linking/passl vs dynlib bit, but one problem I've had just recently over in lc linker troubles with getpwent usage because Ulrich decided that "mandating dynamic linking is a good idea" like 15 years ago. I don't want to perpetuate the cycle of violence if I don't have to. ;-)

I absolutely agree having some ability to disable magic support with something like your -d:cligenMagic:false is a very good idea. I think that should maybe be revisited based upon whether we can get nimble to have a pass-through feature for at least --define.

SolitudeSF commented 5 years ago

i just implemented some draft of it https://github.com/nim-lang/nimble/pull/682

c-blake commented 5 years ago

Looks pretty simple to me. Hopefully it gets accepted.

I can put back in that behavior so, eg., nimble install lc -- -d:cligenMagic:false will install some unmagical lc. Your -d:danger is another good example.

c-blake commented 5 years ago

(In case you didn't notice, I put that back in and it works fine in regular nim c mode.)