Freeze version of hlint used to lint ghcide

[pepeiborra]

Every time that @ndmitchell releases a new hint, our CI becomes blocked (the most recent one is #2116 (comment))

To prevent this, we should change the lint script to stick to a fixed version of hlint. This will probably imply forking Neil's script:

haskell-language-server/fmt.sh

Lines 1 to 3 in a800b9d

	#!/usr/bin/env bash
	set -eou pipefail
	curl -sSL https://raw.github.com/ndmitchell/hlint/master/misc/run.sh | sh -s ghcide/src ghcide/exe ghcide/bench/lib ghcide/bench/exe ghcide/bench/hist shake-bench/src ghcide/test/exe --with-group=extra --hint=ghcide/.hlint.yaml

[ndmitchell]

The script ultimately calls on to https://github.com/ndmitchell/neil/blob/master/misc/run.sh. If you want to pin a version I think the right thing to do is read an optional external variable like PINNED_VERSION or similar and incorporate that on https://github.com/ndmitchell/neil/blob/master/misc/run.sh#L33.

I'd be happy to add such a feature.

[fendor]

Why not refer to an exact commit? Seems simpler than adding an environment variable

[ndmitchell]

If you want a precompiled binary (which you probably do) then you want to be pointing at a binary release. Those are by version number. Note that the script itself doesn't change, it's just driven by the GitHub releases, and currently always grabs the latest.

[jneira]

The ci is throwing ocasional errors trying to get the hlint script (https://github.com/jneira/haskell-language-server/runs/3532327041?check_suite_focus=true):

Run ./fmt.sh
  ./fmt.sh
  shell: C:\Program Files\Git\bin\bash.EXE --noprofile --norc -e -o pipefail {0}
Downloading and running hlint...
curl: (6) Could not resolve host: github.com
Error: Process completed with exit code 6.

So i wonder if we should host directly it to make it more reliable

[Anton-Latukha]

Also, there is a possibility to use GitHub action for it.
Example setup.

Benefits would be:

• HLint installs once, not 20 times

• HLint runs once, not 20 times

  Running HLint once for reference GHC version seems fine and easy mentally. Sometimes HLint rules can apply to new GHCs and not apply to old GHC CPP code & vise versa, which can often put developers between a rock and a hard place. Standardizing HLint run, for example, on one version of base & GHC - ensures everyone knows what HLint check requires to pass it, and check in itself becomes consistent.

  • As seen in the above example - action allows pinning the version (afair git commit hash is also allowed to be used).

  • Goes as a separate check in CI, so if it returns status code - it is openly seen that PR is blocked due to liter status.

  • It allows to check different paths

  • And set the exception level for each: output results into build log, post as info level notes in PR files change review screen, post as warnings (maybe info to info, warnings to warnings), or raise all to error & status return.

  • Results of the scan - appear in the PR review as:

    (because use HLint during dev & pass HLint CI test before merges, - there is no currently pull requests to show it actively, but for example in logs:
    
    I use relude and tuned custom rules at that moment. maybeToMonoid rule is one I was on the verge about.

    About appearance in the review process, for example, if to raise everything:
    

    As you can imagine - that allows solving all that by itself, the developer sees checks, solves them in Haskell code, or tweaks the HLint settings. At that point - only HLint automation rules are what needs to be managed, rules maintenance exponentially settles into almost zero, while the benefit of them is big. Of course, the management of HLint rules and strong constraints by them can spawn a counter-action holy war, I am lucky to be the sole maintainer of a project and a supporter of almost all rules HLint has so for me it was easy to settle quickly into the list, merging everything I respect and adding a number rules to leverage project (Utils) modification & extension of the prelude. As soon as I encounter the rule I do not like - I remove it, for my case that side of the approach, of course, works great.

• Action also allows having different configs for different projects/directories.

• There are several (at least two) GitHub actions for HLint, and they are comparable in features, I chose the one mentioned in the CI.

• There is also GitLab version: https://gitlab.com/pipeline-components/haskell-hlint

Downsides:
* Existing actions are not official.
* and not actively maintained, so, despite the current GitHub Action API stability there can be time when they would stop working. But their simplicity is such that they can be forked and reworked or new created.
* If the project is moving from GitHub - actions can not be moved, HLint setup & configs can, but CI action probably would not be isomorphed.

I currently do not know other downsides.

neil uses own tool through unpublished GitHub-action:

• neil CI
• hlint CI
  So maybe that way of using it gives more stability.

But it is definitely good to run the check once per CI set, as such 20-times-for-1-change frequently hit access/request quotas & GitHub, Docker, Travis, Nix CDN & such love to block those kinds of spammy/DDoS-alike requests. I am the one that does a lot of atomic commits (was 1-3 in commits (most of them semi-manual) in my country not so far ago), one who loves constantly push to remote as a backup, and retroactively clean-up & improve commits, all that during big activity triggers CI rebuilds all the time & causes to hit those types of quotas semi-regularly even being cautious.

[jneira]

@Anton-Latukha thanks for the detailed analysis, the use of a dedicated action looks great and maybe we could create a dedicated job in the test workflow to run hlint on only 3 ghcs and one so (linux) if we want more coverage per ghc.

We had a new issue recently running hlint: #2214 (comment) and i created an issue upstream ndmitchell/neil#55 but maybe it is due the anti-DDoS as you mention

What do you think about contributing in the repo changing the way we use hlint? It could follow my suggestion (add one job in test workflow per 3 major ghc versions, it should throw an error and no continue with the existing workflow) if you think it makes sense

[Anton-Latukha]

Neil is of course the best person to create an official HLint action.
But if for @ndmitchell it would be seen beneficial to some to do - I can apply an evening or several to create a version to be merged into the official Haskell actions, or into HLint, or just host one in my account. Either way, the GitHub Actions API and abilities had several updates after mentioned actions were created, API deprecations, the introduction of whole new formats as Yaml specifications comparing to JS/JSON ones, (non VM) fast containerized actions..

[Anton-Latukha]

@jneira

What do you think about contributing in the repo changing the way we use hlint?

Yes, it is a thing I also wanted to propose & motivated to do. Depending on Mitchell response I can prioritize actions accordingly (do the HLS CI config change, or if neil gives green light - create the action, merge it where appropriate, and then introduce config change here).

As you also proposed it to me - the second part (submit CI config restructure) is already agreed. Upon configuration work going to reach out in the nearest days in IRC/Matrix, to make it under maintainer team requirements.

[Anton-Latukha]

The #2537 solves the topic. There it pins to 3.3.1.

[ndmitchell]

This looks like it is solved. @Anton-Latukha has switched to a GitHub actions plugin, which is being attempted to be merged with the Haskell github action, so that all seems like we're sorted now.

[jneira]

Solved by #2357