-
-
Notifications
You must be signed in to change notification settings - Fork 1k
improve julia language support #3494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
a7f8efa
9583b19
0c54b47
5501721
ae6f19d
f2837cf
9a04b39
1ba5581
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,14 @@ | |
| import shutil | ||
| from collections.abc import Generator | ||
| from collections.abc import Sequence | ||
| from typing import Union | ||
|
|
||
| from pre_commit import lang_base | ||
| from pre_commit.envcontext import _Unset | ||
| from pre_commit.envcontext import envcontext | ||
| from pre_commit.envcontext import PatchesT | ||
| from pre_commit.envcontext import UNSET | ||
| from pre_commit.envcontext import Var | ||
| from pre_commit.prefix import Prefix | ||
| from pre_commit.util import cmd_output_b | ||
|
|
||
|
|
@@ -37,7 +40,7 @@ def run_hook( | |
|
|
||
| cmd = lang_base.hook_cmd(entry, args) | ||
| script = cmd[0] if is_local else prefix.path(cmd[0]) | ||
| cmd = ('julia', script, *cmd[1:]) | ||
| cmd = ('julia', '--startup-file=no', script, *cmd[1:]) | ||
| return lang_base.run_xargs( | ||
| cmd, | ||
| file_args, | ||
|
|
@@ -46,12 +49,17 @@ def run_hook( | |
| ) | ||
|
|
||
|
|
||
| PatchEntry = tuple[str, Union[str, _Unset, tuple[Union[str, Var], ...]]] | ||
|
|
||
|
|
||
| def get_env_patch(target_dir: str, version: str) -> PatchesT: | ||
| return ( | ||
| patches: list[PatchEntry] = [ | ||
| ('JULIA_LOAD_PATH', target_dir), | ||
| # May be set, remove it to not interfer with LOAD_PATH | ||
|
ericphanson marked this conversation as resolved.
|
||
| ('JULIA_PROJECT', UNSET), | ||
| ) | ||
| ] | ||
| if version not in ('system', 'default'): | ||
| patches.append(('JULIAUP_CHANNEL', version)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this isn't quite right. to properly support
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, pre-commit wants an entire copy of julia installed into the hook environment directory? that's very unusual for julia, which uses content-addressed read-only files and packages to allow reuse across environments. It would also make pre-commit install much slower for julia hooks than it currently is. |
||
| return tuple(patches) | ||
|
|
||
|
|
||
| @contextlib.contextmanager | ||
|
|
@@ -68,10 +76,6 @@ def install_environment( | |
| ) -> None: | ||
| envdir = lang_base.environment_dir(prefix, ENVIRONMENT_DIR, version) | ||
| with in_env(prefix, version): | ||
| # TODO: Support language_version with juliaup similar to rust via | ||
| # rustup | ||
| # if version != 'system': | ||
| # ... | ||
|
|
||
| # Copy Project.toml to hook env if it exist | ||
| os.makedirs(envdir, exist_ok=True) | ||
|
|
@@ -99,6 +103,14 @@ def install_environment( | |
| shutil.copy(manifest_file, envdir) | ||
| break | ||
|
|
||
| # copy `src` files if they exist | ||
| src_dir = prefix.path('src') | ||
| if os.path.isdir(src_dir): | ||
| shutil.copytree( | ||
| src_dir, os.path.join(envdir, 'src'), | ||
| dirs_exist_ok=True, | ||
| ) | ||
|
|
||
| # Julia code to instantiate the hook environment | ||
| julia_code = """ | ||
| @assert length(ARGS) > 0 | ||
|
|
@@ -127,6 +139,6 @@ def install_environment( | |
| end | ||
| """ | ||
| cmd_output_b( | ||
| 'julia', '-e', julia_code, '--', envdir, *additional_dependencies, | ||
| cwd=prefix.prefix_dir, | ||
| 'julia', '--startup-file=no', '-e', julia_code, '--', envdir, | ||
| *additional_dependencies, cwd=prefix.prefix_dir, | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,22 @@ def test_julia_hook(tmp_path): | |
| assert run_language(tmp_path, julia, 'src/main.jl') == expected | ||
|
|
||
|
|
||
| def test_julia_hook_version(tmp_path): | ||
| code = """ | ||
| using Example | ||
| function main() | ||
| println("Hello, Julia $(VERSION)!") | ||
| end | ||
| main() | ||
| """ | ||
| _make_hook(tmp_path, code) | ||
| expected = (0, b'Hello, Julia 1.10.10!\n') | ||
| assert run_language( | ||
| tmp_path, julia, 'src/main.jl', | ||
| version='1.10.10', | ||
| ) == expected | ||
|
|
||
|
|
||
| def test_julia_hook_manifest(tmp_path): | ||
| code = """ | ||
| using Example | ||
|
|
@@ -95,3 +111,41 @@ def test_julia_repo_local(tmp_path): | |
| env_dir, julia, 'local.jl --local-arg1 --local-arg2', | ||
| deps=deps, is_local=True, | ||
| ) == expected | ||
|
|
||
|
|
||
| def _make_src_hook(tmp_path, pkg_code, script_code): | ||
| # here we have a package with a src dir and a script dir | ||
| src_dir = tmp_path.joinpath('src') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. feels like from this test this should get handled by the install process. the precedence of copying little bits out of this seems not great and we should find a way to do this step by step
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is in the tests, so we're setting up an example with test files which do get copied as part of the install process |
||
| src_dir.mkdir() | ||
| src_dir.joinpath('ExamplePkg.jl').write_text(pkg_code) | ||
|
|
||
| script_dir = tmp_path.joinpath('scripts') | ||
| script_dir.mkdir() | ||
| script_dir.joinpath('main.jl').write_text(script_code) | ||
|
|
||
| tmp_path.joinpath('Project.toml').write_text( | ||
| 'name = "ExamplePkg"\n' | ||
| 'uuid = "df230c44-b485-4b6a-bafb-763c50abe554"\n' | ||
| '[deps]\n' | ||
| 'Example = "7876af07-990d-54b4-ab0e-23690620f79a"\n', | ||
| ) | ||
|
|
||
|
|
||
| def test_julia_hook_src(tmp_path): | ||
| pkg_code = """ | ||
| module ExamplePkg | ||
| using Example | ||
| export main | ||
| function main() | ||
| println("Hello, world!") | ||
| end | ||
| end | ||
| """ | ||
|
|
||
| script_code = """ | ||
| using ExamplePkg | ||
| main() | ||
| """ | ||
| _make_src_hook(tmp_path, pkg_code, script_code) | ||
| expected = (0, b'Hello, world!\n') | ||
| assert run_language(tmp_path, julia, 'scripts/main.jl') == expected | ||
Uh oh!
There was an error while loading. Please reload this page.