Skip to content

Commit

Permalink
Better support for running scripts. (#7851)
Browse files Browse the repository at this point in the history
* Add support for script build caching to cabal run

Enable caching of script builds by changing the location of the fake
package directory from a tmp directory to:
<cabal_dir>/scipt-builds/abs/path/to/script/

Resolves: #6354
WIP: #7842

* Add support for scripts to cabal build.

Added module Distribution.Client.ScriptUtils for code to deal with
scripts that is common between commands.

WIP: #7842

* Add script support to cabal clean.

This changes the behaviour of cabal clean to accept extra args, which it
now interprets as script files. The behaviour of cabal clean is the same
when given extra args. When given extra args it instead removes the
caches for those scripts and also any orphaned caches (caches for which
the script no longer exists)

In addition this commit changes the cache to use hashes of paths because
this significantly simplifies the implementation of clean, and more
importantly it prevents collisions when a script has the name of the
subdirectory of a previously cached script.

WIP: #7842

* Add script support to cabal repl

repl starts in the correct directory and points directly to rather than
a dummy, so that reloading works properly.

There is a downside to the current approach which is that it uses a
different fake-project.cabal file from run and build, so it cannot share
the same cache with them.

WIP: #7842
WIP: #6149

* Added changelog for pr #7851

* Fix `cabal run script.hs` issue with --builddir

Fixes tests:
cabal-testsuite/PackageTests/NewBuild/CmdRun/Script/cabal.test.hs
cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptLiterate/cabal.test.hs

* Fixes for `build script` and `repl script`

- Fix build issue introduced in 079c5f0, where build was being passed
the wrong target filter
- Fix repl issue where script didn't work in a project context.
- Refactor code to share logic between repl and build/run
- Ensure temp directories are only created when needed

* Bug fixes relating to script support

ScriptUtils:
- Hash prefix for cache dirs was applied incorrectly.
- Overwriting fake-package files causes repeated work in some cases.
CmdClean:
- Clean distdir for script when --builddir is passed
- Always clean orphans because because there is no good way to specify
they should be cleaned. This may be bad behaviour in some obscure cases
(a cache is temporarily orphaned and an unrelated clean is run), but at
worst results in a cache rebuild.

* Add tests for improved script support

- Basic script support for build/repl/clean which checks for cached
project files
- Add check for cached project files to basic run script test
- No repeated work for build/build, build/run, run/run, and repl/repl
- Clean does not remove cache for existing scripts
- Clean does remove orphaned script caches

* Fix clean bug uncovered by 5fad121

- clean was trying to read source-builds even if it didn't exist
- add test specific to this case with other clean tests

* Update documentation for better script support

Ready for review: #7851
May close: #7842, #6354, #6149

* Attempt to fix `repl script` on Windows

PR #7851

* Attempt to fix remote test failures

Test logs showed that the failures where because the tests depended on a
module from cabal-install that some ghc versions could not find.

Instead of depending on cabal-install, I copied the needed function into
Test.Cabal.Prelude (It seemed like an acceptable place for it)

PR #7851

* Attempt to fix `repl script` on Windows

PR #7851

* Attempt to fix tests on old ghc versions

Tests failing on pre-AMP ghcs due to unsanctioned use of (<$>)

PR #7851

* Feedback: Update docs and formatting

PR #7851

* Feedback: code style changes

- remove partial selectors
- make a constant for fake-package.cabal

PR #7851

* Feedback: make hidden control flow explicit

PR #7851

* Feedback: add expected fail script run tests

PR #7851

* Fix `repl script` when cwd is deeper than cachedir

PR #7851

* Use script in-place for build or run

- Set the hs-source-dir to the location of the script for build and run,
  the same as with repl
- This removes the need to copy the script
- repl no longer needs a separate cache because all three commands
  use identical project files
- Adds multi-module support to scripts for free (#6787)
- Add new build/repl test and run multi-module test

PR #7851

* Fix file-locking issue on Windows

PR #7851

* Fix script recompilation based on cwd

- Pass info about cwd to repl through --repl-options instead of hacking
  it into the project file.
- Improve paths output by makeRelativeCanonical, makeRelativeToDir, and
  makeRelativeToCwd.
- Script multi-module support works, but with warning in repl.
- Remove script multi-module mention support in docs.

PR #7851

* Make `repl script` respect --repl-no-load

* Feedback: minor refactor

Move argument truncation from targetStrings out of
withScriptContextAndSelectors to runAction

PR #7851

* Feedback: refactor and comments for repl options

PR #7851

* Don't use hs-source-dirs for scripts.

- instead pass absolute path to script in main-is
- resolves issue with relative paths on Windows
- simplifies code and gives prettier build output
- update tests because build output has changed
- removes ability to use multi-module scripts
  (which was never officially endorsed)
- remove test for multi-module scripts
- add checks for unsupported fields in scripts

PR #7851

* Update changelog for PR #7851
  • Loading branch information
bacchanalia authored Dec 31, 2021
1 parent 0c77ed4 commit bbc11f1
Show file tree
Hide file tree
Showing 55 changed files with 905 additions and 352 deletions.
6 changes: 5 additions & 1 deletion Cabal/src/Distribution/Types/PackageName/Magic.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@ import Distribution.Types.Version
nonExistentPackageThisIsCabalBug :: PackageName
nonExistentPackageThisIsCabalBug = mkPackageName "nonexistent-package-this-is-a-cabal-bug"

-- | Used by @cabal new-repl@ and @cabal new-run@
-- | Used by @cabal new-repl@, @cabal new-run@ and @cabal new-build@
fakePackageName :: PackageName
fakePackageName = mkPackageName "fake-package"

-- | Used by @cabal new-run@ and @cabal new-build@
fakePackageCabalFileName :: FilePath
fakePackageCabalFileName = "fake-package.cabal"

-- | 'fakePackageName' with 'version0'.
fakePackageId :: PackageId
fakePackageId = PackageIdentifier fakePackageName version0
1 change: 1 addition & 0 deletions cabal-install/cabal-install.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ library
Distribution.Client.Sandbox
Distribution.Client.Sandbox.PackageEnvironment
Distribution.Client.SavedFlags
Distribution.Client.ScriptUtils
Distribution.Client.Security.DNS
Distribution.Client.Security.HTTP
Distribution.Client.Setup
Expand Down
16 changes: 8 additions & 8 deletions cabal-install/src/Distribution/Client/CmdBuild.hs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import Distribution.Verbosity
( normal )
import Distribution.Simple.Utils
( wrapText, die' )
import Distribution.Client.ScriptUtils
( AcceptNoTargets(..), withContextAndSelectors, updateContextAndWriteProjectFile, TargetContext(..) )

import qualified Data.Map as Map

Expand Down Expand Up @@ -95,19 +97,19 @@ defaultBuildFlags = BuildFlags
-- "Distribution.Client.ProjectOrchestration"
--
buildAction :: NixStyleFlags BuildFlags -> [String] -> GlobalFlags -> IO ()
buildAction flags@NixStyleFlags { extraFlags = buildFlags, ..} targetStrings globalFlags = do
buildAction flags@NixStyleFlags { extraFlags = buildFlags, ..} targetStrings globalFlags
= withContextAndSelectors RejectNoTargets Nothing flags targetStrings globalFlags $ \targetCtx ctx targetSelectors -> do
-- TODO: This flags defaults business is ugly
let onlyConfigure = fromFlag (buildOnlyConfigure defaultBuildFlags
<> buildOnlyConfigure buildFlags)
targetAction
| onlyConfigure = TargetActionConfigure
| otherwise = TargetActionBuild

baseCtx <- establishProjectBaseContext verbosity cliConfig OtherCommand

targetSelectors <-
either (reportTargetSelectorProblems verbosity) return
=<< readTargetSelectors (localPackages baseCtx) Nothing targetStrings
baseCtx <- case targetCtx of
ProjectContext -> return ctx
GlobalContext -> return ctx
ScriptContext path exemeta -> updateContextAndWriteProjectFile ctx path exemeta

buildCtx <-
runProjectPreBuildPhase verbosity baseCtx $ \elaboratedPlan -> do
Expand Down Expand Up @@ -141,8 +143,6 @@ buildAction flags@NixStyleFlags { extraFlags = buildFlags, ..} targetStrings glo
runProjectPostBuildPhase verbosity baseCtx buildCtx buildOutcomes
where
verbosity = fromFlagOrDefault normal (configVerbosity configFlags)
cliConfig = commandLineFlagsToProjectConfig globalFlags flags
mempty -- ClientInstallFlags, not needed here

-- | This defines what a 'TargetSelector' means for the @bench@ command.
-- It selects the 'AvailableTarget's that the 'TargetSelector' refers to,
Expand Down
43 changes: 36 additions & 7 deletions cabal-install/src/Distribution/Client/CmdClean.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import Distribution.Client.DistDirLayout
( DistDirLayout(..), defaultDistDirLayout )
import Distribution.Client.ProjectConfig
( findProjectRoot )
import Distribution.Client.ScriptUtils
( getScriptCacheDirectoryRoot )
import Distribution.Client.Setup
( GlobalFlags )
import Distribution.ReadE ( succeedReadE )
Expand All @@ -22,9 +24,14 @@ import Distribution.Simple.Utils
import Distribution.Verbosity
( normal )

import Control.Monad
( forM, forM_, mapM )
import qualified Data.Set as Set
import System.Directory
( removeDirectoryRecursive, removeFile
, doesDirectoryExist, getDirectoryContents )
, doesDirectoryExist, doesFileExist
, getDirectoryContents, listDirectory
, canonicalizePath )
import System.FilePath
( (</>) )

Expand Down Expand Up @@ -80,16 +87,21 @@ cleanAction CleanFlags{..} extraArgs _ = do
mdistDirectory = flagToMaybe cleanDistDir
mprojectFile = flagToMaybe cleanProjectFile

unless (null extraArgs) $
die' verbosity $ "'clean' doesn't take any extra arguments: "
++ unwords extraArgs
-- TODO interpret extraArgs as targets and clean those targets only (issue #7506)
--
-- For now assume all files passed are the names of scripts
notScripts <- filterM (fmap not . doesFileExist) extraArgs
unless (null notScripts) $
die' verbosity $ "'clean' extra arguments should be script files: "
++ unwords notScripts

projectRoot <- either throwIO return =<< findProjectRoot Nothing mprojectFile

let distLayout = defaultDistDirLayout projectRoot mdistDirectory

if saveConfig
then do
-- Do not clean a project if just running a script in it's directory
when (null extraArgs || isJust mdistDirectory) $ do
if saveConfig then do
let buildRoot = distBuildRootDirectory distLayout

buildRootExists <- doesDirectoryExist buildRoot
Expand All @@ -103,7 +115,24 @@ cleanAction CleanFlags{..} extraArgs _ = do
info verbosity ("Deleting dist-newstyle (" ++ distRoot ++ ")")
handleDoesNotExist () $ removeDirectoryRecursive distRoot

removeEnvFiles (distProjectRootDirectory distLayout)
removeEnvFiles (distProjectRootDirectory distLayout)

-- Clean specified script build caches and orphaned caches.
-- There is currently no good way to specify to only clean orphaned caches.
-- It would be better as part of an explicit gc step (see issue #3333)
toClean <- Set.fromList <$> mapM canonicalizePath extraArgs
cacheDir <- getScriptCacheDirectoryRoot
existsCD <- doesDirectoryExist cacheDir
caches <- if existsCD then listDirectory cacheDir else return []
paths <- fmap concat . forM caches $ \cache -> do
let locFile = cacheDir </> cache </> "scriptlocation"
exists <- doesFileExist locFile
if exists then pure . (,) (cacheDir </> cache) <$> readFile locFile else return []
forM_ paths $ \(cache, script) -> do
exists <- doesFileExist script
when (not exists || script `Set.member` toClean) $ do
info verbosity ("Deleting cache (" ++ cache ++ ") for script (" ++ script ++ ")")
removeDirectoryRecursive cache

removeEnvFiles :: FilePath -> IO ()
removeEnvFiles dir =
Expand Down
Loading

0 comments on commit bbc11f1

Please sign in to comment.