pre-commit hook fails while running on pre-commit.ci #15

billsioros · 2021-06-29T16:13:25Z

An example run can be found here.

My pre-commit-config is the following:

ci:
  skip: [editorconfig-checker, export-requirements]
  autofix_commit_msg: 'refactor: `pre-commit.ci` auto fix'
  autofix_prs: true
  autoupdate_commit_msg: 'refactor: `pre-commit.ci` auto update'

repos:
  - repo: https://github.com/humitos/mirrors-autoflake.git
    rev: v1.1
    hooks:
      - id: autoflake
        args: ['--in-place', '--remove-all-unused-imports', '--remove-unused-variable']
  - repo: https://github.com/pycqa/isort
    rev: 5.9.1
    hooks:
      - id: isort
  - repo: https://github.com/psf/black
    rev: 21.6b0
    hooks:
      - id: black
        language_version: python3
  - repo: https://github.com/PyCQA/flake8
    rev: 3.9.2
    hooks:
      - id: flake8
        exclude: ^docs|examples|tests
        additional_dependencies: [wemake_python_styleguide]
  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.0.1
    hooks:
      - id: trailing-whitespace
      - id: end-of-file-fixer
      - id: check-yaml
      - id: check-json
      - id: check-toml
      - id: debug-statements
  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v0.910
    hooks:
    - id: mypy
      args: [--config, .mypy.ini]
      files: ^dotify|tests/
      additional_dependencies: [types-all]
  - repo: https://github.com/commitizen-tools/commitizen
    rev: v2.17.11
    hooks:
      - id: commitizen
        stages: [commit-msg]
  - repo: https://github.com/editorconfig-checker/editorconfig-checker.python
    rev: 2.3.5
    hooks:
      - id: editorconfig-checker
  - repo: https://github.com/jendrikseipp/vulture
    rev: v2.3
    hooks:
      - id: vulture
  - repo: https://github.com/tox-dev/tox-ini-fmt
    rev: 0.5.1
    hooks:
      - id: tox-ini-fmt
  - repo: local
    hooks:
      - id: export-requirements
        name: export-requirements
        language: system
        pass_filenames: false
        entry: poetry export --without-hashes -o requirements.txt
        files: ^(pyproject.toml|poetry.lock)$

Thanks in advance!

mmicu · 2021-07-01T09:15:46Z

Hi. Could you try to run the program with sudo? The script downloads the executable and the location must be writable.

If it works with sudo, I can introduce a new variable for selecting the location where the executable will be downloaded.

billsioros · 2021-07-01T09:49:55Z

Hello @mmicu ,

I'm quite sure that there is no way to run the command with sudo on ci 😢 I'll open a corresponding issue on pre-commit.ci's end.

Feel free to check it out here.

billsioros · 2021-07-01T13:58:32Z

Hello again,

as you can see in the linked issue network is not available at runtime 🥳 . One possible solution could be to bundle the executable with the python package. Other than that I'm not sure what else could be done.

mmicu · 2021-07-01T15:33:10Z

Hi. Yes, I could attach the executable to the Python package. I will try to release a new version in a few days.

billsioros · 2021-07-08T06:46:45Z

@mmicu Hello again! Any news on this 😃 ?

mmicu · 2021-07-08T12:42:24Z

Hey. Probably I will have some time to work on it this week.

mmicu · 2021-07-13T16:55:24Z

Hi @billsioros. I added this kind of offline mode in the version 2.3.51. Could you please try it and let me know?

billsioros · 2021-07-14T20:24:35Z

Hi @mmicu, excuse me but I fail to notice a new release 😢

mmicu · 2021-07-14T21:09:01Z

Don't worry. Take your time to test it.

billsioros · 2021-07-14T21:15:22Z

Don't worry. Take your time to test it.

What I meant is that I cannot find a release with the tag 2.3.51 for example https://github.com/editorconfig-checker/editorconfig-checker.python/tree/2.3.51. Excuse me for not being clear enough.

