Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork931
Conversation
See pre-commit PR:pre-commit/pre-commit#3348
bc41985 to54d9104Comparepre_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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I suspect this will need special handling foris_local: True -- otherwise hooks won't be able to usejulia with local scripts --language: r is very similar to this so perhaps some inspiration can be drawn from that? (althoughr has a lot more complicated setup so I guess just look at run part for inspiration)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, thanks for the pointer. Added a test.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 defaultLOAD_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.
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 juliahooks the `entry` value is a (relative) path to a julia script withinthe hooks repository. When executing a julia hook the (globallyinstalled) 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/juliaAccompanying pre-commit.com PR:pre-commit/pre-commit.com#998Fixespre-commit#2689.
asottile left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
very cool! thanks for working on this :D
See pre-commit PR:pre-commit/pre-commit#3348
fredrikekre commentedNov 26, 2024
Thanks for the review and merge! I have updatedpre-commit/pre-commit.com#998 to match the final implementation here. |
See pre-commit PR:pre-commit/pre-commit#3348
fredrikekre commentedJan 16, 2025
@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.