change language of .pre-commit-hooks.yaml to golang

[Freed-Wu]

Refer https://github.com/scop/pre-commit-shfmt/blob/main/.pre-commit-hooks.yaml

[dokempf]

Actually, this does not work straight away, as @adamchainz noted in #297 . I found a solution on my fork that works: dokempf@691f4ee

[Freed-Wu]

What is your environment? It works for me #297 (comment). Can your try:

# ...
  - repo: https://github.com/Freed-Wu/actionlint
    rev: v1.6.25
    hooks:
      - id: actionlint

[dokempf]

Your v1.6.25 does not actually include your change! If I run that, I get Executable actionlint not found.. If I switch to your patch-1 branch in the rev field, I get exactly the error mentioned in #297.

[Freed-Wu]

What is your enviroment? This is mine:

$ go version
go version go1.20.4 linux/amd64
$ uname -r
6.3.2-arch1-1

[dokempf]

I don't have go installed - and that is exactly the point of this change.

[Freed-Wu]

If I uninstalled go, I'll get

An unexpected error has occurred: CalledProcessError: command: ('go', 'install', './...')
return code: 1
expected return code: 0
stdout:
    Executable `go` not found

Can you install go to validate again?

[rhysd]

I'm not a user of pre-commit. So I'm not familiar with the tool.

Considering compatibility, is it safe to replace language: system with language: golang? Or is it better to add new configuration actionlint-golang like actionlint-docker? I'm not sure replacing it is ok. For example, some users may use actionlint with downloading it from releases page and they may not have Go environment. Is the language: golang fallbacks to system-installed actionlint binary when Go toolchain is not installed?

[rhysd]

One more point. pre-commit seems to install the binary using go install ./.... It installs all executable packages in this repository. It means, all unnecessary binaries such as the generate scripts are built and installed as well on your machine. Is it OK? Does pre-commit do sandboxing the installation in some temporary directory? Otherwise it will pollute your ~/go/bin with the unnecessary binaries.

[dokempf]

@rhysd The idea with pre-commit is that all hooks are installed into isolated environments managed by pre-commit. For golang, this means that it will use an isolated, separate GOPATH. As of pre-commit v3.0.0, it bootstraps the entire go environment, so it would give people access to your linter that do not even know that it is written in Go or what Go is. As this makes it a really powerful tool, I would recommend adding a pre-commit version constraint to .pre-commit-hooks.yml to enforce >=3.0.0. With everything I said, I think replacing system -> golang is safe. The only thing that is a bit more complicated with this setup is to run custom compiled actionlint executables as part of the pre-commit run - you need to push the changes to a git repository, tags them and then reference that repo in your pre-commit config.

[rhysd]

@dokempf Thank you for your explanation. It clarifies my several concerns. My remaining concern is:

With everything I said, I think replacing system -> golang is safe.

Isn't it a breaking change for those who don't install Go toolchain and using system-installed actionlint with pre-commit? Though actionlint is written in Go, it is not a tool for Go. Users can use package managers such as Homebrew or pre-built binaries without building it from source. So I think pre-commit config should not be tied with Go by default.

However, I also understand the language: golang configuration is useful since it doesn't require to install actionlint manually. My preference is adding a new configuration actionlint-golang like actionlint-docker and remaining the actionlint config as-is.

I would recommend adding a pre-commit version constraint to .pre-commit-hooks.yml to enforce >=3.0.0.

It makes sense. Adding this constraint means a breaking change for users who are already using actionlint config. It is another reason to add a new config rather than modifying the existing one.

[dokempf]

I guess one could argue how breaking the change is. I would not consider the requirement to update pre-commit to be too breaking. The fact that your existing local installation of actionlint is silently ignored after the change is a bit more troublesome. If you want to apply a very conservative philosophy, introducing a new actionlint-golang is a good choice.

[Freed-Wu]

I would recommend adding a pre-commit version constraint to .pre-commit-hooks.yml to enforce >=3.0.0.

It makes sense.

Fixed.

  minimum_pre_commit_version: 3.0.0

[rhysd]

@dokempf I may be too conservative. Since I'm not a user of pre-commit, I'd like to prioritize actual users' opinions. So I think changing the language config is OK.

In addition, I think it is good to add the classic language: system config as actionlint-system for those who don't have Go toolchain. They can continue to use the same environment with very small effort (changing pre-commit hook ID from actionlint to actionlint-system). I'll add it after merging this PR.

@Freed-Wu @dokempf Current configuration clones the HEAD of main branch, am I correct? If so, I think it is safer to fix the revision cloned by pre-commit to the latest tag name (currently it's v1.6.24). Is it possible? Since v1.6.24 doesn't include the fix for #297, it is not available. But I'd like to know how to do it for the next release (probably 1.6.25).

[rhysd]

I understood that the revision is in .pre-commit-config.yaml hence it is set by users. So we don't need to set it.

https://pre-commit.com/#pre-commit-configyaml---repos

[dokempf]

Thanks for the fast merge @rhysd Be aware that in order for this to be fully usable from within pre-commit, a new release of actionlint is required (mutable references like branches are not properly supported in pre-commit).

[rhysd]

OK, I'll make a new release 1.6.25 tomorrow after adding the legacy actionlint-system hook.

[rhysd]

v1.6.25 was released.

[adamchainz]

I have just tried the new golang hook out on my client's project and it works well. It's 10x faster than the Docker one - taking a 0.91s runtime down to 0.09s. Thank you very much! 🤘