Cabal: Reconsider how to specify dependencies on internal libraries

[ezyang]

Right now, if I define an internal library helper in the package foobar, I refer to it by adding an extra line to build-depends:

name: foobar
version: 1.0
library helper
  ...
library
  build-depends:
    base >= 4.4,
    helper

This is... kind of "wrong" from a semantic sense. There are two big reasons: (1) the primary reason that build-depends exist is to specify version bounds on external packages, but if we are referencing an internal library, you never want to actually specify a version bound: you want to refer to the library internally, and (2) we're clobbering the global namespace of packages, you can't name a helper the same name as an external package you also want to use.

We still have an opportunity to fix this, since internal libraries have not been published in a real release of Cabal. In particular, a new feature of Backpack might be a good way to fix this: the mixins field. Here are the proposed new semantics:

1. We define a new syntax for referring to internal libraries within a package. One possibility (to get the bikeshedding going): pkg/libname refers to the libname internal library of pkg. To refer to the current package, you can use a dot, as in ./libname, or the full package name.
2. The build-depends syntax remains UNCHANGED. Instead, we augment the mixins syntax to accept qualified library names; e.g., mixins: foobar/helper, foobar/other-helper.
3. Here is how module visibility works. As in Backpack today, if a library is mentioned in build-depends but not in mixins, we do the historical behavior: the public library of that package is brought into scope. Otherwise, if ANY mixin refers to a package, we no longer bring that into scope; we bring precisely only the libraries identified by mixins into scope. So for example:

name: foobar
library
  ...
library helper
  ...
executable fooexe
  build-depends: foobar
  mixins: foobar/helper

this package only brings the modules of helper into scope for fooexe, not the main library foobar. (BY THE WAY, if we hate the fact that referring to a package via a mixin causes its default "import" to not be brought into scope, we could add a new variant of build-depends for packages we want to depend on, but should not have any modules brought into scope; maybe just version-depends?)

1. Optionally, we can make it optional to specify the name of the current package in build-depends if you want to use libraries from it in mixins. So continuing the example from above,

name: foobar
library
  ...
executable fooexe
  mixins: foobar/helper

is OK (you can omit the build-depends.) This is a bit clearer. If we ever end up supporting multiple public libraries per package, you'll have to specify that package in build-depends (since we need to know to depsolve for it, and what your version constraints are.)

CC @Ericson2314

[abooij]

What about foreign libraries? I can imagine hypothetical scenarios in which executables might want to depend on foreign libraries.

Though such code is probably solving the wrong problem :)

[ezyang]

That would be a different kind of dependency, so it would have its own field, I think.

[Ericson2314]

I mentioned it on IRC, but just to log it here for posterity, the only issue I have with this is ":" being used as an analogous separator in many other contexts.

[ezyang]

Yeah we can use ":". I am just a little mournful that we can no longer use "./" as an idiom for "in this namespace."

[ezyang]

I implemented this, but actually I'm pretty unhappy with the use of the mixins keyword here: it ends up being pretty confusing if you're not actually using Backpack, and also it is just so natural to put library dependencies in build-depends, it's a bit hard to kick the habit.

So my new counter-proposal is that we should fix (2), but for (1) settle for making it "manifestly obvious" what package an attached version bound corresponds to.

1. Extend build-depends syntax to accept the form pkgname:cname bounds and :cname bounds. If you are in a package named pkg, to refer to the internal library helper, you write pkg:helper or just :helper. Because internal packages must always be qualified, there is no longer any ambiguity problem. Furthermore, if there is an attached version bound, it applies to the package that the component is contained in. This is easy for the dependency solver to read off, because the package that the bound should be applied to is directly written in the syntax. As before, package bounds on internal libraries are nonsensical and will result in check warnings, but if we ever support dependencies on internal libraries from external packages, the bounds are useful.

2. Extend mixins syntax to accept the form pkgname:cname renamings and :cname renamings. If you specify a mixin, this overrides the default from build-depends (as before.) However, you can now specify internal libraries in mixins without declaring them in build-depends.

@hvr, @Ericson2314, @abooij, @dcoutts, @23Skidoo what do you think about this plan?

