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

Top-level command for Freezing dependencies #1519

Merged
merged 1 commit into from
Dec 14, 2013

Conversation

benarmston
Copy link
Contributor

This series of patches implements a new top-level command, freeze, which will resolve the dependencies to exact versions and write a constraints section to cabal.config.

The new command takes a number of options related to resolving dependencies, namely, --solver, --max-backjumps, --reorder-goals and --shadow-installed-packages. Previously, I had asked whether other option are required. As no-one said that they are, I've taken that to mean that they are not.

Issues that may still need addressing include:

  • What should happen if this command is run outside of a sandbox.
  • Whether or not cabal.config can be entirely overwritten.

Regarding the last point, @tibbe has stated that it should not, whilst @gregwebs is of the opinion that it is fine to do so. Before I invest any time in replacing only the constraints section, I'd like to have some agreement on whether or not it is required.

This branch contains a number of dead-end efforts. If you'd like them to be rebased away, let me know. I've hesitated doing that as I didn't want to force push.

See issues #1502 and #1499 for discussion of this feature.

@23Skidoo
Copy link
Member

This branch contains a number of dead-end efforts. If you'd like them to be rebased away, let me know. I've hesitated doing that as I didn't want to force push.

Yes, please do that. Force push is fine for WIP branches.

@benarmston
Copy link
Contributor Author

This branch contains a number of dead-end efforts. If you'd like them to be rebased away, let me know. I've hesitated doing that as I didn't want to force push.

Yes, please do that. Force push is fine for WIP branches.

Done.

@23Skidoo
Copy link
Member

Thanks for working on this. Code looks fine on a first glance.

Regarding your open issues:

What should happen if this command is run outside of a sandbox.

The presence/absence of a sandbox shouldn't affect anything except the choice of package DB, so I don't see a problem here. If there's no .cabal file in the current directory, we should just exit with an error.

Whether or not cabal.config can be entirely overwritten.
Regarding the last point, @tibbe has stated that it should not, whilst @gregwebs is of the opinion that it is fine to do so.

I'm with @tibbe on this.

The fact that base and other GHC-bundled libraries get included in the list of dependencies is IMO a good thing since we can't guarantee that the package will build with a different version of GHC or base. People using freeze for deployment should standardise on a single GHC version both for development and production. People that want to use freeze with several different GHC versions presumably know what they are doing and can edit cabal.config by hand, though adding platform-dependent constraints would be a nice extension.

@gregwebs
Copy link

Of course I am fine with just overriding the constraints section. I just sounds like a lot of wasted effort for a non-ideal solution. It is less effort to write out a cabal.freeze. Of course you then need to have the code that reads the cabal.config constraints section also read cabal.freeze, but that doesn't sound very hard either and this is all forwards progress, not temporary hacks to be later erased.

But either is ok with me :)

@23Skidoo
Copy link
Member

I'm fine with overwriting if we decide to use a separate config.freeze file (but note that a smart update feature will still be required if/when we decide to implement platform-dependent constraints).

@benarmston
Copy link
Contributor Author

A brief look at how loading constraints from cabal.freeze might work confirmed that @gregwebs was correct in thinking that it wouldn't be hard to implement. So I went ahead and did so.

cabal.freeze currently uses the same format as cabal.config but as the file is entirely managed by cabal-install it should be OK to rewrite it in some other format should that become necessary. Keeping support for reading the file in either its current format or some new format doesn't sound like it should be too burdensome. Try parsing it in one format if that fails try the other.

To support upgrading dependencies we would need to be able to load the package environment without the freeze file's constraints being included. Implementing that shouldn't require much more than passing some flags down to tryLoadSandboxPackageEnvironmentFile and using loadUserConfig instead of loadUserAndFreezeConfig.

Unless I'm missing something, I think this is the way to go.

@tibbe
Copy link
Member

tibbe commented Sep 26, 2013

I don't know if this is a good idea. As you discovered, we need to figure out how this new file interacts with the other files and in particular how the user is now supposed to change the list of dependencies, now when we have a feel the user isn't supposed to edit!

What's the supposed workflow for freezing dependencies and then subsequently change them?

@benarmston
Copy link
Contributor Author

First build of the application:

cabal install --only-dependencies
cabal freeze
git add cabal.freeze
git commit -m "Froze all dependencies"

Repeatable builds across the team

Another user pulls the git repo and installs the frozen dependencies:

cabal install --only-dependencies

Upgrading a direct dependency

