Convert validate.sh to a Haskell script

[9999years]

Closes #10317.

A Haskell script will be easier to maintain and expand than the existing Bash script.

This also adds a --pattern PATTERN option which lets you filter tests across all test suites.

I've kept the new script's behavior as close to the old script's behavior as possible, to reduce design discussion on this PR.

TODO

• Add cabal-validate/README.md to make it obvious where to look for logic and how to interact with the program
• Add Haddock comments to most definitions

Future work

• It should be possible to override the default tasty args (currently --hide-successes)
• --list-steps only lists the steps that are enabled with the given command-line options, not all possible steps; ./validate.sh --list-steps and ./validate.sh --list-steps --run-cli-suite give different results! (This is an existing bug in the old script.)
• Perhaps "steps" should be divided into "suites"; for example, the lib-tests step runs five different test suites, making it take much longer than necessary to reproduce test failures.
• Commands printed to users should be shell escaped. (Exiting bug in the old script.)
• --help output could use some love; it wasn't very clear to me what various commands do.
• --keep-going mode to ignore failures and continue executing tests/suites.
• The output is really verbose, because the command lines are so long.

[ulysses4ever]

I wonder if it can be just a cabal script... But this is not crucial at the moment.

[philderbeast]

I hope I'm not jumping the gun with a pull request in draft but HLint is gagged¹ and could help.

