kynan / nbstripout

strip output from Jupyter and IPython notebooks
Other
1.19k stars 95 forks source link

Use within pre-commit on GitHub Actions fails with `.git/index.lock` error #105

Closed SimonBiggs closed 4 years ago

SimonBiggs commented 4 years ago

This issue seems quite similar to https://github.com/kynan/nbstripout/issues/103

I run the following GitHub action which installs pre-commit and runs it on all files.

https://github.com/pymedphys/pymedphys/blob/9a484fd3273b2ea898924ec3efbc16b5bfde377a/.github/workflows/main.yml#L5-L19

Pay particular attention to the following two lines where I make sure .git/index.lock doesn't exist and both before and after running pre-commit I list the contents of the .git directory:

while [ -f .git/index.lock ]; do sleep 1; done; ls -hal .git
pre-commit run --all-files || ls -hal .git

The pre-commit config is here:

https://github.com/pymedphys/pymedphys/blob/9a484fd3273b2ea898924ec3efbc16b5bfde377a/.pre-commit-config.yaml#L4-L7

The results within GitHub actions is the following error:

nbstripout...................................................Failed
hookid: nbstripout

Files were modified by this hook. Additional output:

fatal: Unable to create 
'/home/runner/work/pymedphys/pymedphys/.git/index.lock': File exists.

Another git process seems to be running in this repository, e.g.
an editor opened by 'git commit'. Please make sure all processes
are terminated then try again. If it still fails, a git process
may have crashed in this repository earlier:
remove the file manually to continue.

See: https://github.com/pymedphys/pymedphys/pull/486/checks?check_run_id=233766462#step:4:57

More than happy to provide more debugging info, just let me know.

Cheers, Simon

kynan commented 4 years ago

I can't reproduce the case of index.lock existing. When using the .pre-commit-config.yaml you are using I get a failure whenever a file is git added:

$ cd /tmp
$ git init foo
$ cd foo
$ wget https://github.com/kynan/nbstripout/raw/master/tests/test_diff_output.ipynb
$ cat > .pre-commit-config.yaml <<EOF
repos:
-   repo: https://github.com/SimonBiggs/nbstripout
    rev: 0a4fa37151ce3c2fb522bf64469224c831a41773
    hooks:
    -   id: nbstripout
EOF
$ git add test_diff_output.ipynb
$ pre-commit run --all-files
nbstripout...............................................................Failed
hookid: nbstripout

Files were modified by this hook.

iiuc pre-commit will fail whenever a hook creates an unstaged change. Is that your intended behavior?

I believe @Ohjeah's intention was to "auto-fix" a file during the pre-commit run. We can only have one or the other. On reflection I tend to agree with your intention more. I'm happy to merge #106 but open to hearing suggestions / ideas.

cc @rdturnermtl @lainiwa

asottile commented 4 years ago

fwiw, the git add in the configuration is also blocking inclusion in our list of approved hooks: https://github.com/pre-commit/pre-commit.github.io/pull/264

kynan commented 4 years ago

Thanks @asottile. I agree with @SimonBiggs way of thinking and will merge #106 once my requested changes are in.

kynan commented 4 years ago

Please reopen if you find this to still be an issue.