A new version of foo is released. It has new functionality which I want and
I'm not concerned about remaining compatible with previous versions.

  1. Change e.g., foo == 1.0.* to foo == 1.1.*
  2. cabal install --upgrade-dependencies --ignore-all-frozen which performs a conservative upgrade ignoring the contents of cabal.freeze.
  3. git status
modified: cabal.freeze
modified: application.cabal
  1. git diff to view all dependencies of dependencies changed in cabal.freeze.
  2. cabal test
  3. git commit -am "Updated foo dependency"

Upgrading an indirect dependency

A new version of bar is released. It contains an important bug fix.

  1. Optionally, blacklist the buggy version of bar in cabal.config or application.config.
  2. cabal install --upgrade-dependencies --ignore-frozen bar which performs a conservative upgrade ignoring only the constraint for bar specified in cabal.freeze, but not any other constraint (including constraints for bar given elsewhere).
  3. git status
modified: cabal.freeze
  1. git diff to view all dependencies of dependencies changed in cabal.freeze.
  2. cabal test
  3. git commit -am "Updated bar dependency. Fixes important bug"

What's missing

We don't yet have either --ignore-all-frozen or --ignore-frozen, but I don't think that they would be too difficult to implement.

I'm assuming in this that --upgrading-dependencies will not be overly constrained by the packages which are already installed, will warn of potential breakages, and allow for reinstallations of necessary packages. If used in conjunction with sandboxes I think that this assumption is correct.

Ninja edit: changed application.freeze to application.cabal. Expanded on indirect dependency steps.

@tibbe
Copy link
Member

tibbe commented Sep 26, 2013

  1. Change e.g., foo == 1.0.* to foo == 1.1.*