mmicu · 2021-07-14T21:28:34Z

What I meant is that I cannot find a release with the tag 2.3.51 for example https://github.com/editorconfig-checker/editorconfig-checker.python/tree/2.3.51. Excuse me for not being clear enough.

No worries. It's in PyPI. You can install the latest version with pip install editorconfig-checker. Alternatively, you can use pip install . if you want to install it from the repo itself (offline-mode branch).

In any case, from the pre-commit-config that you posted in the first message, it will be enough to update the version from 2.3.5 to 2.3.51.

billsioros · 2021-07-15T11:01:08Z

What I meant is that I cannot find a release with the tag 2.3.51 for example https://github.com/editorconfig-checker/editorconfig-checker.python/tree/2.3.51. Excuse me for not being clear enough.

No worries. It's in PyPI. You can install the latest version with pip install editorconfig-checker. Alternatively, you can use pip install . if you want to install it from the repo itself (offline-mode branch).

In any case, from the pre-commit-config that you posted in the first message, it will be enough to update the version from 2.3.5 to 2.3.51.

Hello again,

The CI fails on build time. You can find the run here. I'm pretty sure pre-commit does not install the hooks via pip but via git clone, thus the following error is raised

cloning https://github.com/editorconfig-checker/editorconfig-checker.python:

fetch attempt 1...
    => error
    fatal: couldn't find remote ref 2.3.51
fetch attempt 2...
    => error
    fatal: couldn't find remote ref 2.3.51
fetch attempt 3...
    => error
    fatal: couldn't find remote ref 2.3.51

I believe a new release/tag must be created before even being able to test this out.

Edit: Forgot to include a link to the pre-commit-config.yaml, which I changed. 😋

mmicu · 2021-07-15T12:01:28Z

I created the tag 2.3.51. Could you please try to test it again?

billsioros · 2021-07-15T13:20:25Z

I created the tag 2.3.51. Could you please try to test it again?

Thank you very much! I'm facing another issue. The binary seems to be large and it is failing to install in the CI due to a max-size limit. Here, you can find the corresponding CI run. I might have to contact the maintainers of pre-commit to find a solution for this, but out of curiosity, could the binary be shrinked maybe ?

mstruebing · 2021-07-15T13:30:33Z

I just had a quick look because I couldn't believe it would be so big.
editoroconfig-checker/lib/ contains every binary for every platform:

➜ editorconfig-checker.python-2.3.51
$ tree
.
├── LICENSE
├── Makefile
├── README.md
├── docs
│   ├── logo.png
│   └── sample-output.png
├── editorconfig_checker
│   ├── __init__.py
│   ├── __main__.py
│   ├── lib
│   │   ├── __init__.py
│   │   ├── ec-darwin-386.tar.gz
│   │   ├── ec-darwin-amd64.tar.gz
│   │   ├── ec-dragonfly-amd64.tar.gz
│   │   ├── ec-freebsd-386.tar.gz
│   │   ├── ec-freebsd-amd64.tar.gz
│   │   ├── ec-freebsd-arm.tar.gz
│   │   ├── ec-linux-386.tar.gz
│   │   ├── ec-linux-amd64.tar.gz
│   │   ├── ec-linux-arm.tar.gz
│   │   ├── ec-linux-arm64.tar.gz
│   │   ├── ec-linux-mips.tar.gz
│   │   ├── ec-linux-mips64.tar.gz
│   │   ├── ec-linux-mips64le.tar.gz
│   │   ├── ec-linux-mipsle.tar.gz
│   │   ├── ec-linux-ppc64.tar.gz
│   │   ├── ec-linux-ppc64le.tar.gz
│   │   ├── ec-netbsd-386.tar.gz
│   │   ├── ec-netbsd-amd64.tar.gz
│   │   ├── ec-netbsd-arm.tar.gz
│   │   ├── ec-openbsd-386.tar.gz
│   │   ├── ec-openbsd-amd64.tar.gz
│   │   ├── ec-openbsd-arm.tar.gz
│   │   ├── ec-plan9-386.tar.gz
│   │   ├── ec-plan9-amd64.tar.gz
│   │   ├── ec-solaris-amd64.tar.gz
│   │   ├── ec-windows-386.exe.tar.gz
│   │   └── ec-windows-amd64.exe.tar.gz
│   └── wrapper.py
├── publish.sh
├── requirements.txt
├── setup.py
├── test.sh
├── tests
│   └── Dockerfile.template
└── update-binaries.sh

