Add support for julia hooks #3348
Conversation
See pre-commit PR: pre-commit/pre-commit#3348
bc41985 to
54d9104
Compare
pre_commit/languages/julia.py
Outdated
| # 2) prepend the hooks prefix path to the first argument (the file) | ||
| # 3) prepend `julia` as the interpreter | ||
| cmd = lang_base.hook_cmd(entry, args) | ||
| cmd = ('julia', prefix.path(cmd[0]), *cmd[1:]) |
There was a problem hiding this comment.
I suspect this will need special handling for is_local: True -- otherwise hooks won't be able to use julia with local scripts -- language: r is very similar to this so perhaps some inspiration can be drawn from that? (although r has a lot more complicated setup so I guess just look at run part for inspiration)
There was a problem hiding this comment.
Yes, thanks for the pointer. Added a test.
| ret, out = run_language(tmp_path, julia, 'src/main.jl', deps=deps) | ||
| assert ret == 0 | ||
| assert b'Example = 7876af07-990d-54b4-ab0e-23690620f79a' in out | ||
| assert b'TOML = fa267f1f-6049-4f14-aa54-33bafae1ed76' in out |
There was a problem hiding this comment.
maybe I just don't know enough about julia but from a quick search it seems like TOML is a stdlib library? does it still need to be installed?
There was a problem hiding this comment.
It is a standard library, and it can be loaded at top level (e.g. in the Julia REPL) by default, since the @stdlib entry is included in the default LOAD_PATH. However, for Julia packages it is required (so people are used to explicitly adding also stdlibs), and for full reproducibility it is recommended. There is also ongoing work to separate stdlibs to make them less tied to the Julia version, so then they would behave more like normal packages that can be upgraded separately etc.
We can of course make the choice to include @stdlib in the load path for julia hooks, but I would argue that it is better to be explicit with the dependencies in this case.
eb5ea96 to
631aaf3
Compare
This patch adds 2nd class support for hooks using julia as the language. pre-commit will install any dependencies defined in the hooks repo `Project.toml` file, with support for `additional_dependencies` as well. Julia doesn't (yet) have a way to install binaries/scripts so for julia hooks the `entry` value is a (relative) path to a julia script within the hooks repository. When executing a julia hook the (globally installed) julia interpreter is prepended to the entry. Example `.pre-commit-hooks.yaml`: ```yaml - id: foo name: ... language: julia entry: bin/foo.jl --arg1 ``` Example hooks repo: https://github.com/fredrikekre/runic-pre-commit/tree/fe/julia Accompanying pre-commit.com PR: pre-commit/pre-commit.com#998 Fixes pre-commit#2689.
See pre-commit PR: pre-commit/pre-commit#3348
|
Thanks for the review and merge! I have updated pre-commit/pre-commit.com#998 to match the final implementation here. |
See pre-commit PR: pre-commit/pre-commit#3348
|
@asottile just out of curiosity, is there a timeline for a new release (4.1.0?) that includes this feature? Thanks! |
This patch adds 2nd class support for hooks using julia as the language. pre-commit will install any dependencies defined in the hooks repo
Project.tomlfile, with support foradditional_dependenciesas well. Julia doesn't (yet) have a way to install binaries/scripts so for julia hooks theentryvalue is a (relative) path to a julia script within the hooks repository. When executing a julia hook the (globally installed) julia interpreter is prepended to the entry.Example
.pre-commit-hooks.yaml:Example hooks repo: https://github.com/fredrikekre/runic-pre-commit/tree/fe/julia
Accompanying pre-commit.com PR: pre-commit/pre-commit.com#998
Fixes #2689.