florianschanda / miss_hit

MATLAB Independent, Small & Safe, High Integrity Tools - code formatter and more
GNU General Public License v3.0
164 stars 20 forks source link

"strange" interaction with m_style and pre-commit #227

Open Remi-Gau opened 3 years ago

Remi-Gau commented 3 years ago

miss_hit complains about an invalid config file when running a hook but is fine otherwise

MISS_HIT Component affected

Possibly other components: I have not checked yet

Your operating system and Python version

This happens on this "sandbox" repo: https://github.com/Remi-Gau/clean_code_style

This should help reproduce the issue:

git clone https://github.com/Remi-Gau/clean_code_style.git
cd clean_code_style

# install latest miss_hit and pre-commit
pip install -r requirements.txt
pre-commit install

# make some changes
mh_copyright --add-notice S2/*.m

# try to commit
git add --all
git commit -m 'add copyright'

This throws this error:

mh_style.................................................................Failed
- hook id: mh_style
- exit code: 1

miss_hit.cfg: error: config file contains errors
In miss_hit.cfg, line 10
| suppress_rule: "naming_parameters"
|                ^^^^^^^^^^^^^^^^^^^ error: expected valid style rule name
.: error: cannot find project root because the config file contains errors: please add a config file with the 'project_root' directive
MISS_HIT Style Summary: 2 file(s) analysed, 3 error(s)

But simply running mh_style works fine.

Not sure what is happening and I suspect this might be some weird interaction between miss hit and pre-commit

florianschanda commented 3 years ago

That rule (naming_parameters) was added quite recently, and that error is consistent with running an older version. Can you check please which version of miss_hit is used / is installed by the hook? It might not be the same (for whatever reason) as your system installed version.

florianschanda commented 3 years ago

Also, I tried running through your instructions above and I was able to commit:

flosch@fat-potato:~/clean_code_style$ git commit -m 'add copyright'
[main e59354e] add copyright
 3 files changed, 5 insertions(+)

Is there a missing instruction for pre-commit, or did it just silently work?

florianschanda commented 3 years ago

I think I will add way to show the version (in addition to -v) in the generated output, to help debug issues like this.

Remi-Gau commented 3 years ago

Also, I tried running through your instructions above and I was able to commit:

flosch@fat-potato:~/clean_code_style$ git commit -m 'add copyright'
[main e59354e] add copyright
 3 files changed, 5 insertions(+)

Is there a missing instruction for pre-commit, or did it just silently work?

I think I had forgotten an instruction to run pre-commit install: I updated my original post.

Remi-Gau commented 3 years ago

That rule (naming_parameters) was added quite recently, and that error is consistent with running an older version. Can you check please which version of miss_hit is used / is installed by the hook? It might not be the same (for whatever reason) as your system installed version.

Hum... Whether I use a virtual environment or my base conda environment I get the same version. I must be messing something up but not sure what.

(env): mh_style -v
MISS_HIT 0.9.25
florianschanda commented 3 years ago

You could try changing your hook to invoke mh_style -v instead and just observe the version number produced. I think knowing precisely what is run is the first step in resolving this issue:

florianschanda commented 3 years ago

@Remi-Gau do you have any updates/news here to point me in the right direction?

Remi-Gau commented 3 years ago

hey only getting back to this now as I am finally bumping miss_hit version on some of my main projects.

So the hook is not using the right version.

This is the version in my environment.

╰─(env) ⠠⠵ mh_style -v
MISS_HIT 0.9.28

If I use this hook with pre commit:

repos:

-   repo: local

    hooks:

    - id: mh_version
      name: mh_version
      entry: mh_style
      args: [-v]
      verbose: true
      language: python
      additional_dependencies: [miss_hit_core]

I get this:

mh_version...............................................................Passed
- hook id: mh_version
- duration: 0.06s

MISS_HIT 0.9.22
Remi-Gau commented 3 years ago

More confusion.

Outside of all my environments I get this

─remi at remi-XPS-15-9570
╰─○ mh_style -v
MISS_HIT 0.9.18-dev

My base conda environment gives me this

(base) ╭─remi at remi-XPS-15-9570
╰─⠠⠵ mh_style -v     
MISS_HIT 0.9.20

So I am not quite sure where the hook is getting its command from.

florianschanda commented 3 years ago

Ugh :(

I think, when I looked at this pre-commit beast, they did some "clever" stuff with installing things in some local environment; and that the way we configured it was not the official recommended one. And that didn't work because I didn't quite set up the mh repo structure right and it would have been a major pain to fix it.

Remi-Gau commented 3 years ago

Do you want me to open an issue on the pre-commit repo to ask for guidance?

florianschanda commented 3 years ago

I suspect we'll hear the same thing we got told last time: make the miss_hit repo conform by having only a single setup.py.

There is good reasons why I don't want to do that; because it makes developing and running MISS_HIT way harder (I'd have to mess around with pythonpath all the time).

But maybe there is a better way we can set up the hook; or at least force an update of the tool in the local checked-in version that pre-commit seems to use. I wish it could be made to just use the system installed one, but apparently that is not so easy :/

Remi-Gau commented 3 years ago

OK I see.

Will look at regular interval if I can find a way to set up the hook differently.

In the meantime I think I will just try to make sure I always use the latest version of miss_hit in all my environments.