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

Improved docs on cabal freeze. #8053

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Improved docs on cabal freeze. #8053

wants to merge 2 commits into from

Conversation

Martinsos
Copy link
Collaborator

@Martinsos Martinsos commented Mar 18, 2022

I edited the part in docs that talks the most about cabal freeze, following up on my questions from #8047 -> it should resolve them and help others get answers to them.

I drew some of the inspiration from the docs for package.lock.json: https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json/ .

Resolves #8047 .

One thing I am confused with is the location of current freeze docs and am wondering if it is a mistake from before?
Specifically, 7.2.4 Opening an interpreter session is under the 7.2 Package descriptions -> that is already weird -> but then 7.2.4.1 Freezing dependency versions (and a couple of other topics) is under 7.2.4 Opening an interpreter session, which is even weirder.
I corrected the second thing, but I am not sure how to correct the first thing, where should ideally these sections go?

@Mikolaj
Copy link
Member

Mikolaj commented Mar 18, 2022

Yes, that seems a mistake from before. One might search (e.g., in gitk) which commits touched the section headers and how that mixup happened and what might have been the intended location.

cabal repl has a longer writup in at least two other places, so perhaps it's worth comparing the texts and move valuable bits, if any, to one of the other two places?

https://cabal.readthedocs.io/en/latest/search.html?q=repl&check_keywords=yes&area=default

regardless of any updates (even patches) that might get published for these dependencies in the meantime.
Therefore, we have effectively "frozen" the dependencies in place, making our build consistent and reproducible.

``cabal.project.freeze`` is intended to be committed to the version control.
Copy link
Member

@jneira jneira Mar 23, 2022

Choose a reason for hiding this comment

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

I think we should warn about the fact cabal.project.freeze is heavily os dependent, specially if you are using native libs.
The thing is even harder if you support windows, as widely used libs have conditionals in the .cabal file using os(windows)
See the caveats in a recent issue: #8059

Copy link
Member

@jneira jneira Mar 23, 2022

Choose a reason for hiding this comment

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

An alternative could be only freeze the hackage index state, as commented here: #8059 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Also i would note it also freezes cabal flags and it could drive to unexpected results: #7944

Copy link
Member

Choose a reason for hiding this comment

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

Ah, and it is also dependent on ghc version 🙂 : #7367

Copy link
Member

Choose a reason for hiding this comment

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

And there is another request about makes docs clear on what to do when you have both a executable and a library: #5750.

Copy link
Member

@jneira jneira Mar 23, 2022

Choose a reason for hiding this comment

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

