-
Notifications
You must be signed in to change notification settings - Fork 841
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
[WIP] Lock files #4550
[WIP] Lock files #4550
Changes from 77 commits
3be0c5a
889059c
7abafb5
d10af00
d1d9138
1ec1a64
d20749b
e0ee3ba
3bbc3d1
71668fc
b8e56cf
bad1443
d099cc4
a9a8268
da47898
fb3446c
c868f6a
446b147
c76b1f6
325c7d0
e4cd0ab
79659e9
7c4d1b5
8958031
624ffaf
cd9b7a9
b2553cb
4ea05bf
c05fc59
dad2b16
d211d2a
536929c
c0467f5
1a6ada2
4060499
70db768
6d05164
d6cba4d
47573a0
da27d94
6533f97
6a5c179
fd6f0fb
ed4e800
c99ecf5
f688564
016b16d
86ff18c
25e3427
8e11c8a
fac02b0
acf2e04
84940c2
6d8cc54
378f3c3
d755d25
1afa890
f168499
d463b76
f6559db
a7e6c25
a419587
d284f10
1805828
ed5e41d
27f3a88
28921f5
89da1ef
10ad675
02efbec
6e0be06
6e05cd0
246ff8e
5819514
c662491
281ed27
6149c75
72b1867
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,6 +87,14 @@ import System.PosixCompat.Files (fileOwner, getFileStatus) | |
import System.PosixCompat.User (getEffectiveUserID) | ||
import RIO.PrettyPrint | ||
import RIO.Process | ||
import Stack.Lock | ||
( LockFile(..) | ||
, generatePackageLockFile | ||
, generateSnapshotLayerLockFile | ||
, isLockFileOutdated | ||
, loadPackageLockFile | ||
, loadSnapshotLayerLockFile | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need such a large multiline import list? Why not just everything from |
||
|
||
-- | If deprecated path exists, use it and print a warning. | ||
-- Otherwise, return the new path. | ||
|
@@ -513,6 +521,18 @@ loadConfig :: HasRunner env | |
loadConfig configArgs mresolver mstackYaml inner = | ||
loadProjectConfig mstackYaml >>= \x -> loadConfigMaybeProject configArgs mresolver x inner | ||
|
||
cachedCompletePackageLocation :: (HasPantryConfig env, HasLogFunc env, HasProcessContext env) => Map RawPackageLocation PackageLocation | ||
-> RawPackageLocation | ||
-> RIO env PackageLocation | ||
cachedCompletePackageLocation cachePackages rp@(RPLImmutable rpli) = do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor naming suggestion: what about |
||
let item = Map.lookup rp cachePackages | ||
case item of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what if |
||
Nothing -> do | ||
pl <- completePackageLocation rpli | ||
pure $ PLImmutable pl | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe in 1 line as |
||
Just pl -> pure pl | ||
cachedCompletePackageLocation _ (RPLMutable rplm) = pure $ PLMutable rplm | ||
|
||
-- | Load the build configuration, adds build-specific values to config loaded by @loadConfig@. | ||
-- values. | ||
loadBuildConfig :: LocalConfigStatus (Project, Path Abs File, ConfigMonoid) | ||
|
@@ -587,8 +607,37 @@ loadBuildConfig mproject maresolver mcompiler = do | |
{ projectResolver = fromMaybe (projectResolver project') mresolver | ||
} | ||
|
||
resolver <- completeSnapshotLocation $ projectResolver project | ||
(snapshot, _completed) <- loadAndCompleteSnapshot resolver | ||
lockFileOutdated <- isLockFileOutdated stackYamlFP | ||
if lockFileOutdated | ||
then do | ||
logDebug "Lock file is outdated or isn't present" | ||
generatePackageLockFile stackYamlFP | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We go through the FS to read the lock file to see if we can avoid doing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see no reasons not to reuse already completed locations without going through FS |
||
else logDebug "Lock file is upto date" | ||
|
||
lockFile <- liftIO $ addFileExtension "lock" stackYamlFP | ||
(cachedPackageLock, origResolver, compResolver) <- liftIO $ do | ||
lf <- loadPackageLockFile lockFile | ||
return (lfPackageLocations lf, lfoResolver lf, lfcResolver lf) | ||
|
||
|
||
resolver <- if projectResolver project == origResolver | ||
then pure compResolver | ||
else completeSnapshotLocation $ projectResolver project | ||
|
||
case resolver of | ||
SLFilePath path -> do | ||
outdated <- isLockFileOutdated (resolvedAbsolute path) | ||
when outdated (generateSnapshotLayerLockFile resolver stackYamlFP) | ||
_ -> return () | ||
|
||
cachedSnapshotLock <- case resolver of | ||
SLFilePath path -> do | ||
let sf = resolvedAbsolute path | ||
slf <- liftIO $ addFileExtension "lock" sf | ||
liftIO $ loadSnapshotLayerLockFile slf (parent stackYamlFP) | ||
_ -> pure Map.empty | ||
|
||
(snapshot, _completed) <- loadAndCompleteSnapshot resolver cachedSnapshotLock | ||
|
||
extraPackageDBs <- mapM resolveDir' (projectExtraPackageDBs project) | ||
|
||
|
@@ -600,11 +649,8 @@ loadBuildConfig mproject maresolver mcompiler = do | |
pp <- mkProjectPackage YesPrintWarnings resolved (boptsHaddock bopts) | ||
pure (cpName $ ppCommon pp, pp) | ||
|
||
let completeLocation (RPLMutable m) = pure $ PLMutable m | ||
completeLocation (RPLImmutable im) = PLImmutable <$> completePackageLocation im | ||
|
||
deps0 <- forM (projectDependencies project) $ \rpl -> do | ||
pl <- completeLocation rpl | ||
pl <- cachedCompletePackageLocation cachedPackageLock rpl | ||
dp <- additionalDepPackage (shouldHaddockDeps bopts) pl | ||
pure (cpName $ dpCommon dp, dp) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,77 +1,84 @@ | ||
{-# LANGUAGE NoImplicitPrelude #-} | ||
{-# LANGUAGE OverloadedStrings #-} | ||
{-# LANGUAGE ScopedTypeVariables #-} | ||
|
||
module Stack.Freeze | ||
( freeze | ||
, FreezeOpts (..) | ||
, FreezeMode (..) | ||
, FreezeOpts(..) | ||
, FreezeMode(..) | ||
) where | ||
|
||
import Data.Aeson ((.=), object) | ||
import Data.Aeson ((.=), object) | ||
import qualified Data.Yaml as Yaml | ||
import RIO.Process | ||
import qualified RIO.ByteString as B | ||
import Stack.Prelude | ||
import Stack.Types.Config | ||
import RIO.Process | ||
import Stack.Prelude | ||
import Stack.Types.Config | ||
|
||
data FreezeMode = FreezeProject | FreezeSnapshot | ||
data FreezeMode | ||
= FreezeProject | ||
| FreezeSnapshot | ||
|
||
newtype FreezeOpts = FreezeOpts | ||
{ freezeMode :: FreezeMode | ||
} | ||
|
||
freeze :: HasEnvConfig env => FreezeOpts -> RIO env () | ||
freeze (FreezeOpts mode) = do | ||
mproject <- view $ configL.to configMaybeProject | ||
case mproject of | ||
Just (p, _) -> doFreeze p mode | ||
Nothing -> logWarn "No project was found: nothing to freeze" | ||
mproject <- view $ configL . to configMaybeProject | ||
case mproject of | ||
Just (p, _) -> doFreeze p mode | ||
Nothing -> logWarn "No project was found: nothing to freeze" | ||
|
||
completeFullPackageLocation :: | ||
(HasPantryConfig env, HasLogFunc env, HasProcessContext env) | ||
=> RawPackageLocation | ||
-> RIO env PackageLocation | ||
completeFullPackageLocation (RPLImmutable rpli) = do | ||
pl <- completePackageLocation rpli | ||
pure $ PLImmutable pl | ||
completeFullPackageLocation (RPLMutable rplm) = pure $ PLMutable rplm | ||
|
||
doFreeze :: | ||
(HasProcessContext env, HasLogFunc env, HasPantryConfig env) | ||
( HasProcessContext env | ||
, HasLogFunc env | ||
, HasPantryConfig env | ||
, HasEnvConfig env | ||
) | ||
=> Project | ||
-> FreezeMode | ||
-> RIO env () | ||
doFreeze p FreezeProject = do | ||
let deps = projectDependencies p | ||
resolver = projectResolver p | ||
completePackageLocation' pl = | ||
case pl of | ||
RPLImmutable pli -> PLImmutable <$> completePackageLocation pli | ||
RPLMutable m -> pure $ PLMutable m | ||
resolver' <- completeSnapshotLocation resolver | ||
deps' <- mapM completePackageLocation' deps | ||
let rawCompleted = map toRawPL deps' | ||
rawResolver = toRawSL resolver' | ||
if rawCompleted == deps && rawResolver == resolver | ||
then | ||
logInfo "No freezing is required for this project" | ||
else do | ||
logInfo "# Fields not mentioned below do not need to be updated" | ||
|
||
if rawResolver == resolver | ||
then logInfo "# No update to resolver is needed" | ||
else do | ||
logInfo "# Frozen version of resolver" | ||
B.putStr $ Yaml.encode $ object ["resolver" .= rawResolver] | ||
|
||
if rawCompleted == deps | ||
then logInfo "# No update to extra-deps is needed" | ||
else do | ||
logInfo "# Frozen version of extra-deps" | ||
B.putStr $ Yaml.encode $ object ["extra-deps" .= rawCompleted] | ||
|
||
let deps :: [RawPackageLocation] = projectDependencies p | ||
resolver :: RawSnapshotLocation = projectResolver p | ||
resolver' :: SnapshotLocation <- completeSnapshotLocation resolver | ||
deps' :: [PackageLocation] <- mapM completeFullPackageLocation deps | ||
let rawCompleted = map toRawPL deps' | ||
rawResolver = toRawSL resolver' | ||
if rawCompleted == deps && rawResolver == resolver | ||
then logInfo "No freezing is required for this project" | ||
else do | ||
logInfo "# Fields not mentioned below do not need to be updated" | ||
if rawResolver == resolver | ||
then logInfo "# No update to resolver is needed" | ||
else do | ||
logInfo "# Frozen version of resolver" | ||
B.putStr $ Yaml.encode $ object ["resolver" .= rawResolver] | ||
if rawCompleted == deps | ||
then logInfo "# No update to extra-deps is needed" | ||
else do | ||
logInfo "# Frozen version of extra-deps" | ||
B.putStr $ | ||
Yaml.encode $ object ["extra-deps" .= rawCompleted] | ||
doFreeze p FreezeSnapshot = do | ||
resolver <- completeSnapshotLocation $ projectResolver p | ||
result <- loadSnapshotLayer resolver | ||
case result of | ||
Left _wc -> | ||
logInfo "No freezing is required for compiler resolver" | ||
Right (snap, _) -> do | ||
snap' <- completeSnapshotLayer snap | ||
let rawCompleted = toRawSnapshotLayer snap' | ||
if rawCompleted == snap | ||
then | ||
logInfo "No freezing is required for the snapshot of this project" | ||
else | ||
liftIO $ B.putStr $ Yaml.encode snap' | ||
resolver <- completeSnapshotLocation $ projectResolver p | ||
result <- loadSnapshotLayer resolver | ||
case result of | ||
Left _wc -> logInfo "No freezing is required for compiler resolver" | ||
Right (snap, _) -> do | ||
snap' <- completeSnapshotLayer snap | ||
let rawCompleted = toRawSnapshotLayer snap' | ||
if rawCompleted == snap | ||
then logInfo | ||
"No freezing is required for the snapshot of this project" | ||
else liftIO $ B.putStr $ Yaml.encode snap' |
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.
could you explain this change? Why do we use
CFILatest
when we have a particular blob key?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.
This change is for the suggestions part. Without this, if a package is not in the snapshot - The stack build tool will recommend it to be added like this:
With the above change, it will recommend it like this:
The reason I made that change was because of Michael's comment in the issue:
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.
But why not change the suggestion properly instead? This change above complicates the logic a bit an gives no hints about reasons