4 directories, 42 files

I do not know anything about the python ecosystem and how everything works here, but is this needed?
At least it explains your error @billsioros

mmicu · 2021-07-15T13:37:45Z

I had to add every possible binary for this kind of offline mode. The Python wrapper simply runs the executable, so it must be chosen based on the target machine.

The new logic also removes the unused tarballs and extracts content from the one that matches the target machine. However, since there is no chance to know the target machine, I had to include all of them. Then, network is not available for pre-commit, so I cannot download a single tarball (as I was doing with the previous version).

mstruebing · 2021-07-15T13:43:31Z

I understand, thank you for that explanation.

So I see two options here:

Remove some of the binaries and lose access for them in offline mode (could be loaded later on ofc if connected to the internet)
Do not change anything and make the other tools responsible for having enough space

Personally I would vote for 2. - but as I'm not the one who is using this and developed it, don't take my opinion to serious.
Altough, it might make sense to think of if offline mode is needed at all.

mmicu · 2021-07-15T14:03:36Z

I understand, thank you for that explanation.

No problem.

Personally I would vote for 2. - but as I'm not the one who is using this and developed it, don't take my opinion to serious.
Altough, it might make sense to think of if offline mode is needed at all.

I would go for 2. as well. If we want to use this offline mode, I would keep all the binaries even if the resulted package becomes quite big. If you want to install this package in a machine with no internet connection, then this feature becomes fundamental. For sure we are talking about 200MB against a few ones, but at least we can also support offline machines now.

Regarding the size limit, of course we cannot adapt our tool to external ones. So, as you wrote, they are responsible for having enough space.

@billsioros, I expect a kind of parameter to increase the size limit. Maybe you can check if the tool support such option.

asottile · 2021-07-15T23:07:43Z

A better way to do this would be to download the platform specific binaries during the setup.py execution and not bundle them all as checked in binaries in your repository

you can find an example of this in shellcheck-py -- you would pick the architecture of your particular running python and encode that in your produced wheel (if applicable). then you wouldn't run up against the download limits and the no-network-at-runtime bit would be satisfied

here's the particular code from shellcheck-py: https://github.com/shellcheck-py/shellcheck-py/blob/master/setup.py

mmicu · 2021-07-16T08:36:29Z

Hi @asottile, thank you for your suggestion. Yes, you're right. That would be the best solution to apply. I will work on it soon.

sergei-ivanov · 2021-07-24T17:13:09Z

As an alternative, I have just proposed #16 that skips all automatic downloading altogether. We use ASDF to manage the tools locally, and on CI we have a custom docker image that already bundles all necessary tools (also provisioned via ASDF at image build time).

When I forked the current repo, git clone pulled down 100Mb, all because of the bundled binaries on the offline-mode branch. This means that when it's merged into master, anyone who declares the hook in their .pre-commit-config.yaml will have to pull down 100Mb of stuff, most part of which will be useless. I don't think it's a great idea, to be honest. I would rather shift the responsibility on CI admins to make sure that the right versions of the tools are pre-installed.

EDIT: re-reading the whole thread, I think @asottile's suggestion makes a lot of sense. It will fix running the python hook on pre-commit CI within its rather restrictive environment. But we would still prefer an alternative no-download hook.

billsioros · 2021-07-27T12:11:17Z

