Add package-db flag for v2 commands

[patrickdoc]

---

Please include the following checklist in your PR:

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

---

This PR teaches v2-* commands how to use the --package-db flag (#5773). The records were already present in the config types, just commented out. I've un-commented them and injected the values into the two places they were needed (initial plan and elaboration).

TODO:

• Modify package hashes based on PackageDBStack
• Maintain backwards-compat for the default stack hash
• Handle flag passed in cabal.project file
• Document how the flag works

The added tests cover:

• adding an additional package db to your build (--package-db=xyz)
• clearing all package dbs (--package-db=clear)
• clearing the default package dbs and using all manual (--package-db=clear --package-db=global --package-db=xyz)

[jneira]

@patrickdoc thanks for the pr, i will ping @phadej as he opened the original issue

[fgaz]

Oooh this will be useful for debugging #7270

[jneira]

Out of curiosity how this interacts with the V2 store package-db? is it cleared with clear? sorry if it is something obvius

[phadej]

I think that package-db stack should be recorded in package hashes. This patch doesn't seem to include changes there.

EDIT: yes, it very much should be recorded, as otherwise store will look broken, as it will refer to units in package-db which you don't know about. (cabal itself doesn't have tools to check the integrity of the store, there is an issue about it, my cabal-store-check is rough proof-of-concept). Here I assume that store is appended as the last database, after specified --package-db stack.

[phadej]

Most importantly, this lacks documentation.

Out of curiosity how this interacts with the V2 store package-db?

Is important question to answer, in particular.

[gbaz]

My understanding/expectation is that it interacts as described above -- i.e., cabal will provide the packages in the store following the package-db stack, just as it does with the default stack?

Let's think through the hash case closely. Suppose I have a in some stack, and b depends on a and is put in the store. Now I compile against a stack without a. I think that what will happen is cabal's solver will realize both b and a are required, and in turn it will not be able to find a and so will rebuild it and place it in the store. The question is then if it will attempt to use the b linked against the "old" a or if it will instantiate a new one. Further, if it does the former, will that even be an issue?

I think its worth testing first to be sure that this path doesn't somehow work, but I also understand very much the concern here.

I'd also like to figure out where the documentation goes? The flag is already listed in the "Setup.hs flags" section of the docs, which is where most similar flags that cabal-install accepts but are passed to the underlying Setup are listed. (Ah -- maybe the cabal.project section as well!)

[phadej]

Let's think through the hash case closely.

My comment to include package db stack in hashes of packages will render that scenario impossible.

In cases when there are additional package-db and aren't, hashes of b package units will be different, so the "hanging" b in the store won't be reused.

In simple: package hash should record all dependencies and configurations. Package-db stack is a dependency/configuration.

[gbaz]

Right -- I was just trying to both fully express what the precise bad outcome could be and also expressing a question as to whether that outcome would occur or not -- i.e. would the hash actually need to be changed, or is it possible that things would work without it.

Note that changing the hash can be problematic (without careful attention to backwards compat) because a simple cabal-install version bump could wind up invalidating an entire store.

[patrickdoc]

Most importantly, this lacks documentation.

The goal was to stick to the description provided by the current documentation:

.. option:: --package-db=db

Allows package dependencies to be satisfied from this additional
package database *db* in addition to the global package database.
All packages in the user's package database will be ignored. The
interpretation of *db* is implementation-specific. Typically it will
be a file or directory. Not all implementations support arbitrary
package databases.

This pushes an extra db onto the db stack. The :option:`--global` and
:option:`--user` mode switches add the respective [Global] and [Global,
User] dbs to the initial stack. There is a compiler-implementation
constraint that the global db must appear first in the stack, and if
the user one appears at all, it must appear immediately after the
global db.

To reset the stack, use ``--package-db=clear``.

Before the change, the PackageDB stack was (relevant code in ProjectPlanning elaboration):
[CompilerDB, InplaceDB]

After the change, the PackageDB stack is:
[CompilerDB, <user flag dbs>, InplaceDB]

Where <user flag dbs> can optionally clear to remove the CompilerDB from the stack.

My tests are mostly based on the "read" aspect of picking up available packages from alternative stores as opposed to the "write" aspect of where they go when they are built. I'll work on clearing up what happens in the store.

Any thoughts on where you'd like the docs to be once that is worked out?

[gbaz]

@patrickdoc I think the "cabal.project" section of the documentation could have the flag (as that also has v2 opts) as well, rather than just the Setup.hs section.