[Ericson2314]

That sounds great! A slight nit is : as the first char gives me absolute-path vibes, so something analogous to . for the current package might still be nice---but maybe I'm biased from prior discussion and this won't confuse most users.

[hvr]

Having some special symbolic identfier to refer to the current package would be nice; however, if we can't agree on one (I'm not sure about the leading :int-lib-name thing either; would a lonely : denote the default library of the current package then?), it wouldn't be the worst thing to just always require the explicit pkgname:-qualifier...

@ezyang That being said, I like the general direction of (1) in https://github.com/haskell/cabal/issues/4155#issuecomment-269847777 ; haven't thought about the (2) part yet

[ezyang]

I picked leading colon because it's how Bazel seems to do it. Bare colon seems like too much: you can just specify the name of the package by itself (which is how existing code does.) But I won't add this feature in for now. Simpler code!

Don't worry about the (2) part, it's just the "obvious" thing.

[Ericson2314]

Ah, so all named/internal library deps will need an explicit package name, whether that library is part of the current packatge or not? That strikes me as the perfect first step :).

[ezyang]

Yep.

[23Skidoo]

@hvr, @Ericson2314, @abooij, @dcoutts, @23Skidoo what do you think about this plan?

+1 from me.

As before, package bounds on internal libraries are nonsensical and will result in check warnings, but if we ever support dependencies on internal libraries from external packages, the bounds are useful.

Do I understand correctly that pkgname:cname dependencies where pkgname != current_pkgname will result in an error as well?

[hvr]

Do I understand correctly that pkgname:cname dependencies where pkgname != current_pkgname will result in an error as well?

@23Skidoo at least as long as we don't support exposed convenience libraries (which would allow package families such as amazonka-* to collapse 89(!) sub-packages into a single package with multiple exposed libraries reducing the pkg-index meta-data overhead by one or two OOMs; which is also something that .deb or .rpms support; I still wonder why cabal shouldn't support this either)

[hvr]

I just ran the numbers; a single amazonka upload currently results in 89 auto-generated packages being added to the package index:

$ cabal list --simple-output | grep amazonka | grep -F 1.4.5
amazonka 1.4.5
amazonka-apigateway 1.4.5
amazonka-application-autoscaling 1.4.5
amazonka-appstream 1.4.5
amazonka-autoscaling 1.4.5
amazonka-budgets 1.4.5
amazonka-certificatemanager 1.4.5
amazonka-cloudformation 1.4.5
amazonka-cloudfront 1.4.5
amazonka-cloudhsm 1.4.5
amazonka-cloudsearch 1.4.5
amazonka-cloudsearch-domains 1.4.5
amazonka-cloudtrail 1.4.5
amazonka-cloudwatch 1.4.5
amazonka-cloudwatch-events 1.4.5
amazonka-cloudwatch-logs 1.4.5
amazonka-codebuild 1.4.5
amazonka-codecommit 1.4.5
amazonka-codedeploy 1.4.5
amazonka-codepipeline 1.4.5
amazonka-cognito-identity 1.4.5
amazonka-cognito-idp 1.4.5
amazonka-cognito-sync 1.4.5
amazonka-config 1.4.5
amazonka-core 1.4.5
amazonka-datapipeline 1.4.5
amazonka-devicefarm 1.4.5
amazonka-directconnect 1.4.5
amazonka-discovery 1.4.5
amazonka-dms 1.4.5
amazonka-ds 1.4.5
amazonka-dynamodb 1.4.5
amazonka-dynamodb-streams 1.4.5
amazonka-ec2 1.4.5
amazonka-ecr 1.4.5
amazonka-ecs 1.4.5
amazonka-efs 1.4.5
amazonka-elasticache 1.4.5
amazonka-elasticbeanstalk 1.4.5
amazonka-elasticsearch 1.4.5
amazonka-elastictranscoder 1.4.5
amazonka-elb 1.4.5
amazonka-elbv2 1.4.5
amazonka-emr 1.4.5
amazonka-gamelift 1.4.5
amazonka-glacier 1.4.5
amazonka-health 1.4.5
amazonka-iam 1.4.5
amazonka-importexport 1.4.5
amazonka-inspector 1.4.5
amazonka-iot 1.4.5
amazonka-iot-dataplane 1.4.5
amazonka-kinesis 1.4.5
amazonka-kinesis-analytics 1.4.5
amazonka-kinesis-firehose 1.4.5
amazonka-kms 1.4.5
amazonka-lambda 1.4.5
amazonka-lightsail 1.4.5
amazonka-marketplace-analytics 1.4.5
amazonka-marketplace-metering 1.4.5
amazonka-ml 1.4.5
amazonka-opsworks 1.4.5
amazonka-opsworks-cm 1.4.5
amazonka-pinpoint 1.4.5
amazonka-polly 1.4.5
amazonka-rds 1.4.5
amazonka-redshift 1.4.5
amazonka-rekognition 1.4.5
amazonka-route53 1.4.5
amazonka-route53-domains 1.4.5
amazonka-s3 1.4.5
amazonka-sdb 1.4.5
amazonka-servicecatalog 1.4.5
amazonka-ses 1.4.5
amazonka-shield 1.4.5
amazonka-sms 1.4.5
amazonka-snowball 1.4.5
amazonka-sns 1.4.5
amazonka-sqs 1.4.5
amazonka-ssm 1.4.5
amazonka-stepfunctions 1.4.5
amazonka-storagegateway 1.4.5
amazonka-sts 1.4.5
amazonka-support 1.4.5
amazonka-swf 1.4.5
amazonka-test 1.4.5
amazonka-waf 1.4.5
amazonka-workspaces 1.4.5
amazonka-xray 1.4.5