@Martinsos sorry, i dont want to scare you. I am reviewing the existing issues about freeze and it turns out it has several caveats.
The pr looks nice overall and i love its user friendly style. I would ask only to mention the fact they are (or they might be) dependent on ghc and os, rest of things (freeze index state alternative, flags and #5750) could be added in other prs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hah @jneira, no worries, nothing to be scared of :D, I am glad you are sharing all this info as I wasn't aware of it!
Since we are using cabal freeze in our project currently, I am anyway trying to learn as much as I can about it and how to use it correctly (as is visible in #8059 and #8047), and while doing that I discovered some of these caveats on my own, while the rest I am happy to learn about.
This PR is really a way for me to capture all that I learn on this "journey" and make it a bit easier for others, therefore I am happy to capture as much info as we can.

Let me play a bit more with freeze following days, and I will be coming back here to either update PR with what I learn, or ask additional questions, until we have something that gives a good overview of freeze.

Some questions immediately:

  1. flags -> do I want to freeze them or not? I have to admit I don't understand this topic well enough -> what would be your quick take on it? Are there use cases where I want to freeze them, are there ones where i don't want to? What are those? Is there a general quick advice on how to think about this?
  2. dependent on ghc version -> makes sense, it is not a problem for the project I am currently working on as it is executable that we build with specific ghc always, but I guess it can be a problem for some libraries? Although I thought for them it is not recommended to use freeze file. I guess I am asking what is the situation where one has multi-ghc project and needs freeze file. Probaly should ask that on the relevant issue though and not here -> i is not so important right now anyway.
  3. As for using freeze when project has both executable and a library -> do we know the answer? I am not sure -> I guess it comes down to ensuring that freeze file doesn't get included into the sdist, therefore it doesn't get distributed with a library, but it does get distributed with an executable?

Btw, looking at different issues -> different GHC versions, different OS-es, it seems to me the proper solution might be in the direction of having per-environment (where environment is defined as combo of os, ghc and arch) freeze files. Meaning that user could define environments it cares about, and then on cabal freeze, freeze file for each of those environments would be generated (is that possible? Can we generate freeze file for osx while on linux -> I guess in theory we should be able to, right?). Additionally, cabal would use correct freeze file based on the current environment.
But I will take this question to #8059 .

Copy link
Member

Choose a reason for hiding this comment

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

flags -> do I want to freeze them or not? I have to admit I don't understand this topic well enough -> what would be your quick take on it? Are there use cases where I want to freeze them, are there ones where i don't want to? What are those? Is there a general quick advice on how to think about this?

well there are two kind of cabal flags, automatic and manual and that determines its freezing imo:

  • the cabal solver tries the automatic flags to get a working build plan, so freeze them makes more sense imo. The user usually dont care about them (if they work :-) ) although you can set them explicitly. The archetipical example is integer-gmp/integer-simple which switch between two implementations of the same api.
  • manual ones must be explicitly set by the user, you can use the cli -f option or store it in the cabal.project(.local) if you want to avoid set them in every invocation. That is the case of Provide a way to let cabal freeze freeze dependencies, only #7944 where you have a debug flag you only want to set sometimes. So freeze them could make sense or could not, depending of its usage.

But cabal freeze store them all so you have to delete manually the unwanted ones

@Martinsos
Copy link
Collaborator Author

Relevant comment from @gbaz regarding what is the purpose of freeze file : #8059 (comment) . This view is different than what I described in docs -> before merging this PR, we should agree on what is the purpose of freeze file.

@jneira
Copy link
Member

jneira commented Mar 26, 2022

Relevant comment from @gbaz regarding what is the purpose of freeze file : #8059 (comment) . This view is different than what I described in docs -> before merging this PR, we should agree on what is the purpose of freeze file.

I agree with @gbaz, freze files in its actual implementation, are not suitable to be shared for several devs or be included in the vcs, in the general case. So i think the

cabal.project.freeze is intended to be committed to the version control.

and

This consistency can be valuable as it ensures that all teammates, deployments, and continuous integration are installing the exactly same dependencies.
So if you are running and testing the code on your local machine, you are guaranteed that your teammate and your continuos integration will be running the exact same code,
and that at the end that exact same code will get deployed.

would be problematic.
Another use case of freeze files is in ci, where you can easily produce and consume the same freeze file per ghc and os.

However i think the rest of the pr could be useful 🙂

``cabal.config`` file. All environments which share this file will use
the dependency versions specified in it.
generates ``cabal.project.freeze`` file, which describes the exact dependency tree as it was resolved at that moment by Cabal.
This means it captures an exact version of every dependency, including dependencies of dependencies, recursively all the way.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you definitely want to include the caveats from #8059 that this is a platform-specific freeze, and as such is not necessarily usable by other people.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I mostly agree with this comment: #8059 (comment)

@Martinsos
Copy link
Collaborator Author

Thanks all, this makes a lot of sense. I started this PR thinking that freeze file serve a purpose of team-wide, cross-platform build reproducibility, but that is not correct, instead in their current form they serve a more niche purpose.

I will amend the PR to be very clear about that, while using the information provided in the comment #8059 (comment) + descriptions that you provided above. I believe it will already be very useful to make it explicit for what are freeze files good now, so users don't get confused (as I did).

I could also very shortly mention index-state as potentially more fitting mechanism to achieve some kind of practical build reproducibility.

@andreasabel andreasabel marked this pull request as draft May 2, 2022 15:50
@andreasabel
Copy link
Member

@Martinsos : I converted this PR to draft because you expressed your intent to revise it.

@ulysses4ever
Copy link
Collaborator

@Martinsos any chance you get back to this? Truly valuable work has been done already, and I’m looking forward to see it improved per reviews above and merged!

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 1, 2022
@ulysses4ever ulysses4ever removed the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Sep 3, 2022
@philderbeast
Copy link
Collaborator

philderbeast commented Apr 26, 2024

@Martinsos I've cherry-picked your changes and moved all but the top-level cabal freeze content to a guide. This is available as https://github.com/cabalism/cabal/tree/docs/freeze-brrgh.

I'd like to work with you on this. If you think my branch preserves your content1 then would you be happy to bring those changes into this pull request branch? If you'd rather, I can open a separate pull request.

Footnotes

  1. I think it does. My intent was to move but not change it so that it fits in with the new docs layout.

@philderbeast
Copy link
Collaborator

@Mikolaj do you have a preference on how to proceed? Do we risk detaching stale review comments from the snippets they refer to?

@Mikolaj
Copy link
Member

Mikolaj commented Apr 26, 2024

@Mikolaj do you have a preference on how to proceed? Do we risk detaching stale review comments from the snippets they refer to?

I don't understand the details, but generally, PRs are cheap, if you all feel like sparing some extra effort juggling multiple copies in order to preserve comments in the context of their code, go for it.

@Martinsos
Copy link
Collaborator Author

@philderbeast sorry for late response -> I just got a baby 10 days ago and was a bit busy with that :D!
It has been some time since I have worked on this PR draft so I lost a lot of context, and with current increased load on the private life I don't have the capacity at the moment to tackle this properly, so please take whatever you like from my draft and create your PR. Let me know once you did and I can close this PR. I am glad that you are working on this and that it will get done, because I remember I cared quite a bit about it when I was working on it (and still do think it is important and useful).

@philderbeast
Copy link
Collaborator

Congratulations @Martinsos!

@philderbeast philderbeast mentioned this pull request May 7, 2024
2 tasks
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.

Recommended way to use cabal freeze for an executable?
7 participants