I think this is wrong. The .cabal is for saying what you can build this package with, the freeze file (whether it's cabal.config or cabal.freeze) is for saying what you want to build with. They might often be the same, but not necessarily so. That is why we need to put the constraints in cabal.config, it's a user-editable file with the purpose of talking about what you want to build a particular package with.

@gregwebs
Copy link

The user changes dependencies in the .cabal file. It they want to upgrade for example, they put a new lower bound in the .cabal file.

Just running the cabal freeze command at this point should be able to re-freeze the dependencies with the new constraints in the cabal file. This process could be shell-scripted as:

  1. If the existing frozen dependencies are not installed, install them (a hack to produce a conservative upgrade)
  2. rm cabal.freeze
  3. The same cabal freeze that would be performed for an initial freeze. Equivalent to cabal install --only-dependencies && writing out cabal.freeze

Obviously we don't want to actually delete cabal.freeze, but running cabal freeze needs to ignore an existing cabal.freeze if it violates the .cabal file. If cabal.freeze does not violate the cabal file then there is nothing to do.

@gregwebs
Copy link

oh, whoops, I was offline for a while, responding to tibbe's original comment today

@benarmston
Copy link
Contributor Author

A new version of foo is released. It has new functionality which I want and
I'm not concerned about remaining compatible with previous versions.

  1. Change e.g., foo == 1.0.* to foo == 1.1.*

I think this is wrong. The .cabal is for saying what you can build this package with

But the use case stated that the new functionality is wanted and backwards
compatibility is not desired. With those requirements, the package cannot be built with foo == 1.0.*.

Let's add a new use case to cover your complaint.

Increase upper bound for a direct dependency

A new version of baz is released. Our application is to remain compatible with the previous version but is to also admit the new version.

  1. Change e.g., baz >= 1.0.0 && < 1.1 to baz >= 1.0.0 && < 1.2.
  2. cabal install --upgrade-dependencies --ignore-all-frozen which performs a conservative upgrade ignoring the contents of cabal.freeze.
  3. git status
 modified: cabal.freeze
 modified: application.cabal
  1. git diff to view all dependencies of dependencies changed in cabal.freeze.
  2. cabal test
  3. git commit -am "Updated bar dependency"

@benarmston
Copy link
Contributor Author

I mostly like the idea of refreezing by rerunning cabal freeze and having it ignore cabal.freeze. But freezing something that is already frozen and having it change seems surprising. I think having cabal freeze take the options --ignore-all-frozen and --ignore-frozen <pkg> would be clearer.

So my examples would change so that

cabal install --upgrade-dependencies --ignore-all-frozen

becomes

cabal freeze --ignore-all-frozen
cabal install --upgrade-dependencies

And similar for --ignore-frozen <pkg>.

@gregwebs
Copy link

my comment is still relevant, we don't need to require --upgrade-dependencies and --ignore-frozen flags, the user can just run cabal freeze as a smart default.

@tibbe I think we all agree that we should keep the cabal.config file constraints field so that the user can manually add specific constraints there. There is no reason for an application developer to use cabal.config constraints (and have to manually edit 2 files, they should only manually edit 1 file). But I am sure it will still be useful for distributing libraries or open source applications.

@gregwebs
Copy link

@tibbe To clarify the point further, you are properly advocating for the Library section of the .cabal file.

As an application developer at work I am not interested in listing lower version ranges that an application can be built with. Downgrades are rare other than immediately after a deploy, in which case I can just look at the previous commit of the .cabal file and revert it. The .cabal file can contain larger upper bounds that can indicate what could be built by upgrading versions. But if I really want that kind of information I can always just put a comment in the .cabal file so that the information is all in one place rather than spreading it across 2 files.

As a distributor of an open source application, I may want to indicate all the versions that my application could be built with in the .cabal file.

As a distributor of a library, I definitely want to indicate all versions that my application can be built with, and use cabal.config as a development tool to reproduce bugs.

@benarmston
Copy link
Contributor Author

@gregwebs you're right about not requiring the --upgrade-dependencies flag in my examples. But without an --ignore-frozen flag of some kind how would you upgrade a particular indirect dependency? If all of the constraints in cabal.freeze are ignored the refreeze will not be as conservative as it could be. If the build fails or if tests start failing it may be harder than it needs to be to find out why.

@gregwebs
Copy link

@benarmston I agree it isn't going to be perfect and that we will need some advanced options, I am advocating for a default.

My understanding is that since the dependencies are already installed, it will essentially be using cabal.freeze, but again I know that isn't a perfect process.

The cabal freeze (upgrade) command will write out the new cabal.freeze immediately, so when builds and tests are run next git diff will show exactly what had changed.

@benarmston
Copy link
Contributor Author

@gregwebs I think then that we can agree then that we need both a way to upgrade everything and a way to upgrade just a specific set of packages. Let's leave disagreements about what the default should be until we've decided on whether there should be a cabal.freeze file.

@benarmston
Copy link
Contributor Author

@tibbe I think I may have missed the point of your complaint. Was it about the equality constraint rather than marking an old version no longer compatible? If so, you're right that equality constraints shouldn't be used in a .cabal file, but I don't think I did.

My understanding is that foo == x.y.* is expanded to foo >= x.y.0 && < x.(y+1). I could have written this out in full:

  1. Change e.g., foo >= 1.0.0 && < 1.1 to foo >= 1.1.0 && < 1.2.

I think that is a perfectly valid change to a .cabal file when a new version of a library is released containing new functionality that we want to use and don't wish to try and maintain backwards compatibility with previous versions.

I'm primarily a proprietary-application developer rather than a library developer, and choosing not to maintain backwards compatibility with versions of a library I'm not using is a common choice. For open source applications and library developers there are obviously different dynamics at play.

Perhaps it would be helpful if we collected the use cases we wished to support with this feature. We'd then be able to talk with an explicit common focus in mind. As it stands at the moment, I think that the implicit differences in our expectations of how cabal is used and how this feature will be useful are hindering the conversation.

@tibbe
Copy link
Member

tibbe commented Sep 27, 2013

Perhaps taking a step back will help. There are two people involved in this process, the package author and the package builder. Sometimes they're the same person, but not necessarily so.

  • The author is the only one that can make permanent changes to the .cabal file. We're not changing this.
  • Since the builder cannot permanently change the .cabal file, there must be another user-editable file for him/her to make changes.

What is this user-editable file that the builder can edit to influence which dependencies are used? It's cabal.config. (Note that even if the builder could change the main .cabal file, that wouldn't be enough as that file only talks about direct dependencies, while we also want to freeze indirect dependencies.)

Here's an example that I hope shows why cabal.config must be the file the builder modifies to influence which packages and versions are used:

  • The .cabal file specifies a foo >= 1.0 && < 1.1 dependency.
  • The builder wants to pick a specific version in this range (not necessarily the latest version). Lets say 1.0.5.
  • The builder wants to persist this choice for the rest of his/her team.

It is not enough to to now run cabal freeze and only check in the cabal.freeze file, because another user that doesn't have the version of cabal.config used when running cabal freeze might not get the same result from the dependency solver. Therefore cabal.config must contained the desired versions and be checked in. It's the only file the builder can use to influence which versions are used in a reproducible way.

Temporarily changing the .cabal file and running freeze does not work. Since the new .cabal file is not necessarily checked in (if the builder is not also the author), another run of cabal freeze on another machine will not yield the same results.

@tibbe
Copy link
Member

tibbe commented Sep 27, 2013

