Override imported package version equalities

[philderbeast]

Fixes #9511. Allows overriding package version equalities by giving priority to shallower constraints in the project import tree.

• 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.
• Manual QA notes have been included.
• Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

[philderbeast]

I have local tests that I used that reference two versions of hashable.

constraints: hashable ==1.4.3.0
constraints: hashable ==1.4.2.0

When I add these tests to cabal-testsuite I'm no longer able to access hackage but is there another way to still reference these versions of hashable for the tests?

$ cabal run cabal-testsuite:cabal-tests -- \
   --with-cabal=./dist-newstyle/.../cabal \
   cabal-testsuite/PackageTests/VersionPriority/local-vs-local.test.hs
...
-----BEGIN CABAL OUTPUT-----
Could not resolve dependencies:
[__0] trying: cabal-version-override-0.1.0.0 (user goal)
[__1] unknown package: hashable (dependency of cabal-version-override)
[__1] fail (backjumping, conflict set: cabal-version-override, hashable)
After searching the rest of the dependency tree exhaustively,
   these were the goals I've had most trouble fulfilling: cabal-version-override (2), hashable (1)
-----END CABAL OUTPUT-----
...

How can I add a dependency on a package from Hackage in a test? By
default, the test suite is completely > independent of the contents of Hackage,
to ensure that it keeps working across all GHC versions. If possible, define the
package locally. If the package needs to be from Hackage (e.g., you are testing
the global store code in new-build), use withRepo "repo" to initialize a "fake"
Hackage with the packages placed in the repo directory.
SOURCE: cabal-testsuite FAQ

[mpickering]

You should create a folder which contains package tarballs and use withRepo (perhaps you answered your own question).

[gbaz]

I'm glad you're working on this, but I think this by-depth design is probably overcomplicated? Wouldn't it be simpler (for users as well as implementers) to just have later constraints override earlier constraints?

[philderbeast]

I'm glad you're working on this, but I think this by-depth design is probably overcomplicated? Wouldn't it be simpler (for users as well as implementers) to just have later constraints override earlier constraints?

I'm not aware of any other configuration we have that is order dependent so (before working on this) would have expected the following two projects to pick the same override .

-- cabal.local-vs-stackage.project
packages: .
constraints: hashable ==1.4.2.0
import: https://www.stackage.org/nightly-2023-12-07/cabal.config

-- cabal.local-vs-stackage.project
packages: .
import: https://www.stackage.org/nightly-2023-12-07/cabal.config
constraints: hashable ==1.4.2.0

$ cabal freeze --project-file=cabal.local-vs-stackage.project --dry-run
Resolving dependencies...
SOURCE: project config ProjectConfigImport {importDepth = 0, importPath = "/.../cabal.local-vs-stackage.project"}, CONSTRAINT: PackageConstraint (ScopeQualified QualToplevel (PackageName "hashable")) (PackagePropertyVersion (ThisVersion (mkVersion [1,4,2,0])))
SOURCE: project config ProjectConfigImport {importDepth = 1, importPath = "https://www.stackage.org/nightly-2023-12-07/cabal.config"}, CONSTRAINT: PackageConstraint (ScopeQualified QualToplevel (PackageName "hashable")) (PackagePropertyVersion (ThisVersion (mkVersion [1,4,3,0])))
Freeze file not written due to flag(s)

$ cabal freeze --project-file=cabal.stackage-vs-local.project --dry-run
Resolving dependencies...
SOURCE: project config ProjectConfigImport {importDepth = 1, importPath = "https://www.stackage.org/nightly-2023-12-07/cabal.config"}, CONSTRAINT: PackageConstraint (ScopeQualified QualToplevel (PackageName "hashable")) (PackagePropertyVersion (ThisVersion (mkVersion [1,4,3,0])))
SOURCE: project config ProjectConfigImport {importDepth = 0, importPath = "/.../cabal.stackage-vs-local.project"}, CONSTRAINT: PackageConstraint (ScopeQualified QualToplevel (PackageName "hashable")) (PackagePropertyVersion (ThisVersion (mkVersion [1,4,2,0])))
Freeze file not written due to flag(s)

[gbaz]

We sort of have order dependence in that we accumulate information monoidally. It just so happens that the monoids we accumulate in are typically the list monad. But if there is any project setting which is itself order-dependent in the accumulated list, then that will be observable.

In general, I think order-dependence will be more intuitive to end-users than depth-dependence -- and there are other places, such as remote package repos, where an "overriding" semantics might be useful as well, so unless we want to annotate them all with depth, then it makes sense to start using order-dependence more, in a uniform way.

[philderbeast]

Although a stack resolver is not an import, I did an experiment and there's no order dependence there.

# stack.yaml
packages: ["."]
resolver: nightly-2023-12-07

# stack.stackage-vs-local.yaml
packages: ["."]
resolver: nightly-2023-12-07
extra-deps: [hashable-1.4.2.0]

# stack.local-vs-stackage.yaml
packages: ["."]
extra-deps: [hashable-1.4.2.0]
resolver: nightly-2023-12-07

$ stack build --stack-yaml=stack.yaml --dry-run
No packages would be unregistered.

Would build:
* cabal-version-override-0.1.0.0: database=local, source=/.../cabal-version-override/, after:
  hashable-1.4.3.0.