Hi @asottile, thank you for your suggestion. Yes, you're right. That would be the best solution to apply. I will work on it soon.

Hello yet again! Are there any news on this? 😃

mmicu · 2021-07-27T14:42:54Z

Hi @sergei-ivanov and @billsioros. I will implement what @asottile proposed once I am back at work.

mmicu · 2021-08-07T09:05:23Z

@billsioros, could you please try to install version 2.3.52? Please, note that in this new version you need to use ec instead of editorconfig-checker in order to run the tool.

billsioros · 2021-08-09T10:21:33Z

What I meant is that I cannot find a release with the tag 2.3.51 for example https://github.com/editorconfig-checker/editorconfig-checker.python/tree/2.3.51. Excuse me for not being clear enough.

No worries. It's in PyPI. You can install the latest version with pip install editorconfig-checker. Alternatively, you can use pip install . if you want to install it from the repo itself (offline-mode branch).
In any case, from the pre-commit-config that you posted in the first message, it will be enough to update the version from 2.3.5 to 2.3.51.

Hello again,

The CI fails on build time. You can find the run here. I'm pretty sure pre-commit does not install the hooks via pip but via git clone, thus the following error is raised
cloning https://github.com/editorconfig-checker/editorconfig-checker.python:

fetch attempt 1...
    => error
    fatal: couldn't find remote ref 2.3.51
fetch attempt 2...
    => error
    fatal: couldn't find remote ref 2.3.51
fetch attempt 3...
    => error
    fatal: couldn't find remote ref 2.3.51
I believe a new release/tag must be created before even being able to test this out.

Edit: Forgot to include a link to the pre-commit-config.yaml, which I changed. 😋

@mmicu Hello! Would it be possible to create a new GitHub release? As I've mentioned in a previous comment pre-commit does not use pip to install the hooks specified by the config file and as a result I'm unable to test the hook out 😢 If you have in mind a way of testing this out without creating a new release please tell me so and I'll be glad to do so!

mmicu · 2021-08-09T10:59:00Z

@billsioros, you're right, I completely forgot about that. I created the new tag 2.3.52.

billsioros · 2021-08-09T11:17:46Z

@mmicu I was able to update the hook. As you mentioned before the executable is now named ec. If I'm not mistaken a corresponding change should be made to .pre-commit-hooks.yaml.

-   id: editorconfig-checker
    name: editorconfig-checker
    description: '`editorconfig-checker` is a tool to check if your files consider your .editorconfig-rules.'
    entry: editorconfig-checker # should be ec
    language: python
    types: [text]
    require_serial: true

DISCLAIMER: I have never developed a pre-commit hook, so I'm going out on a limb here to say this 😄

mmicu · 2021-08-09T11:23:36Z

@billsioros, @sergei-ivanov created a pull request for that.

Once this issue will be closed and the corresponding code merged to master, he can rebase and change both .pre-commit-hooks.yaml and README as you suggested.

sergei-ivanov · 2021-08-09T11:27:36Z

@mmicu No, my PR is slightly different. It bypasses Python completely, and relies on ec to be in the $PATH. It's complementary to the existing hook, it gives the users the choice to manage ec download and installation outside of pre-commit.

EDIT: you will likely need to modify the existing hook as @billsioros has suggested. And when its all merged, I'll rebase my PR on top of it.

mmicu · 2021-08-09T11:58:36Z

@sergei-ivanov, Thank you for the clarification. Then, let's continue the discussion in the PR page directly.

@billsioros, I will change the README based on this fix as you suggested. However, could you please share a link with a successful run with the latest version?

sergei-ivanov · 2021-08-09T12:04:14Z

@mmicu I think what @billsioros meant was: if the executable changed to ec you will need to update this line in this repo:

editorconfig-checker.python/.pre-commit-hooks.yaml

Line 4 in 9a54318

entry: editorconfig-checker

...to this:

    entry: ec