I'm primarily a proprietary-application developer rather than a library developer, and choosing not to maintain backwards compatibility with versions of a library I'm not using is a common choice. For open source applications and library developers there are obviously different dynamics at play.

I suspect that this is the root of the confusion. For both you and Greg builder == author, we also support builder != author and that only works if there's a user-editable file other than the .cabal file where version constraints are specified and checked into source control. If you only check in a machine-generate cabal.freeze file that's overwritten each time, there's no place for the builder to make the choices of versions permanent. Since there are two people involved in the process of selecting dependencies (author + builder), it follows that there must be two user-editable files (that are checked in to source control).

@gregwebs
Copy link

  • cabal.config for author != builder
  • cabal.freeze for author == builder

Another way to state the distinction

  • distributing source code
  • distributing binaries

We completely understand that the industry use case of building binaries is not going to be appropriate for the open source use case of distributing code. Nobody is opposing using cabal.config. We have gone to a lot of effort to explain why forcing cabal.config on industry users is far from ideal.

@benarmston and I need to continue down this path or abandon cabal as the primary build tool as much of industry does. We would really appreciate your help in making sure what we are doing will also make distributing source code a more pleasant experience.

So lets figure out how to support both use cases. If we can start by agreeing we want to support both to the fullest extent possible and that using cabal.config is not sufficient for industry users building binaries, we can move forward with this.

@tibbe
Copy link
Member

tibbe commented Sep 27, 2013

  • cabal.config for author != builder
  • cabal.freeze for author == builder

I think this complicates things:

  • Users will need to understand what they should edit when and understand how the plethora of config files (.cabal, cabal.sandbox.config, cabal.config, cabal.freeze) interact.
  • The implementation is similarly complicated. Just to give an example among many: configure now needs to check if cabal.freeze has been updated and if so reconfigure.

You're also not getting any expressive power for this extra complication. If you insist you can just edit the .cabal file, run freeze and have the cabal.config file be completely managed by Cabal (since you're not making any manual changes to it).

So lets figure out how to support both use cases. If we can start by agreeing we want to support both to the fullest extent possible and that using cabal.config is not sufficient for industry users building binaries, we can move forward with this.

I hate to disappoint, but Cabal is an opinionated piece of software. I'm happy to support alternative use cases, as long as I don't have to compromise on the "happy path" we're designing for the UI. I was about to merge the patches before cabal.freeze was added.

@gregwebs
Copy link

Users will need to understand what they should edit when and understand how the plethora of config files (.cabal, cabal.sandbox.config, cabal.config, cabal.freeze) interact.
You're also not getting any expressive power for this extra complication. If you insist you can just edit the .cabal file, run freeze and have the cabal.config file be completely managed by Cabal (since you're not making any manual changes to it).

I completely disagree that fewer files == less complexity. It is much simpler for the implementer and user if cabal.freeze is machine generated (as a comment at the top will indicate) and if cabal.config is user edited.

The implementation is similarly complicated. Just to give an example among many: configure now needs to check if cabal.freeze has been updated and if so reconfigure.

Why is this complicated? Doesn't it already do this for cabal.config?
I would rather merge cabal.config constraints and cabal.freeze than deal with the impossible case of a user editing a cabal.config and then running cabal.freeze

I hate to disappoint, but Cabal is an opinionated piece of software. I'm happy to support alternative use cases, as long as I don't have to compromise on the "happy path" we're designing for the UI. I was about to merge the patches before cabal.freeze was added.

Please keep merging patches, including this one. This has not been a "happy path" for me. Unfortunately I am far beyond the time I had imagined it would take to discuss this feature.

@benarmston I will leave the rest of the discussion up to you.

@ygale
Copy link
Contributor

ygale commented Oct 11, 2013

I am very pleased to see progress on this important issue. Thanks to all! Our needs have been well represented by @gregwebs; I hope the feature will be as close as possible to what he has described in this thread.

There has been discussion of the need for --ignore-all-frozen and --ignore-frozen flags, as suggested by @benarmston. Regardless of what is decided on that point, it still makes sense also to implement a simpler and more general --ignore-constraint flag. See #949.

I need to manage cabal projects with very large and complex indirect dependency graphs - for example, those that tend to arise for complex web apps, with ~150 nodes, and more than an order of magnitude more nodes in the graph considered by the solver. And I need reproducible builds; and also feature-equivalent builds with different GHC versions. Until now, the tools provided by cabal were just not sufficient, and the work was difficult and time consuming. These new features, together with other features already added such as constraints in local cabal.config files, will be a vast improvement. Thank you!

@tibbe
Copy link
Member

tibbe commented Oct 11, 2013