* hashable-1.4.3.0: database=snapshot, source=hashable (from Hackage)

No executables to be installed.

$ stack build --stack-yaml=stack.stackage-vs-local.yaml --dry-run
No packages would be unregistered.

Would build:
* cabal-version-override-0.1.0.0: database=local, source=/.../cabal-version-override/, after:
  hashable-1.4.2.0.
* hashable-1.4.2.0: database=snapshot, source=hashable (from Hackage)

No executables to be installed.

$ stack build --stack-yaml=stack.local-vs-stackage.yaml --dry-run
No packages would be unregistered.

Would build:
* cabal-version-override-0.1.0.0: database=local, source=/.../cabal-version-override/, after:
  hashable-1.4.2.0.
* hashable-1.4.2.0: database=snapshot, source=hashable (from Hackage)

No executables to be installed.

[philderbeast]

You should create a folder which contains package tarballs and use withRepo (perhaps you answered your own question).

Thanks @mpickering, the way you suggested doing it is working.

[philderbeast]

Wouldn't it be simpler (for users as well as implementers) to just have later constraints override earlier constraints?

I want to be alerted if I have conflicting constraints at the same depth in the import tree. We shouldn't resolve the following (and keep reporting that solving "Could not resolve dependencies"):

-- cabal.project
packages: .
constraints: hashable ==1.4.3.0
constraints: hashable ==1.4.2.0

[philderbeast]

@Mikolaj I don't know why the macos checks cancelled. Seems I can only restart one at a time. Is there another way to push these along?

[Mikolaj]

That's what the github CI does occasionally for macos (congestion, probably). I've cancelled all and rerun all failed. I've bumped your role to "Maintain". Let me know if you can now rerun all from the button on the top-right of the CI runs screen.

[michaelpj]

I want to be alerted if I have conflicting constraints at the same depth in the import tree. We shouldn't resolve the following (and keep reporting that solving "Could not resolve dependencies")

What about explicitly listed priority levels?

-- error
constraints: hashable ==1.4.3.0
constraints: hashable ==1.4.2.0

-- override constraint wins regardless of order
constraints: override hashable ==1.4.3.0
constraints: hashable ==1.4.2.0

-- error
constraints: override hashable ==1.4.3.0
constraints: override hashable ==1.4.2.0

That would handle the basic case of overriding imported constraints (do we have any others?). If we were worried about more complex cases we could have annotated priority levels. That's kind of ugly, but it's what e.g. NixOS does to resolve setting conflicts.

[philderbeast]

Thanks for the suggestion @michaelpj. I don't think we have to worry about overriding version equality constraints within the root project file. If there's a conflict (because we have two differing version equality constraints in the root project) hopefully the warning or error message is good enough to find the problem.

I guess the concern here is that we won't be aware that we're overriding another constraint for the same package, one that is imported and not immediately visible from the root project? Would it help to report the overrides?

[michaelpj]

I think my concern with the current design is that it ties the thing we want (constraint overriding) to a conceptually unrelated thing (whether the constraint comes from an import). For example

import ../my-company-serious-package-set

constraints: hashable ==1.4.2.0

What should happen here? It entirely depends! Perhaps my company policy is that nobody is allowed to deviate from the package set, and so this should be an error. There is no particular reason to assume that "imported" should mean "overridable".

My suggestion is to explicitly make statements about the thing we want to say: "this constraint overrides" (could also have "this constraint is overridable").

[philderbeast]

@michaelpj I don't see how preventing overrides could be enforced when an import can be downloaded locally and edited at will.

[philderbeast]

Let me know if you can now rerun all from the button on the top-right of the CI runs screen.

Yes I can now see [Rerun all jobs] and [Rerun all failed jobs] buttons at the top right of the screen, thanks @Mikolaj.

[philderbeast]

I think having any of our project semantics interact with the solver is a sure path to confusion. we need our project semantics to have a clear deterministic meaning, and our solver to work on a simple fixed set of inputs. undermining either property will be very bad for people's understanding, as well as the modularity of the code base.

Thanks @gbaz, I agree.

Trying out last wins (rather than shallowest wins) was slow going. The elements of the gcs :: Map PN [LabeledPackageConstraint] input to solve' had constraints in reversed order. Changing to Map.fromListWith (flip (++)) preserves the order of constraints.

Note

I'm guessing that gcs stands for grouped constraints.

cabal/cabal-install-solver/src/Distribution/Solver/Modular.hs

Line 69 in feaa338

gcs = M.fromListWith (++) (map pair pcs)

There's quite a few other places in the codebase where the order of elements may be reversed by fromListWith.

Note the reverse ordering of "cba" in the example.
SOURCE: Map.fromListWith

[gbaz]

I'm glad you are implementing the last version wins approach, but I have two concerns about how this code has evolved. I think that "last version wins" should not be implemented as affecting the solver. None of the code here should affect the solver. It should only affect how we collect and treat inputs to the solver. That sidesteps the whole issue of how the solver may or may not order constraints with fromListWith. Inputs to the solver itself should not be order dependent. The only place order dependence should matter is in processing distinct version constraint stanzas in project files.

Further, I think we should pick one approach and stick with it, rather than introducing a new flag that lets us switch between approaches. For testing purposes, its fine to have a flag so people can experiment. But what we merge and release should only be one way or the other, having explored the possibilities.