-
Notifications
You must be signed in to change notification settings - Fork 701
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
New freeze command #3503
Merged
Merged
New freeze command #3503
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,164 @@ | ||
{-# LANGUAGE NamedFieldPuns, RecordWildCards #-} | ||
|
||
-- | cabal-install CLI command: freeze | ||
-- | ||
module Distribution.Client.CmdFreeze ( | ||
freezeAction, | ||
) where | ||
|
||
import Distribution.Client.ProjectPlanning | ||
( ElaboratedInstallPlan, rebuildInstallPlan ) | ||
import Distribution.Client.ProjectConfig | ||
( ProjectConfig(..), ProjectConfigShared(..) | ||
, commandLineFlagsToProjectConfig, writeProjectLocalFreezeConfig | ||
, findProjectRoot ) | ||
import Distribution.Client.ProjectPlanning.Types | ||
( ElaboratedConfiguredPackage(..) ) | ||
import Distribution.Client.Targets | ||
( UserConstraint(..) ) | ||
import Distribution.Solver.Types.ConstraintSource | ||
( ConstraintSource(..) ) | ||
import Distribution.Client.DistDirLayout | ||
( defaultDistDirLayout, defaultCabalDirLayout ) | ||
import Distribution.Client.Config | ||
( defaultCabalDir ) | ||
import qualified Distribution.Client.InstallPlan as InstallPlan | ||
|
||
|
||
import Distribution.Package | ||
( PackageName, packageName, packageVersion ) | ||
import Distribution.Version | ||
( VersionRange, thisVersion, unionVersionRanges ) | ||
import Distribution.PackageDescription | ||
( FlagAssignment ) | ||
import Distribution.Client.Setup | ||
( GlobalFlags, ConfigFlags(..), ConfigExFlags, InstallFlags ) | ||
import Distribution.Simple.Setup | ||
( HaddockFlags, fromFlagOrDefault ) | ||
import Distribution.Simple.Utils | ||
( die, notice ) | ||
import Distribution.Verbosity | ||
( normal ) | ||
|
||
import Data.Monoid as Monoid | ||
import qualified Data.Map as Map | ||
import Data.Map (Map) | ||
import Control.Monad (unless) | ||
import System.FilePath | ||
|
||
|
||
-- | To a first approximation, the @freeze@ command runs the first phase of | ||
-- the @build@ command where we bring the install plan up to date, and then | ||
-- based on the install plan we write out a @cabal.project.freeze@ config file. | ||
-- | ||
-- For more details on how this works, see the module | ||
-- "Distribution.Client.ProjectOrchestration" | ||
-- | ||
freezeAction :: (ConfigFlags, ConfigExFlags, InstallFlags, HaddockFlags) | ||
-> [String] -> GlobalFlags -> IO () | ||
freezeAction (configFlags, configExFlags, installFlags, haddockFlags) | ||
extraArgs globalFlags = do | ||
|
||
unless (null extraArgs) $ | ||
die $ "'freeze' doesn't take any extra arguments: " | ||
++ unwords extraArgs | ||
|
||
cabalDir <- defaultCabalDir | ||
let cabalDirLayout = defaultCabalDirLayout cabalDir | ||
|
||
projectRootDir <- findProjectRoot | ||
let distDirLayout = defaultDistDirLayout projectRootDir | ||
|
||
let cliConfig = commandLineFlagsToProjectConfig | ||
globalFlags configFlags configExFlags | ||
installFlags haddockFlags | ||
|
||
|
||
(_, elaboratedPlan, _, _) <- | ||
rebuildInstallPlan verbosity | ||
projectRootDir distDirLayout cabalDirLayout | ||
cliConfig | ||
|
||
let freezeConfig = projectFreezeConfig elaboratedPlan | ||
writeProjectLocalFreezeConfig projectRootDir freezeConfig | ||
notice verbosity $ | ||
"Wrote freeze file: " ++ projectRootDir </> "cabal.project.freeze" | ||
|
||
where | ||
verbosity = fromFlagOrDefault normal (configVerbosity configFlags) | ||
|
||
|
||
|
||
-- | Given the install plan, produce a config value with constraints that | ||
-- freezes the versions of packages used in the plan. | ||
-- | ||
projectFreezeConfig :: ElaboratedInstallPlan -> ProjectConfig | ||
projectFreezeConfig elaboratedPlan = | ||
Monoid.mempty { | ||
projectConfigShared = Monoid.mempty { | ||
projectConfigConstraints = | ||
concat (Map.elems (projectFreezeConstraints elaboratedPlan)) | ||
} | ||
} | ||
|
||
-- | Given the install plan, produce solver constraints that will ensure the | ||
-- solver picks the same solution again in future in different environments. | ||
-- | ||
projectFreezeConstraints :: ElaboratedInstallPlan | ||
-> Map PackageName [(UserConstraint, ConstraintSource)] | ||
projectFreezeConstraints plan = | ||
-- | ||
-- TODO: [required eventually] this is currently an underapproximation | ||
-- since the constraints language is not expressive enough to specify the | ||
-- precise solution. See https://github.com/haskell/cabal/issues/3502. | ||
-- | ||
-- For the moment we deal with multiple versions in the solution by using | ||
-- constraints that allow either version. Also, we do not include any | ||
-- constraints for packages that are local to the project (e.g. if the | ||
-- solution has two instances of Cabal, one from the local project and one | ||
-- pulled in as a setup deps then we exclude all constraints on Cabal, not | ||
-- just the constraint for the local instance since any constraint would | ||
-- apply to both instances). | ||
-- | ||
Map.unionWith (++) versionConstraints flagConstraints | ||
`Map.difference` localPackages | ||
where | ||
versionConstraints :: Map PackageName [(UserConstraint, ConstraintSource)] | ||
versionConstraints = | ||
Map.mapWithKey | ||
(\p v -> [(UserConstraintVersion p v, ConstraintSourceFreeze)]) | ||
versionRanges | ||
|
||
versionRanges :: Map PackageName VersionRange | ||
versionRanges = | ||
Map.fromListWith unionVersionRanges $ | ||
[ (packageName pkg, thisVersion (packageVersion pkg)) | ||
| InstallPlan.PreExisting pkg <- InstallPlan.toList plan | ||
] | ||
++ [ (packageName pkg, thisVersion (packageVersion pkg)) | ||
| InstallPlan.Configured pkg <- InstallPlan.toList plan | ||
] | ||
|
||
flagConstraints :: Map PackageName [(UserConstraint, ConstraintSource)] | ||
flagConstraints = | ||
Map.mapWithKey | ||
(\p f -> [(UserConstraintFlags p f, ConstraintSourceFreeze)]) | ||
flagAssignments | ||
|
||
flagAssignments :: Map PackageName FlagAssignment | ||
flagAssignments = | ||
Map.fromList | ||
[ (pkgname, flags) | ||
| InstallPlan.Configured pkg <- InstallPlan.toList plan | ||
, let flags = pkgFlagAssignment pkg | ||
pkgname = packageName pkg | ||
, not (null flags) ] | ||
|
||
localPackages :: Map PackageName () | ||
localPackages = | ||
Map.fromList | ||
[ (packageName pkg, ()) | ||
| InstallPlan.Configured pkg <- InstallPlan.toList plan | ||
, pkgLocalToProject pkg | ||
] | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC @tibbe was quite vehemently against having a separate
.freeze
file, that's why we only havecabal.config
and notcabal.freeze.config
in the standard path. See the discussion here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if any of the
new-*
commands is supposed to modifyproject.config
. I always assumed thatproject.config
is a purely manually edited file, as opposed tocabal.project.*
of whichcabal.project.local
andcabal.project.freeze
are instances...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well,
cabal.config
is also supposed to be manually editable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here's my theory that tries to unify what is otherwise an ad-hoc list of config files: the
cabal.project
file would contain ainclude cabal.project.*
directive, and certain commands like configure, freeze etc manage files that can be included. By default they would be included but we'd make that all completely explicit.So in future, a
cabal project init
might make a defaultcabal.project
file like:Having them as separate files resolves the problem of automatically editing manually managed files. It's not that it's technically a problem to edit the main
cabal.project
file, e.g. we could have commands to help do that like adding a new package into the list, but the problem that we don't know how to edit things because we cannot distinguish between manually specified things and generated things.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@23Skidoo so looking at the long discussion in that ticket, I'm not sure the objections there apply. It doesn't seem from that ticket that @tibbe really objects to having a separate file for freeze, but he's more concerned with UI complexity. Issues like, how do users know what files are involved, which ones do they edit etc. I think we can answer those questions quite clearly by using an explicit scheme like the above, where you can see how the top level project file includes others. Also, having comments in the relevant files will help make it clear.
It's true we have to make sure we're clear about the freeze workflow, but for the moment I think it's pretty much the same as before. We may want to add an unfreeze that just rm's the cabal.project.freeze file. I also don't foresee any problems with checking in the freeze file into source control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dcoutts Personally I'm not against having a separate freeze file, just wanted to make sure that @tibbe's objections from that discussion were taken into consideration.
Also, if the freeze file is not supposed to be manually editable, I guess it should include a top-level comment with a warning, just like
cabal.sandbox.config
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-reading that ticket I think @tibbe's complains are handled, here are the main ones:
By making the includes explicit it will be clear how the files interact: there's really just the cabal.project file but it can explicitly include other things. The new implementation already deals properly with tracking changes to the project files and reconfiguring correctly, so this isn't a problem.
I was just thinking about what the comment should be, e.g. we might want to explain that you can manage these manually by coping into the main file, but at the moment there's then no way to not include the generated one if it exists. So we may want to move forward on the explicit include approach so that we can provide a coherent story (including putting appropriate comments in the generated files).