We suppress a lot of HLint suggestions at the root :-(

cabal/.hlint.yaml

Lines 1 to 95 in 3a6f73e

	# Warnings currently triggered by your code
	- ignore: {name: "Avoid NonEmpty.unzip"} # 1 hint
	- ignore: {name: "Avoid lambda"} # 46 hints
	- ignore: {name: "Avoid lambda using `infix`"} # 22 hints
	- ignore: {name: "Eta reduce"} # 116 hints
	- ignore: {name: "Evaluate"} # 10 hints
	- ignore: {name: "Functor law"} # 10 hints
	- ignore: {name: "Fuse concatMap/map"} # 3 hints
	- ignore: {name: "Fuse foldr/map"} # 3 hints
	- ignore: {name: "Fuse mapMaybe/map"} # 1 hint
	- ignore: {name: "Fuse traverse_/fmap"} # 1 hint
	- ignore: {name: "Fuse traverse_/map"} # 1 hint
	- ignore: {name: "Hoist not"} # 16 hints
	- ignore: {name: "Missing NOINLINE pragma"} # 1 hint
	- ignore: {name: "Monoid law, left identity"} # 3 hints
	- ignore: {name: "Monoid law, right identity"} # 3 hints
	- ignore: {name: "Move filter"} # 4 hints
	- ignore: {name: "Move guards forward"} # 4 hints
	- ignore: {name: "Redundant $"} # 175 hints
	- ignore: {name: "Redundant $!"} # 1 hint
	- ignore: {name: "Redundant <$>"} # 16 hints
	- ignore: {name: "Redundant =="} # 1 hint
	- ignore: {name: "Redundant bracket"} # 232 hints
	- ignore: {name: "Redundant fmap"} # 1 hint
	- ignore: {name: "Redundant guard"} # 2 hints
	- ignore: {name: "Redundant if"} # 3 hints
	- ignore: {name: "Redundant lambda"} # 19 hints
	- ignore: {name: "Redundant multi-way if"} # 1 hint
	- ignore: {name: "Redundant return"} # 7 hints
	- ignore: {name: "Replace case with fromMaybe"} # 5 hints
	- ignore: {name: "Replace case with maybe"} # 10 hints
	- ignore: {name: "Unused LANGUAGE pragma"} # 167 hints
	- ignore: {name: "Use $>"} # 5 hints
	- ignore: {name: "Use ++"} # 4 hints
	- ignore: {name: "Use :"} # 25 hints
	- ignore: {name: "Use <$"} # 2 hints
	- ignore: {name: "Use <$>"} # 86 hints
	- ignore: {name: "Use <&>"} # 14 hints
	- ignore: {name: "Use <=<"} # 5 hints
	- ignore: {name: "Use =<<"} # 7 hints
	- ignore: {name: "Use =="} # 3 hints
	- ignore: {name: "Use >=>"} # 3 hints
	- ignore: {name: "Use ?~"} # 1 hint
	- ignore: {name: "Use Down"} # 3 hints
	- ignore: {name: "Use Just"} # 2 hints
	- ignore: {name: "Use bimap"} # 7 hints
	- ignore: {name: "Use camelCase"} # 96 hints
	- ignore: {name: "Use catMaybes"} # 3 hints
	- ignore: {name: "Use concatMap"} # 1 hint
	- ignore: {name: "Use const"} # 36 hints
	- ignore: {name: "Use elem"} # 2 hints
	- ignore: {name: "Use fewer imports"} # 19 hints
	- ignore: {name: "Use first"} # 4 hints
	- ignore: {name: "Use fmap"} # 24 hints
	- ignore: {name: "Use fold"} # 1 hint
	- ignore: {name: "Use for"} # 1 hint
	- ignore: {name: "Use forM_"} # 1 hint
	- ignore: {name: "Use fromMaybe"} # 1 hint
	- ignore: {name: "Use fromRight"} # 1 hint
	- ignore: {name: "Use fst"} # 1 hint
	- ignore: {name: "Use if"} # 4 hints
	- ignore: {name: "Use infix"} # 20 hints
	- ignore: {name: "Use isAsciiLower"} # 2 hints
	- ignore: {name: "Use isAsciiUpper"} # 2 hints
	- ignore: {name: "Use isDigit"} # 2 hints
	- ignore: {name: "Use isJust"} # 1 hint
	- ignore: {name: "Use isNothing"} # 1 hint
	- ignore: {name: "Use lambda-case"} # 47 hints
	- ignore: {name: "Use lefts"} # 1 hint
	- ignore: {name: "Use list comprehension"} # 16 hints
	- ignore: {name: "Use list literal"} # 3 hints
	- ignore: {name: "Use list literal pattern"} # 11 hints
	- ignore: {name: "Use map once"} # 7 hints
	- ignore: {name: "Use map with tuple-section"} # 3 hints
	- ignore: {name: "Use mapMaybe"} # 13 hints
	- ignore: {name: "Use max"} # 1 hint
	- ignore: {name: "Use maybe"} # 8 hints
	- ignore: {name: "Use minimumBy"} # 1 hint
	- ignore: {name: "Use newtype instead of data"} # 26 hints
	- ignore: {name: "Use notElem"} # 8 hints
	- ignore: {name: "Use null"} # 2 hints
	- ignore: {name: "Use record patterns"} # 16 hints
	- ignore: {name: "Use replicateM"} # 1 hint
	- ignore: {name: "Use replicateM_"} # 2 hints
	- ignore: {name: "Use rights"} # 2 hints
	- ignore: {name: "Use second"} # 7 hints
	- ignore: {name: "Use section"} # 17 hints
	- ignore: {name: "Use traverse"} # 1 hint
	- ignore: {name: "Use tuple-section"} # 28 hints
	- ignore: {name: "Use typeRep"} # 2 hints
	- ignore: {name: "Use unless"} # 20 hints
	- ignore: {name: "Use unwords"} # 8 hints
	- ignore: {name: "Use void"} # 22 hints
	- ignore: {name: "Use when"} # 1 hint
	- ignore: {name: "Use uncurry"} # 1 hint

Side-stepping that configuration that ignores 94 suggestions lets HLint come forward with a few suggestions:

$ sed 's/^- ignore/# &/' .hlint.yaml > .hlint-open.yaml \
  && hlint cabal-validate/main/Main.hs --hint=.hlint-open.yaml
cabal-validate/main/Main.hs:125:26-52: Suggestion: Use :
Found:
  ["build"] ++ cabalArgs opts
Perhaps:
  "build" : cabalArgs opts

cabal-validate/main/Main.hs:128:25-54: Suggestion: Use :
Found:
  ["list-bin"] ++ cabalArgs opts
Perhaps:
  "list-bin" : cabalArgs opts

cabal-validate/main/Main.hs:213:12-60: Warning: Use maybe
Found:
  fromMaybe getNumCapabilities (pure <$> opts . jobs)
Perhaps:
  maybe getNumCapabilities pure (opts . jobs)

cabal-validate/main/Main.hs:303:12-43: Suggestion: Redundant bracket
Found:
  (option (maybeReader parseStep))
    (long "step"
       <>
         help "Run only a specific step (can be specified multiple times)")
Perhaps:
  option
    (maybeReader parseStep)
    (long "step"
       <>
         help "Run only a specific step (can be specified multiple times)")

cabal-validate/main/Main.hs:(373,1)-(374,66): Warning: Eta reduce
Found:
  boolOption defaultValue trueName modifiers
    = boolOption' defaultValue trueName ("no-" <> trueName) modifiers
Perhaps:
  boolOption defaultValue trueName
    = boolOption' defaultValue trueName ("no-" <> trueName)

cabal-validate/main/Main.hs:472:16-42: Suggestion: Use tuple-section
Found:
  \ exitCode -> (exitCode, "")
Perhaps:
  (, "")
Note: may require `{-# LANGUAGE TupleSections #-}` adding to the top of the file

6 hints

Footnotes

1. The gag order is RFC: Applying HLint's #9110. We want to protect existing code from churn but this is new code. ↩

[philderbeast]

I wonder if it can be just a cabal script... But this is not crucial at the moment.

@ulysses4ever, it could be but unfortunately it can't also be a script. It can't be both an installable executable and a script (but I wish it could), reported as #10324.

[ulysses4ever]

@9999years requested review from philderbeast and ulysses4ever

It's on my radar but I'll only get to it the next week I'm afraid

[ulysses4ever]

Amazing work, thanks!

My only big concern is how many times it will have to be built from scratch and whether it will bring a noticeable slowdown in CI. The dependency footprint looks non-negligible and hence I expect noticeable build times. Perhaps, it’d be wiser to have it as a standalone GitHub project that could host its own binary releases, which we could download during cabal CI. Or something fancy like nix-based build and cachix.

[ulysses4ever]

I removed the merge-related labels for the time being so that we avoid a premature merge. If after reading the feedback you really think it’s ready to go, please feel free to re-apply the labels.

[mpickering]

It seems that the validate script is built with the same version of GHC as the one which is being tested.

It would be better in my opinion if the validate script was decoupled from the version of GHC being tested. (This was achieved when it was a bash script)

It would be bad to get into a situation where adding testing for a new compiler is blocked because one of the dependencies of the validate script hasn't been updated yet.

[chreekat]

It seems that the validate script is built with the same version of GHC as the one which is being tested.

On that note, the script depends on terminal-size, which depends on ghc-prim, which "is an internal package, only for the use of GHC developers. GHC users should not use it!" Perhaps that dependency could be dropped, at the very least?

[geekosaur]

Is that not available from ansi-terminal? If not, why does terminal-size depend explicitly on ghc-prim? The correct way to get terminal size doesn't require GHC internals, afaik. (Granting that the way I know is POSIX; I don't know what Windows requires, but I also am not sure you get any options there, I recall it resizing fonts to keep the 80x25 size.)

