Skip to content
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

Add a --lock flag to cabal freeze to promote a freeze file to a lock file #10785

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

LaurentRDC
Copy link

@LaurentRDC LaurentRDC commented Feb 8, 2025

This pull request adds a --lock flag to cabal freeze, promoting a freeze file to a lock file.

Fixes #10784 .

QA Notes

Calling cabal freeze --lock should produce a freeze file, with one added line compared to cabal freeze:

reject-unconstrained-dependencies: all

Checklist

@LaurentRDC LaurentRDC marked this pull request as ready for review February 9, 2025 00:45
@ulysses4ever
Copy link
Collaborator

I think the changelog and docs should disclose the connection between the new flag and the old one.

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks neat, thank you for the contribution!

@hasufell
Copy link
Member

hasufell commented Feb 9, 2025

Why is the flag named --lock and not --reject-unconstrained-dependencies?

@jasagredo
Copy link
Collaborator

Why is the flag named --lock and not --reject-unconstrained-dependencies?

I think the term "lock file" or "lock" is very widely used for the purpose of this PR in other languages (js with Yarn and rust's Cargo come to mind). It is however true that what it really does is rejecting any unconstrained dependencies.

I this it is a question of familiarity vs precision.

@hasufell
Copy link
Member

hasufell commented Feb 9, 2025

I think it's not a good thing to invent aliases. It will make searching documentation harder and understanding the relation between flags.

@ulysses4ever
Copy link
Collaborator

It's not an alias: while --lock affects behavior of cabal freeze (makes it generate a slightly different freeze file than before), --reject-unconstrained-dependencies affects behavior of v2-builds making them reject plans with dependencies not converted by provided constraints. These are absolutely different things, so, I don't see why they should be named the same.

@hasufell
Copy link
Member

hasufell commented Feb 9, 2025

It's not an alias: while --lock affects behavior of cabal freeze (makes it generate a slightly different freeze file than before), --reject-unconstrained-dependencies affects behavior of v2-builds making them reject plans with dependencies not converted by provided constraints. These are absolutely different things, so, I don't see why they should be named the same.

Maybe I'm reading the proposal wrong:

This new --lock flag reuses the mechanism behind --reject-unconstrained-dependencies, by writing the equivalent of --reject-unconstrained-dependencies=all to the freeze file.

@LaurentRDC
Copy link
Author

LaurentRDC commented Feb 9, 2025

As @ulysses4ever mentioned, --lock isn't an alias, but its relationship to --reject-unconstrained-dependencies is a bit confusing.

--reject-unconstrained-dependencies=all means "when creating the build plan, please reject unconstrained dependencies as inputs". Consider a simple cabal project, with a single cabal file that looks like this:

...
executable myexe
  build-depends:  base ^>=4.20
                , containers
...

Calling cabal freeze --reject-unconstrained-dependencies=all will fail because there is an input dependency with no constraints, containers. This defeats the purpose of cabal freeze in the first place.

What we want to do here instead is:

  1. Create a build plan with whatever constraints we have (and don't have);
  2. Mark the project configuration with reject-unconstrained-dependencies=all afterwards.

This is what --lock does. Logically, you can think of it as:

cabal freeze && cabal configure --reject-unconstrained-dependencies=all

although we want the reject-unconstrained-dependencies=all to go in the cabal.project.freeze file instead.

Right now, in documentation, I wrote that:

Under the hood, the --lock flag reuses the mechanism behind
--reject-unconstrained-dependencies, by writing the equivalent of
--reject-unconstrained-dependencies=all to the freeze file.

What if I wrote this instead?

The --lock flag is related to --reject-unconstrained-dependencies=all. --reject-unconstrained-dependencies=all restricts unconstrained dependencies in the build plan to be frozen, which is probably not what you want. On the other hand, --lock restricts unconstrained dependencies for future build plans by setting the configuration option reject-unconstrained-dependencies=all in the freeze file.

@hasufell
Copy link
Member

hasufell commented Feb 9, 2025

Calling cabal freeze --reject-unconstrained-dependencies=all will fail because there is an input dependency with no constraints, containers. This defeats the purpose of cabal freeze in the first place.

Yes, my intuition was that cabal freeze --reject-unconstrained-dependencies=all will not use it for resolving, but just add it to the freeze file.

And now I realize the interface is confusing:

  • cabal freeze --constraint='containers == 2.0 is used for resolving and will make it into the freeze file
  • cabal freeze --with-compiler=ghc-9.4.8 will not write with-compiler: ghc-9.4.8 to the freeze file, but use it for resolution
  • cabal freeze --lock only writes a stanza to the freeze file

I think it might be worthwhile to have some type of interface that allows to add "arbitrary" (but cabal sanity checked) stanzas to the freeze file. E.g. cabal freeze --add-constraint='...' --add-with-compiler='...'. This will make sure the resulting freeze file can at least be parsed and no arbitrary strings are written to it.

@LaurentRDC
Copy link
Author

I see what you mean. A freeze file is a valid cabal project file (a fact I didn't know until making this PR!), so technically any command accepted by cabal configure could also make its way to the freeze file.

The freeze file write out project configuration according to this particular line of code, which as you can see only relays a few things (constraints, index state, active repos). This PR adds another project configuration value to pass to the config.

I still think reject-unconstrained-dependencies is special, and the addition of a new flag is still warranted. It could conceivably be used as input to the freezing process, but also as output (with the new --lock flag). Consider a cabal project whose dependencies are all specified, but the dependency bounds are wide. In this case, cabal freeze --reject-unconstrained-dependencies=all would be about refining the build plan.

@hasufell
Copy link
Member

hasufell commented Feb 9, 2025

What are the semantics of a "lock file" and that of a "freeze file"? Can we describe that in an abstract sense while ignoring the implementation details of cabal and all its switches?

From the docs, it seems a "freeze file" is:

  • something that ensures the same dependency versions are selected in each environment (does it really deliver that promise?)

Now, a lock file is: ???


I'm worried about the usability. How would a new user understand cabal freeze and then cabal freeze --lock? Why does freeze not lock etc?

If this is just for power users, I'd much rather prefer an option to add arbitrary stanzas.

If we have an idea of what locking means and maybe it's more than just adding one line to the freeze file, could it be its own subcommand even?

@LaurentRDC
Copy link
Author

I would summarize it as:

  • A freeze file is a set of dependency version pins, and the metadata to understand these constraints (e.g. index state);
  • A lock file is a set of exhaustive dependency version pins, and the metadata to understand these constraints.

I initially assumed that cabal freeze created a lockfile, but as mentioned in the associated issue (#10784), a freeze file does not constrain future dependencies. I personally don't see a use-case for a freeze file that isn't exhaustive, but it wasn't explicitly a problem until I had to keep track of all dependencies (including transitive ones). To be clear, freeze files also exist in other build tools (e.g. pip freeze creates a a freeze file, i.e. the set of dependencies may not be exhaustive).

Should cabal lock be its own command? This would definitely help discovery. Moreover, ideally, a lock file would also include some hash of the source distribution of a dependency, to ensure that you truly get something reproducible, like stack.yaml.lock files. Can this easily be done with cabal?

We can get 95% of the way to lockfiles with the PR here.

Alternatively, we could change this PR to be a documentation change, specifying that a freeze file isn't exhaustive, and pointing users to the reject-unconstrained-dependencies configure option, and punt the implementation of lockfiles down the road. I am comfortable with this outcome.

@philderbeast
Copy link
Collaborator

Should cabal lock be its own command?

@Martinsos started and I'm continuing to write up a guide to freezing with #9984. If freezing and locking are different behaviours (I'm not sure) then perhaps they should be separate commands and have separate file extensions for their output, .freeze and .lock.

@georgefst
Copy link

I'd always assumed that freeze files still allowing unconstrained dependencies was an oversight. Perhaps it's worth reaching out to the community, e.g. via a Discourse thread, to see whether anyone actually wants the current behaviour. And if not, then rather than adding any new flags or commands, just fix the existing cabal freeze by making it write reject-unconstrained-dependencies: all.

@jasagredo
Copy link
Collaborator

jasagredo commented Feb 9, 2025

Now that I think about it, if I don't depend on text, then run cabal freeze --lock, add text > 1 to my cabal file, will the build fail?

My suspicion is that it won't fail because text is constrained (although not in the freeze file) defeating the purpose of this PR, no? text dependency will not be frozen yet the build will pass.

I.e. will the reject-unconstrained-dependencies complain if the dependency is not constrained in the freeze file but constrained in the cabal file albeit very loosely constrained (even with >0)?

@LaurentRDC
Copy link
Author

LaurentRDC commented Feb 9, 2025

Now that I think about it, if I don't depend on text, then run cabal freeze --lock, add text > 1 to my cabal file, will the build fail?

My suspicion is that it won't fail because text is constrained (although not in the freeze file) defeating the purpose of this PR, no? text dependency will not be frozen yet the build will pass.

Ahh damn, I just tested it and you're right. This means that the locking mechanism must go beyond re-using reject-unconstrained-dependencies.

As @georgefst mentions, I was surprised by the cabal freeze behavior. Let me start some discussions to see whether we should amend the freezing mechanism.
If not, I will need to think more deeply about this PR

@LaurentRDC
Copy link
Author

Since the changes in this PR are clearly not enough to have the behavior I'm looking for, I suggest moving the discussion over on the original issue (#10784 ) until an appropriate design can be determined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to promote freeze files to lock files
6 participants