Another thought could be that we could enable this flag just with build and not with install, which would bypass the issue of the store hashes in a very straightforward way, and might still suffice for all the intended use cases?

[jneira]

thanks for the info, it is very useful indeed, I still miss the place of the store package db in that picture

[patrickdoc]

I've been researching the packageDB and store, and I'm not sure I understand the model well enough to implement a valid + satisfying solution.

Here's a hypothetical situation:

---

Package in Store, but not in Package-DB

Reproducer:

• Take a trivial package p, and add a dependency on say
• cabal --with-store=</abs/path/to/temp/store> build to avoid messing with your real store
• ghc-pkg -f </abs/path/to/temp/store/compiler/packagedb> unregister say
• Clean and try to build p again

I get this failure:

Resolving dependencies...
Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - p-1.0 (lib) (configuration changed)
Configuring library for p-1.0..
cabal-3.4.0.0: The following package dependencies were requested
--dependency='say=say-0.1.0.1-6447a6fe8d382e8935bead5fcfe974a89f857ced8f6dc88cfb6263870267c1c9'
however the given installed package instance does not exist.

---

I see the options as:

1. Fail, force the user to delete the invalid entry from the store (current behavior)
2. Succeed, overwrite the existing store entry
3. Succeed, register the existing store entry in the package-db

My understanding of the model is that 3 is theoretically acceptable. A hash should uniquely define a build of a package. So if I have a build that matches my expectations, it would be safe to use it.

If that is true, then we technically wouldn't need to change the hash with different package-db stacks, and the store could be shared.

If it is not true, then we lose out on a reasonable large sharing opportunity. But perhaps the use case is small enough that we don't really care about taking advantage of this possibility.

[patrickdoc]

Requested changes are complete:

• package db stack added to the hash
• flag documentation added to the guide

[Kleidukos]

LGTM.

[jneira]

So if you uses --package-db, could not you specify the <compiler store> as one of the pieces of the stack? is it included always by default? or you have to set it explicitly with the path if you want it to be included?

I would include a note in docs to make clear what happens with it if you uses the flag

[jneira]

i would change <compiler store> with <cabal store> as that package db is created and used by cabal. to distinguish it from the global, which is tied to the compiler

[jneira]

Before the change, the PackageDB stack was (relevant code in ProjectPlanning elaboration):
[CompilerDB, InplaceDB]

After the change, the PackageDB stack is:
[CompilerDB, , InplaceDB]

If this is still up to date could be used as a model to add the note i miss

[patrickdoc]

So I tried to keep this relatively concise, but using my computer (debian) as an example:

global: ~/.ghcup/ghc/8.10.7/lib/ghc-8.10.7/package.conf.d
user: not defined
store: ~/.cabal/store/ghc-8.10.7/package.db
in-place: ./dist-newstyle/packagedb/ghc-8.10.7

By default, the package stack you will have with v2 commands is:
[global, store]
So all remote packages required by your project will be registered in the store package db (because it is last).

When cabal starts building your local projects, it appends the in-place db to the end:
[global, store, in-place]
So your local packages get put in dist-newstyle instead of the store.

This flag manipulates the default prefix:
[global, store]

To more directly answer your questions:

• yes, you can specify the store package db with: --package-db /home/patrick/.cabal/store/ghc-8.10.7/package.db
• yes, it is included by default

The main tricky bit is the clear option, which removes all previous entries. Ex.
--p-db foo -> [global, store, foo]
--p-db clear --p-db foo -> [foo]
--p-db clear --p-db foo --p-db clear --p-db bar --p-db baz -> [bar, baz]

[jneira]

wow, that is a great explanation, imo it deserves be included in docs including the super useful final examples, would you mind do it in this pr as you have it almost written?

[patrickdoc]

Sure. Just under the flag description or a more general place?

[jneira]

in the flag description sounds good to me, it is a somewhat advanced stuff and useful for someone who already wants to tweak seminternal things

[patrickdoc]

updated

[Mikolaj]

@phadej: I'm guessing your comments have been addressed sufficiently and so I will take the liberty of dismissing the review blockage, as per usual, unless you object. Thank you once again for your insight!

[fgaz]

Please add @SInCE annotations

[patrickdoc]

updated

has been addressed

[jneira]

Many thanks for the pr and for addressing my suggestions, looks really great

[Mikolaj]

@fgaz: have your comments been addressed?

Dear all, should we merge?

[fgaz]

@fgaz: have your comments been addressed?

Yes, let's merge!