[9999years]

On that note, the script depends on terminal-size, which depends on ghc-prim, which "is an internal package, only for the use of GHC developers. GHC users should not use it!" Perhaps that dependency could be dropped, at the very least?

terminal-size only depends on ghc-prim when built with GHC between 7.2 and 7.6, which I suspect we do not have to worry about:

https://github.com/biegunka/terminal-size/blob/8c5765027ae6194191d6da11d69a49c37a78a73a/terminal-size.cabal#L32-L34

[9999years]

It seems that the validate script is built with the same version of GHC as the one which is being tested.

It would be better in my opinion if the validate script was decoupled from the version of GHC being tested. (This was achieved when it was a bash script)

Nothing requires this to be the case; we could easily do cabal build --with-ghc ... cabal-validate in CI or locally. The 3-line validate.sh entrypoint is simply provided as a convenience.

[9999years]

It would be bad to get into a situation where adding testing for a new compiler is blocked because one of the dependencies of the validate script hasn't been updated yet.

The new dependencies are ansi-terminal, terminal-size, and typed-process. All of the others are either bundled with GHC or used by other components in this repository already.

ansi-terminal could be removed pretty easily; we only use it to write ANSI escape sequences.

terminal-size only depends on base >=4 && <5 (except on Windows, where it depends on process and Win32, both bundled with GHC), and it's only 120 lines of code anyways, so I think that's fine.

typed-process pulls in unliftio-core (all other dependencies are bundled with GHC or included in other Cabal components already), which also seems pretty reasonable. We could remove this, but the ergonomics and maintainability of cabal-validate would suffer (IMO) and we'd have to devote a fair amount of effort to reproducing its error messages.

[9999years]

My only big concern is how many times it will have to be built from scratch and whether it will bring a noticeable slowdown in CI.

It seems like the print-config step, which builds cabal-validate, finishes in about 17 seconds in CI. (Judging by local runs, about ~1 second or less of that is running the cabal-validate script.) That seems like a reasonable cost to me, and from the fact that none of the dependencies are built in that step, it seems like the dependencies are cached as well, so we can expect to not pay that cost unless we're changing validate.sh.

[chreekat]

terminal-size only depends on ghc-prim when built with GHC between 7.2 and 7.6, which I suspect we do not have to worry about:

Oh, thanks for pointing that out! I only got as far as the package home page on Hackage, which lists the dependencies without mentioning the constraints.