This is because it cannot be controlled by the consumer of the hook, it is set up by the hook author.

mmicu · 2021-08-09T12:06:29Z

@sergei-ivanov, yes, I forgot to mention it, but I meant both .pre-commit-hooks.yaml and README.

billsioros · 2021-08-09T12:11:21Z

@mmicu I haven't been able to get a successful run on the CI. After the recent update, which introduced the executable name change, I'm not able to get a successful run locally either.

mmicu · 2021-08-09T12:17:46Z

@billsioros, could you please try now? The new version is 2.3.53.

billsioros · 2021-08-09T13:49:02Z

@mmicu The hook works great! Thank you very much for your efforts! Feel free to close the issue. Thanks again 😄

mmicu · 2021-08-10T12:35:17Z

@billsioros, great, I am glad that it worked.

Thank you all for you help and suggestions.

billsioros mentioned this issue Jul 1, 2021

Failing to run pre-commit hook pre-commit-ci/issues#67

Closed

billsioros mentioned this issue Jul 1, 2021

ci: do not skip editorconfig-checker on pre-commit.ci the-dotify-project/dotify#99

Closed

mstruebing mentioned this issue Jul 16, 2021

Load correct binary at install time, not at first runtime editorconfig-checker/editorconfig-checker.javascript#292

Open

sergei-ivanov mentioned this issue Aug 9, 2021

Add system pre-commit hook #16

Merged

mmicu closed this as completed Aug 10, 2021

sergei-ivanov mentioned this issue Aug 17, 2021

Move logic to setup.py #17

Merged

pre-commit hook fails while running on pre-commit.ci #15

pre-commit hook fails while running on pre-commit.ci #15

Comments

billsioros commented Jun 29, 2021

mmicu commented Jul 1, 2021

billsioros commented Jul 1, 2021

billsioros commented Jul 1, 2021

mmicu commented Jul 1, 2021

billsioros commented Jul 8, 2021

mmicu commented Jul 8, 2021

mmicu commented Jul 13, 2021

billsioros commented Jul 14, 2021

mmicu commented Jul 14, 2021

billsioros commented Jul 14, 2021 • edited Loading

mmicu commented Jul 14, 2021 • edited Loading

billsioros commented Jul 15, 2021 • edited Loading

mmicu commented Jul 15, 2021

billsioros commented Jul 15, 2021

mstruebing commented Jul 15, 2021

mmicu commented Jul 15, 2021

mstruebing commented Jul 15, 2021

mmicu commented Jul 15, 2021 • edited Loading

asottile commented Jul 15, 2021

mmicu commented Jul 16, 2021

sergei-ivanov commented Jul 24, 2021 • edited Loading

billsioros commented Jul 27, 2021

mmicu commented Jul 27, 2021 • edited Loading

mmicu commented Aug 7, 2021 • edited Loading

billsioros commented Aug 9, 2021

mmicu commented Aug 9, 2021

billsioros commented Aug 9, 2021 • edited Loading

mmicu commented Aug 9, 2021

sergei-ivanov commented Aug 9, 2021 • edited Loading

mmicu commented Aug 9, 2021

sergei-ivanov commented Aug 9, 2021

mmicu commented Aug 9, 2021

billsioros commented Aug 9, 2021

mmicu commented Aug 9, 2021

billsioros commented Aug 9, 2021

mmicu commented Aug 10, 2021

billsioros commented Jul 14, 2021 •

edited

Loading

mmicu commented Jul 14, 2021 •

edited

Loading

billsioros commented Jul 15, 2021 •

edited

Loading

mmicu commented Jul 15, 2021 •

edited

Loading

sergei-ivanov commented Jul 24, 2021 •

edited

Loading

mmicu commented Jul 27, 2021 •

edited

Loading

mmicu commented Aug 7, 2021 •

edited

Loading

billsioros commented Aug 9, 2021 •

edited

Loading

sergei-ivanov commented Aug 9, 2021 •

edited

Loading