@benarmston Are you OK with changing this back to not use the extra cabal.freeze file? If so I'm happy to merge it.

@benarmston
Copy link
Contributor Author

@tibbe I've been trying to find the time to write some use cases to convince you that using a cabal.freeze file is the better option. Unfortunately, that time has not materialized in the last couple of weeks.

If I can't find the time this week, or if I can but fail to convince you, I'll change this back to not use cabal.freeze. How does that sound?

@tibbe
Copy link
Member

tibbe commented Oct 14, 2013

@benarmston Sounds good to me. I will be out for a week, starting Wednesday, so I might be a bit slow to respond.

@sol
Copy link
Member

sol commented Oct 27, 2013

Personally, I think cabal.freeze is the better solution. But then again, for me it's most important that we have a solution some time soon (as currently you can't do live deploys with cabal unless you have some custom solution in place that locks dependencies).

Regarding opinionated, UI, etc., for me this often times boils down to personal taste (and we had similar discussions here many times). For me the core issue here is that the core infrastructure components (GHC/Cabal/Hackage/Haddock...) are too closely tied together which makes it inherently hard to fork and try new ideas (which by extension hampers innovation).

@tibbe
Copy link
Member

tibbe commented Nov 11, 2013

@benarmston have you found any time to look into this? I think it's a really important feature for 1.20.

Add new top-level `freeze` command, which resolves the dependencies to exact
versions and writes a `constraints` section to `cabal.config`. This causes
future builds to use the same fully constrained dependencies.

The command takes a number of options related to resolving dependencies,
namely, `--solver`, `--max-backjumps`, `reorder-goals` and
`--shadow-installed-packages`. These are used to create an `InstallPlan` in
much the same way that `install` does so. The `InstallPlan` is converted to a
list and all `PlanPackage`s are inspected to determine their exact version.
These versions are then either written to `cabal.config` or to standard output
depending on the presence of `--dry-run`.

Limitations in resolving dependencies
-------------------------------------

In order to keep the initial implementation of this new command simpler, a
number of options are not yet supported.  There should be no great difficulty
in supporting the options `--flags`, `--enable-{tests,benchmarks}`,
`--constraint` and `--preference`.  However, the options concerned with
compilers may prove more difficult.

Different versions of GHC ship with different library versions, the
constraints that are written currently include all dependencies, including
`base`. This prevents the constraints, as written, from being used with
alternate versions of GHC.

There are three solutions to this problem: 1) have the user edit the
constraints section, 2) exclude certain packages from the list of constraints,
3) develop a mechanism for per-arch-os-compiler constraining. As neither (2)
nor (3) have been developed we default to (1).

The lack of a good story for per-compiler constraints has lead to the options
`--with-compiler`, `--ghc`, `--uhc` et al, not being supported.

Further limitations:
--------------------

 - The `cabal.config` file is completely overwritten. Just the `constraints`
   section should be overwritten.
@benarmston
Copy link
Contributor Author

@tibbe I apologise for having taken so long to respond to you on this.

I have found only a little time to work on this over the last month or so. As previously agreed upon, although not in the suggested time scales, I've reverted the commit introducing the use of cabal.freeze. I've also rebased on to the master branch to make sure that the changes still apply cleanly, and squashed a fixup commit.

I agree with you, this is an important feature for 1.20. So I'd like to remove any obstacles for getting this branch merged, and ensuring that some form of freezing is included. Then any discussion on whether to use a cabal.freeze file can continue separately, either on this issue or another.

You had written previously that you were close to merging the commit in this state, i.e., pre cabal.freeze. Is that still the case, or is clobbering the cabal.config file going to prove problematic?

Regarding the work I'd already done on using a cabal.freeze file, as a number of people have expressed a preference to that, I shall push another branch containing those commits.

@tibbe tibbe merged commit c6138e8 into haskell:master Dec 14, 2013
@tibbe
Copy link
Member

tibbe commented Dec 14, 2013

@benarmston Thanks for taking the time to do the changes. I've merged the commit so it will be a part of 1.20, expected early next year.

I will file a separate bug for making cabal freeze more convenient by only updating the constraints section.

@tibbe
Copy link
Member

tibbe commented Dec 14, 2013

Filed issue #1615.

@mietek
Copy link
Contributor

mietek commented Dec 10, 2014

I mostly like the idea of refreezing by rerunning cabal freeze and having it ignore [cabal.config]. But freezing something that is already frozen and having it change seems surprising.

@benarmston: Can you confirm that cabal freeze always ignores any constraints in the existing cabal.config?

Continuing in #2265.

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.

7 participants