-
Notifications
You must be signed in to change notification settings - Fork 697
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
Support for adding flag constraints in Cabal files #2821
Comments
I guess it's problematic when manual flags affect the exposed API of a package (e.g. disabling instances or other API parts) or its behavior (e.g. enabling some debugging output), as other packages depending on those same dependencies expect a unaltered API. |
The design of the flag system (from way back now) is specifically such that package authors cannot pick flags of dependent packages, and libraries are not supposed to change their API based on flags. The reason for this is that if this were allowed (and it were used) then it would become impossible to translate cabal packages into binary packages for systems like debian / fedora etc. Many people felt, and still feel that being able to translate into binary package systems is an important feature, and thus this restriction remains. If the tls package is changing its interface based on flags then it's using flags for the wrong purpose. It's perhaps worth exploring what is really going on in this case and see what the most appropriate solution is. Flags may not be the right solution. Actually, it looks like the tls package does not use flags for much at all, it only has a 'compat' flag. Looking at 'cryptonite' the only suspicious looking flags there are "support_deepseq" which does change the interface by adding or not adding instances, and "integer-gmp". The "integer-gmp" is a tricky one, though fortunately not one that affects most users. Do you have a specific example where this is a problem? |
It arises from situations in which there are changes in fundamental libraries: integer-gmp vs. integer-simple is a classic example, as is network vs hans, etc. I run into it fairly regularly with the HaLVM, but I suspect other cross-compilers will have similar problems. |
You could use this to emulate a sort of "optional dependency" where package P provides instances for classes defined in packages A and B, but only when A and B are already being pulled in for some other reason. Optional dependencies like that would avoid one major source of temptation to create orphan instances. |
I've recently come around to the idea that maybe we should use flags to expose extra functionality (like instances, optional bonuses when people have more dependencies). This would help package authors address a very big issue, which is "how many dependencies should I put in the core package, as opposed to split out into separate packages." The issue is even more keenly felt when orphans are involved. (By the way, the description of optional dependencies is incomplete: the solver would have to somehow "prefer" solutions which have more flags off / less dependencies.) I would claim that the only way to reasonably do this is if users can interact with a global namespace of flags, as requested in this ticket, so that they can say, "Well, if package FOO is providing this instance, don't do anything, otherwise, bring in my compatibility declaration." This is VERY related to version bounds, but instead it's a "feature" bound which is solved against. Of course, the Stack and distro people would hate this, because they don't have a flag solver. In general, they'd want to flip as many flags on as possible, because otherwise there is NO WAY to get that code. Maybe this is less of a deal for package distros, who just end up with a bushier dependency graph, but a big deal for Stack users who still have to build everything you depend on. Also, it becomes all the more difficult for a user to determine if they've determined the correct feature flags for their package (it's the version bounds are bogus problem, but doubled!) |
To compare, Cargo's equivalent of flags (features) supports specifying what "features" should be enabled in a dependency. See: http://doc.crates.io/manifest.html#the-[features]-section |
Cabal does nothing to prevent exposing additional modules over flags even for simplest cases.
git-annex requires Text.JSON.Generic. Maybe 'cabal check' could detect these and warn about the problem. |
I have a number of flags that disable modules that are sort of 'extra functionality' that are tagged as unsupported configurations. This avoids distros maintaining one-off patches against my code that each randomly remove different amounts of functionality and make distribution harder to support for things like stage1 builds on obscure platforms. In this case I have a set of modules that are the intended state of the package, but offer unsupported flags to disable stuff for expert users. This is morally against the grain of what @dcoutts wants, but this can shave half an hour off of a fresh build for some of my users, so the users of these configurations put in the time to manage their flags. I also have situations where flags add modules that are otherwise provided by the package I'm shimming over. e.g.
Both of these techniques rely on the ability to change the exposed package list based on the presence of flags, so please be careful that warnings don't turn into errors and package rejections. |
Related: sol/hpack#112 |
Debian and Fedora have solved this issue for cargo (rust package manager) by translating each package+feature into a separate distro package with its own set of dependencies. For example, the failure crate with the feature "derive" is mapped to the Debian binary package "librust-failure+derive-dev". This has existed since around 2016-2017 when we first starting translating cargo crates into distro packages. The same solution could be adopted for cabal if it were to implement this feature, although I can imagine the Haskell maintainers in those distros would have to do some extra work relating to this.
This ability is ideal for implementing instances - instead of arguing about which package the instances should be defined in, either package (that defines the class or the data) can define the instance, with a flag that enables the extra dependency, either enabled by default or not. Users of that instance can then depend on this package with the flag explicitly enabled. |
One further issue that cargo does not consider (I opened an issue as rust-lang/cargo#7769) is that with this sort of ability, the flags become effectively part of a package's API and therefore subject to Package Versioning Policy constraints. So alongside this ability, it would be nice to allow only certain flags to be exposed in this way (i.e. to other packages' cabal dependencies) and be constrained by PVP; whilst other flags remain private and tweakable only by the end user, without being constrained by PVP. |
I'd add that |
It is worth noting that the problem can be mitigated for some use cases using public sublibraries. Instead of name: my-library
library
exposed-modules: My.Type
if flag(aeson-instances)
build-depends: aeson
exposed-modules: My.Type.Aeson put this in your cabal file: name: my-library
library
exposed-modules: My.Type
library aeson-instances
build-depends: aeson
exposed-modules: My.Type.Aeson Then downstream libraries can depend on the sublibrary using |
One of the difficulties with the current system of flags is that it places all the responsibility for getting various flags right to the end-user installing some package way down the chain. It would be nice if those of us writing libraries that only work with certain upstream flag combinations could specify flags in the Cabal file, rather than having to provide long list of flag constraints for our users to follow at the end of the process.
I'm imagining something like:
and so on.
As it stands, I have to provide downstream users explicit instructions about passing
--constraint="cryptonite -integer-gmp" --constraint="tls -network" --constraint="tls +HaNS"
, which is a pain for everyone involved.The text was updated successfully, but these errors were encountered: