Skip to content
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

a hook for jsonnet formatting #1785

Closed
gaurav517 opened this issue Feb 5, 2021 · 17 comments
Closed

a hook for jsonnet formatting #1785

gaurav517 opened this issue Feb 5, 2021 · 17 comments
Labels

Comments

@gaurav517
Copy link

I am trying to write a hook to format jsonnet files.

Goal is: If a changed file is *.jsonnet or *.libsonnet, then run jsonnetfmt -i <file_path>. (Assuming jsonnetfmt in the path)

What would be the best approach? I believe system language would work? Is there an example that could help me?

Thanks.

@gaurav517
Copy link
Author

ok.. I got it:

- id: jsonnet-format
  name: jsonnet/libsonnet files should be formatted
  description: formats jsonnet/libsonnet files
  entry: '/usr/local/bin/jsonnetfmt -i'
  files: \.(jsonnet|libsonnet)$
  language: system

@paulhfischer
Copy link
Contributor

paulhfischer commented Feb 5, 2021

you might have a look at the go implementation of jsonnet, as one of the big benefits of using pre-commit is the automatic environment handling and installation of hooks which isn't possible with the system hook type. thus your configuration might look something like this:

repos:
-   repo: local
    hooks:
    -   id: jsonnetfmt
        name: jsonnetfmt
        entry: jsonnetfmt -i
        language: golang
        files: '\.(jsonnet|libsonnet)$'
        additional_dependencies: ["github.com/google/go-jsonnet/cmd/jsonnetfmt"]

(also it might be useful to create a PR and add pre-commit-hooks.yaml to go-jsonnet)

@gaurav517
Copy link
Author

I am testing above hook:

I add following in .pre-commit-hooks.yaml of a repo: `git-hooks`:
- id: jsonnet-format
  name: Format should be valid for jsonnet/libsonnet files
  entry: jsonnetfmt -i
  language: golang
  files: '\.(jsonnet|libsonnet)$'
  additional_dependencies: [ "github.com/google/go-jsonnet/cmd/jsonnetfmt" ]

so I run pre-commit try-repo ~/Documents/git-hooks jsonnet-format --verbose in another repo and I see:

Format should be valid for jsonnet/libsonnet files...(no files to check)Skipped

However there is one file that has wrong formatting (apps/my/deployment/mydeploy.jsonnet). Any idea why it could be skipping that file?

@gaurav517
Copy link
Author

Tried files: '.*\.(jsonnet|libsonnet)$'.. but same.. skipped the file.

@paulhfischer
Copy link
Contributor

paulhfischer commented Feb 5, 2021

is the file tracked? pre-commit will only run on tracked files. also does the file have unstaged changes? else just running pre-commit won't run on this file and you'll have to use pre-commit run --all-files.

@gaurav517
Copy link
Author

yes this file is tracked. Also the file has unstaged changes. I actually tested with pretty-format-json as well (changing a json file format). All of them are skipped

git status                                                                                                                                            
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   apps/my/ci/file.json
        modified:   apps/my/deploy/file.jsonnet

pre-commit                                                                                                                                                         
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /Users/kgaurav2/.cache/pre-commit/patch1612539446.
[INFO] Installing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
Check JSON...........................................(no files to check)Skipped
Pretty format JSON...................................(no files to check)Skipped
Format should be valid for jsonnet/libsonnet files...(no files to check)Skipped

Wonder what is happening.

@gaurav517
Copy link
Author

in verbose mode:

pre-commit run --verbose 
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /Users/kgaurav2/.cache/pre-commit/patch1612539821.
Check JSON...........................................(no files to check)Skipped
- hook id: check-json
Pretty format JSON...................................(no files to check)Skipped
- hook id: pretty-format-json
Format should be valid for jsonnet/libsonnet files...(no files to check)Skipped
- hook id: jsonnet-format
[INFO] Restored changes from /Users/kgaurav2/.cache/pre-commit/patch1612539821.

patch looks fine.

@asottile
Copy link
Member

asottile commented Feb 5, 2021

by default pre-commit only runs on staged files -- you probably want pre-commit try-repo --all-files / pre-commit run --all-files

@paulhfischer
Copy link
Contributor

if you have a look at the output, pre-commit stashed your files as you haven't added them via git add apps/my/ci/file.json apps/my/deploy/file.jsonnet.

$ git status
On branch master
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   t.jsonnet

no changes added to commit (use "git add" and/or "git commit -a")

$ pre-commit
[WARNING] Unstaged files detected.
[INFO] Stashing unstaged files to /Users/paulfischer/.cache/pre-commit/patch1612540509.
jsonnetfmt...........................................(no files to check)Skipped
[INFO] Restored changes from /Users/paulfischer/.cache/pre-commit/patch1612540509.

$ git add t.jsonnet

$ git status
On branch master
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
	modified:   t.jsonnet

$ pre-commit
jsonnetfmt...............................................................Passed

@gaurav517
Copy link
Author

Thanks a lot. This works.

@asottile asottile closed this as completed Feb 6, 2021
@gaurav517
Copy link
Author

gaurav517 commented Feb 6, 2021

Another question, if we add this hook in pre-commit-hooks.yaml in https://github.com/google/go-jsonnet, then when a user uses this hook, will it download whole go-jsonnet repo in ~/.cache/pre-commit?

@asottile
Copy link
Member

asottile commented Feb 6, 2021

a shallow clone of that, yes

@gaurav517
Copy link
Author

I added this hook in pre-commit-hooks.yaml in https://github.com/google/go-jsonnet, and then when I try to use this hook as:

repos:
  - repo: https://github.com/google/go-jsonnet
    rev: 4a3144a417b7eb9b1f7e56741a9e72f3155de3fa
    hooks:
      - id: jsonnet-format

then I see following error.

Traceback (most recent call last):
  File "/usr/local/Cellar/pre-commit/2.10.0/libexec/lib/python3.9/site-packages/pre_commit/error_handler.py", line 65, in error_handler
    yield
  File "/usr/local/Cellar/pre-commit/2.10.0/libexec/lib/python3.9/site-packages/pre_commit/main.py", line 378, in main
    return run(args.config, store, args)
  File "/usr/local/Cellar/pre-commit/2.10.0/libexec/lib/python3.9/site-packages/pre_commit/commands/run.py", line 403, in run
    install_hook_envs(hooks, store)
  File "/usr/local/Cellar/pre-commit/2.10.0/libexec/lib/python3.9/site-packages/pre_commit/repository.py", line 224, in install_hook_envs
    _hook_install(hook)
  File "/usr/local/Cellar/pre-commit/2.10.0/libexec/lib/python3.9/site-packages/pre_commit/repository.py", line 82, in _hook_install
    lang.install_environment(
  File "/usr/local/Cellar/pre-commit/2.10.0/libexec/lib/python3.9/site-packages/pre_commit/languages/golang.py", line 81, in install_environment
    cmd_output_b('go', 'get', './...', cwd=repo_src_dir, env=env)
  File "/usr/local/Cellar/pre-commit/2.10.0/libexec/lib/python3.9/site-packages/pre_commit/util.py", line 154, in cmd_output_b
    raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)
pre_commit.util.CalledProcessError: command: ('/usr/local/bin/go', 'get', './...')
return code: 2
expected return code: 0
stdout: (none)
stderr:
    go: downloading github.com/sergi/go-diff v1.1.0
    go: downloading github.com/fatih/color v1.9.0
    go: downloading github.com/mattn/go-colorable v0.1.4
    go: downloading github.com/mattn/go-isatty v0.0.11
    go: downloading golang.org/x/sys v0.0.0-20191026070338-33540a1f6037
    # github.com/google/go-jsonnet/c-bindings
    libjsonnet.cpp:5:14: fatal error: 'libjsonnet.h' file not found

Any idea? Thanks.

@paulhfischer
Copy link
Contributor

paulhfischer commented Feb 6, 2021

libjsonnet.h is located here (in another repo) and only symlinked in the go implementation. Thus when cloning the repo you would need the --recursive flag. pre-commit does that (see here).

@asottile
Copy link
Member

asottile commented Feb 6, 2021

looks like they're missing this directive for the cgo compiler:

https://github.com/google/go-jsonnet/blob/4a3144a417b7eb9b1f7e56741a9e72f3155de3fa/c-bindings/c-bindings.go#L14

@asottile
Copy link
Member

asottile commented Feb 6, 2021

oh actually hmm, this should still work -- I bet the second side-clone isn't happening when building the GOPATH

@asottile
Copy link
Member

asottile commented Feb 6, 2021

let me make a separate issue for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants