Better support for running scripts. #7851
Conversation
|
hi! many thanks for open the wip pr, i hope it could help to get some general feedback from maintainers. Maybe you have thought on but for completeness i think the pr would need:
Not sure if you have some knowledge about the functional test suite, feel free to ask anything about. Surely it will contain similar tests over the script feature, and some about build/repl which can be used as a model. Although this is still a WIP i would ask maintainers to share their general thoughts and the possible drawbacks about the chosen implementation, specially if they could be a blocker for merging the pr, thanks in advance! One of the concerns could be the interaction of this with the fake-package machinery. Afaiu the plan is to eventually remove it and replace it with a all-in-memory implementation (see #6977). //cc @gbaz @fgaz @Kleidukos |
|
Also for completeness it needs changes to the non-haddock docs, because it changes the interface for |
|
The following tests are still failing and it's not clear to me why (callProcess exits with 1 and no further info is printed, but they both seem to be behaving correctly):
In addition, the existing tests caught an issue with --builddir being set. Are there any other flags I should potentially worry about that existing tests might not catch? |
|
The testsuite has gaps, so it can fail to catch anything. It's definitely worth debugging the return code 1. Processes are fragile and there are known issues, but we should at least try to determine what exactly is misbehaving and perhaps file a bug report about that. |
|
@Mikolaj I found a problem in my code that may have been causing those tests to fail (I was passing the wrong ComponentKind for build), but I'm in the middle of of a refactor to solve another issue I found while writing tests (repl didn't handle the case when the script was in a directory with a project correctly). Hopefully those tests will pass after I finish my current work. |
|
I added the tests suggested by @jneira, with the except that I don't test that build artifacts are created in the cache dir because the test runner always passes a --builddir flag which overrides that location. I do test that the fake project artifacts are created in the cache dir. I have manually tested that the cache dir is used correctly when --builddir is not passed. In addition, it would in theory be nice to tests for the code path in which the script resides outside of a project directory, but this is impossible afaik because all tests live inside the cabal-tests project directory. |
|
I don't remember --- does cabal walk up the directory tree checking all ancestors for a project file? If so, can't a test script create a directory in /tmp, cd to it and run there? I don't count on such functionality already in the test suite and probably not worth adding it, but can such an ad-hoc solution work for this particular test? |
|
@Mikolaj cabal does walk up the tree. If we wanted to add the functionality, then placing a test directory in /tmp would probably be how to do it, but I think it might require a significant changes to the test runner. |
Ready for review: haskell#7851 May close: haskell#7842, haskell#6354, haskell#6149
bacchanalia
changed the title
|
Champion! I'll get a review out asap today :3 |
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 haskell#7851
Tests failing on pre-AMP ghcs due to unsanctioned use of (<$>) PR haskell#7851
Ready for review: haskell#7851 May close: haskell#7842, haskell#6354, haskell#6149
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 haskell#7851
Tests failing on pre-AMP ghcs due to unsanctioned use of (<$>) PR haskell#7851
- remove partial selectors - make a constant for fake-package.cabal PR haskell#7851
- 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 (haskell#6787) - Add new build/repl test and run multi-module test PR haskell#7851
- 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 haskell#7851
Move argument truncation from targetStrings out of withScriptContextAndSelectors to runAction PR haskell#7851
- 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 haskell#7851
✅ Branch has been successfully rebased |
This enables running it via simply `./Build.hs`. It's difficult (possibly not standardized at all, I can't remember) to specify arguments here, so for now this will only work if GHC 8.10.7 is the version enabled globally. And using `cabal install --lib` (or `cabal-env`, or my script for extracting environment files from cabal scripts) is still preferable due to the slowness of `cabal run`, and HLS support. Cabal scripts are set to become more usable in 3.8: haskell/cabal#7851.
Add caching to
cabal run scriptand script support with caching tobuild,repl, andclean.This is intended to close #7842, #6354, and #6149
edit: Title and body changed to reflect end of WIP.