GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add support for julia hooks by fredrikekre · Pull Request #3348 · pre-commit/pre-commit

Conversation

fredrikekre commented Nov 1, 2024

Copy link

Copy Markdown

Contributor

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:

- 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 #2689.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

fredrikekre added a commit to fredrikekre/pre-commit.com that referenced this pull request

Nov 1, 2024


          Documentation for julia hooks

0e459e7

See pre-commit PR: pre-commit/pre-commit#3348

fredrikekre mentioned this pull request

Nov 1, 2024

Documentation for julia hooks pre-commit/pre-commit.com#998

Merged

fredrikekre force-pushed the fe/julia branch 2 times, most recently from bc41985 to 54d9104 Compare

November 3, 2024 01:13

asottile reviewed

Nov 9, 2024

View reviewed changes

Comment thread

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:])

asottile Nov 9, 2024

Copy link

Copy Markdown

Member

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 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)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

fredrikekre Nov 10, 2024

Copy link

Copy Markdown

Contributor Author

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

pre_commit/languages/julia.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

pre_commit/languages/julia.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

pre_commit/languages/julia.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

tests/languages/julia_test.py

		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

asottile Nov 9, 2024

Copy link

Copy Markdown

Member

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

fredrikekre Nov 10, 2024

Copy link

Copy Markdown

Contributor Author

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 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

fredrikekre force-pushed the fe/julia branch from eb5ea96 to 631aaf3 Compare

November 10, 2024 00:32


          Add support for julia hooks

85783bd

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.

asottile force-pushed the fe/julia branch from 631aaf3 to 85783bd Compare

November 25, 2024 23:31

asottile approved these changes

Nov 25, 2024

View reviewed changes

asottile left a comment

Copy link

Copy Markdown

Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

fredrikekre reacted with rocket emoji

All reactions

🚀 1 reaction

asottile enabled auto-merge

November 25, 2024 23:32

asottile merged commit 74233a1 into pre-commit:main

Nov 25, 2024

fredrikekre deleted the fe/julia branch

November 25, 2024 23:42

fredrikekre added a commit to fredrikekre/pre-commit.com that referenced this pull request

Nov 26, 2024


          Documentation for julia hooks

2134a7e

See pre-commit PR: pre-commit/pre-commit#3348

fredrikekre commented Nov 26, 2024

Copy link

Copy Markdown

Contributor Author

Thanks for the review and merge! I have updated pre-commit/pre-commit.com#998 to match the final implementation here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

fredrikekre added a commit to fredrikekre/pre-commit.com that referenced this pull request

Nov 26, 2024


          Documentation for julia hooks

f8eda3b

See pre-commit PR: pre-commit/pre-commit#3348

fredrikekre commented Jan 16, 2025

Copy link

Copy Markdown

Contributor Author

@asottile just out of curiosity, is there a timeline for a new release (4.1.0?) that includes this feature? Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericphanson mentioned this pull request

Feb 5, 2025

Update pre-commit hooks to native-julia JuliaTesting/ExplicitImports.jl#100

Open

fredrikekre mentioned this pull request

Feb 27, 2025

Error/warn on wrong runic version fredrikekre/runic-pre-commit#1

Closed

Reviewers

asottile

asottile approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2nd-class support Julia

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.