-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide pkg-config packages for optional modules #675
Conversation
This makes it way easier for dependent projects to probe whether libsecp256k1 was built with an optional module they might require.
Thanks! I think this looks good but I have no idea about pkg-config. |
Doesn't it kind of defeat the purpose of having modules which are experimental because they are unreiewed or patent problematic just exposed to callers like that? IMO that really just make the experimental stuff seem like blame shifting onto end users who are less qualified to deal with the consequences. |
Not every optional module is experimental, though, and I expect that longer term there will be more non-experimental but optional pieces of the codebase (simply because not every user needs them and they're clearly separated functionality). For those, something like this makes perfect sense. Right now, only the ECDH module is experimental. If the recovery stuff would be added today, it would probably be marked as experimental too, but for historical reasons it wasn't. There is probably a different discussion to be had about what ought to be/remain experimental, and how to properly avoid people relying on it. |
My long time experience is that optional caller-visible features in libraries on 'big systems' is pretty negative: All programs on the system share the same library so the system wide library needs to have all the optional parts turned on. When parts get left off (e.g. when things get upgraded without package configs changing), you get mysterious failures. Or you get stuff like applications where chunks of their functionality are silently omitted because some dependency three levels down had a build flag forgotten. Having components which can be optionally disabled in specialized embedded cases is another matter-- I think that works out fine. But those cases don't use pkg-config, so this wouldn't help them, nor do embedded-on-big-system cases like libsecp256k1 in bitcoin use pkg-config. AFAIK only the system-library sort of usage does, and that's precisely where optional library features mostly don't work. I don't think this change is harmful (except for the point about experimental features) but I think it's not a very good alternative to making all first class features (stuff that desktop applications could depend on being available) on by default unless specifically disabled.
Pkg-conf can detect the incompatibility but it doesn't do anything about it, you're still stuck with something that doesn't work, it can't tell you how to fix it. A functionality test (e.g. trying to link the relevant functions) is an much stronger test for the functionality being available (which also does nothing about making it work). Historically the standard way of dealing with interface compatibility is via tests rather than sniffing for a variety of reasons, but mostly because a test is definitive. I would argue that it would be much better to just ship m4 tests for the modules that users could use, except not everything that uses pkg-config uses configure. What happens when something that used to be an optional module becomes a mandatory feature, e.g. because it matures or because some other component starts using it internally? A test would pass (presuming the interface didn't also change at the same time), but checking for flags wouldn't unless a special effort was made to preserve all historical flags. |
The error messages from linkers are much more incomprehensible than error messages from build systems. Of course you are right that you only know if it works if you can run and test it, but these package offer an extra option of probing. In case the flag becomes a standard feature, the file can be installed always, the pkg-config files are tiny. And it is not likely to happen very often given that experimental features tend to go in the zkp library. The reason I am requesting this, is because I'd like to probe for ECDH support from Cabal. Cabal supports pkg-config, but it doesn't use m4. Cabal's solver can take your pkg-config list into consideration before choosing build flags for packages. It cannot probe for symbol existence. If compilation were really the only proper way to probe, we wouldn't need packages or version numbers at all! But this is not the status quo. If there is no easy way to probe for ECDH support, people are pushed towards just bundling libsecp256k1 builds everywhere. This remains an option either way, but I think it is too extreme if secp256k1 actively discourages OS packaging. |
We should indeed enable all non-experimental features by default. Let's assume we do this. Would it still make sense to add the additional package files? Is this common? edit: |
I don't believe I've ever personally encountered multiple package files for build time optional features, though I'm sure it exists. It's much more common to use feature testing (or... even more common for downstream users to just have linking fail ... ). |
Normally, people use version numbers to solve this problem. For example, almost all Linux distributions have version numbers bounds on a package they depend on. Packages typically do not have optional features. That's why you're not gonna find many packages providing more than one pkg-config module. The version number approach translates well to binary packages since a package remains backwards compatible after adding symbols. Most people use binary package managers. Windows even had the practice of installing multiple versions of the C++ runtime library implementation, where you could consider the version number part of the package name. Since libsecp256k1 doesn't have releases or version numbers, that excludes this project from officially interacting with any binary package manager. This is what makes libsecp256k1 different. The alternative of trying to use a symbol and then failing if it doesn't link, is AFAIK popularized by autotools, which predates pkg-config. As previously mentioned, pkg-config is an addition that doesn't interfere with users who are not using pkg-config, so you are not forced to use it. Consumers can use |
That's why I'm reviving this PR. If we make a release and we want this to be included, then it will be good to merge this before the release. So far with my limited pkg-config background, I still think this is a useful addition to the project. |
@luke-jr I saw you commented on #817 regarding pkg-config. I thought I'd like to notify you of this PR since it uses pkg-config to signal available modules. You suggested multiple having multiple shared objects, which is a bit different. Of course this is mutually exclusive with #817, if the deprecation means removal. |
I see several other packages on my system with module pkg-config files:
Bitcoin Knots does, and could arguably benefit from this now that the consensus code requires optional features. |
Closing this. All modules which we recommend for use are now enabled by default. |
This makes it way easier for dependent projects
to probe whether libsecp256k1 was built with an
optional module they might require.
Fixes #666