[23Skidoo]

@hvr This is a convincing use case.

[ezyang]

@23Skidoo Yes, as long as we don't support exposed convenience libraries.

[ezyang]

Update: This patchset seems tremendously annoying to implement, so I am now inclined to can it and keep the old syntax.

[Ericson2314]

Oh really? https://github.com/haskell/cabal/issues/4155#issuecomment-269847777 seems like surface-level parser tweaks? I really liked it.

[ezyang]

The problem was that it was really tempting to refactor the internal representations so that #4206 would make sense (basically, Setup scripts would know how to build against internal libraries from other packages, even if no other tooling knew), but that ended up being a pretty big refactor which I got bogged down in.

[Ericson2314]

Gotcha. Well hopefully we can at least do the surface level parsing change before 2.0? I'd really prefer pkg:lib to be the only way to access a non-primary library, even if it is internal.

[ezyang]

It's not just a surface level parsing change, as if we introduce pkg:lib as a thing, it will now be possible to name an internal library the same as an external package, and use both simultaneously. So there's going to be some substantive changes.

[Ericson2314]

Hmm, one could just add an assertion that those namespaces be disjoint? Or at least that no referenced external library has the same name even if there is another out there somewhere in the project plan?

[Ericson2314]

@ezyang have you pushed a branch with your work there? I'd be happy to dumb it down for a 2.0 stop-gap :)

[ezyang]

Yeah, here it is: https://github.com/ezyang/cabal/tree/cabal-shake-aborted-mixin

Sorry, it's not cleaned up yet!

The commits are:

1. Some unfinished test porting, which you can ignore
2. "Implement mixin-based internal libraries." which implements the old version of the proposal (you use mixins to refer to internal libraries, build-depends: pkg:lib is not supported
3. Top two commits are in progress work to refactor to support build-depends: pkg:lib but which I got bogged down on.

[Ericson2314]

@ezyang thanks!

[Ericson2314]

Working on finishing @ezyang's work in https://github.com/haskell/cabal/pull/4265

[23Skidoo]

@ezyang @Ericson2314

The 2.0 release is rapidly approaching, can we postpone this for 2.2?

[Ericson2314]

Won't the current syntax be grandfathered in then? I'll have some time early next week. Otherwise I'd truely appreciate if anyone else could take a stab at it.

[23Skidoo]

Remilestoned.

[fgaz]

@phadej isn't this fixed for .cabal 3.4 with one of your recent patches?

[phadej]

@fgaz yes good find. This is fixed by fix for #6083 (#6907).