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.

Sign up for GitHub

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

Jump to bottom

Fix environment-dependent git pre-commit hooks #10165

Open
janosh opened this issue Feb 18, 2020 · 51 comments
Open

Fix environment-dependent git pre-commit hooks #10165

janosh opened this issue Feb 18, 2020 · 51 comments
Labels
area-internal Label for non-user facing issues feature-request Request for new features or functionality needs proposal Need to make some design decisions

Comments

@janosh
Copy link

janosh commented Feb 18, 2020

This is a follow up to #9948 and vscode#90178. After looking at the problem of environment-dependent pre-commit hooks from both the extension's and the editor's side, it seems to me the best solution would be for the Python extension to make sure that whatever environment the user has selected in the status bar is also active during the commit process (using VS Code's source control panel).

I think this would be the expected behavior by most users if you asked them. At least it took me by surprise that my active environment doesn't apply during the commit process even though it's indicated in the status bar. Hence why it took me a while to figure out this is the reason my pre-commit hooks aren't working.

One way to activate the status bar environment in the spawned git process might be as @joaomoreno mentioned to have a git wrapper that's local to the source control panel (i.e. doesn't affect git anywhere else) and simply activates the environment before handing over to real git.
@janosh janosh added triage-needed Needs assignment to the proper sub-team feature-request Request for new features or functionality labels Feb 18, 2020
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Feb 20, 2020
@karthiknadig
Copy link
Member

@joaomoreno @janosh In think the best (generic) way is here is if python extension can provide the environment variables. This way pyhton extension can provide the activated environment variables or set the PATH variable (depends on the type of environment). To achieve this the extension will need a hook from VS Code so we can provide these env vars when it spawns git process.

@karthiknadig
Copy link
Member

We have marked this issue as "needs decision" to make sure we have a conversation about your idea. We plan to leave this feature request open for at least a month to see how many 👍 votes the opening comment gets to help us make our decision.

@luabud luabud added feature-* needs proposal Need to make some design decisions and removed needs decision labels Mar 25, 2020
@luabud luabud added area-internal Label for non-user facing issues and removed feature-* labels May 13, 2020
@TobiRoby
Copy link

Any updates? I would realy appreciate this behaviour!

@karthiknadig
Copy link
Member

Sorry, no updates on this yet.

@janosh janosh mentioned this issue Jul 17, 2020
Closed
@janosh
Copy link
Author

janosh commented Jul 18, 2020

In my particular case of git hooks powered by pre-commit, an obvious workaround (in hindsight) - as suggested by @asottile in pre-commit#1537 - is to keep the commit hooks and the environment separate.

That is, uninstall pre-commit confined to the virtual/conda/whatever environment (pip uninstall pre-commit) and reinstall it globally (on macOS brew install pre-commit). Simple as that.

Might still be worth it for the Python extension to activate the environment indicated by the status bar during the commit process for cases where a global solution does not exist.

@ervteng
Copy link

ervteng commented Jul 21, 2020

I'm also having this issue, but only recently. Actually, pre-commit was working fine until the last update of VSCode, so not sure what changed.

@jaygorrell
Copy link

Might still be worth it for the Python extension to activate the environment indicated by the status bar during the commit process for cases where a global solution does not exist.

Definitely. In my case, a commit hook runs pyright and needs access to the virtual environment. Thanks for this issue... it took me a while to find one.

@Wesleyliao
Copy link

I'm having this issue as well, also recently and on Windows. Specifically the VS Code Source Control panel uses the wrong Python path (does not respect the interpreter selected in the Status Bar) during commit when pre-commit is installed.

@LudwigStumpp
Copy link

LudwigStumpp commented Apr 13, 2021 •
edited
Loading

Same issue for me. Do not have access to the activated conda environment when using the VS Code Source Control Panel.

What I am trying to do is to run pytest (installed in virtual anaconda environment) as a pre-commit hook using the pre-commit library. When commiting from the Integrated Terminal, everything is fine since it can locate the installed pytest library (installed in virtual anaconda environment). However commiting from the Source Control Panel, there is an error saying Executable "pytest" not found.

@LudwigStumpp
Copy link

LudwigStumpp commented Apr 13, 2021 •
edited
Loading

One way to activate the status bar environment in the spawned git process might be as @joaomoreno mentioned to have a git wrapper that's local to the source control panel (i.e. doesn't affect git anywhere else) and simply activates the environment before handing over to real git.

Here is how I managed to get it working:

Create file test.sh that first activates the environment virtual_environment and then runs the test using pytest:

#!/bin/bash
source ~/miniconda3/etc/profile.d/conda.sh
conda activate virtual_environment
pytest

And call it from within .pre-commit-config.yaml

- repo: local
  hooks:
    - id: pytest-check
      name: pytest-check
      entry: bash test.sh
      language: system

@chris1248
Copy link

Just make the git commit hooks respect the python environment selected in the bottom status bar.
Why are we sitting here 3 months later arguing about this?

@guoquan
Copy link

guoquan commented Jul 31, 2021

Any progress on this? My pre-commit hooks have been being consoled only for a while.

@LudwigStumpp s solution seems to work for any individual.

Here is how I managed to get it working:

Create file test.sh that first activates the environment virtual_environment and then runs the test using pytest:

However, when it comes to team development, I don't think it is possible for the team to share the same wrap shell. We use the same python, but there could be different virtual environment names, different managers (conda or venv), or even different shells.

Actually, I had a similar issue when I was working with node.js 's git pre-commit hook, where git did not active correct npm version.

git module should really resolve and load users' real console for running its commands.

@LudwigStumpp
Copy link

@guoquan could you please upvote my solution so that others see that it is working? :)
Let's hope that this gets fixed soon, though!

@guoquan
Copy link

guoquan commented Aug 17, 2021

@LudwigStumpp Definitely! That should solve the issue for independent developers.

What I wanted to bring into the discussion is, in team development, one can only make very humble assumptions to this test.sh. For example, people in one team may use conda, miniconda, or venv; people may use different names for the environment.

Digging into pre-commit, it seems it will create and maintain venv by itself for checking. So I am thinking, pre-commit doesn't want the developers to take care of the environment. Thus "environment-dependent" is a pseudo‑problem. The only thing needed in the environment governed by the user is pre-commit.

So my current workaround is to install (only) pre-commit in the system-wise python.

Looking back at the issue, what we need now is vsc to activate the correct environment that contains pre-commit.

@JoshFerge
Copy link

any updates on this?

@karthiknadig
Copy link
Member

@JoshFerge this issue haven't been prioritized yet. We plan to do it eventually but no ETAs for that yet.

@karthiknadig karthiknadig removed their assignment Oct 19, 2021
This was referenced Jan 19, 2023
Closed
Closed
synrg added a commit to dronefly-garden/dronefly that referenced this issue Jan 29, 2023
@synrg
Remove pre-commit until vscode plays nicely with it.
8ee7095 
- The unresolved issue is that vscode doesn't play nice with pre-commit installed
  in the venv: microsoft/vscode-python#10165
- For now, the accepted workaround seems to be to install pre-commit for
  in your system python instance.
@afischer-opentext-com
Copy link

afischer-opentext-com commented Feb 22, 2023 •
edited
Loading

Since pip 23/pep 668 we are discouraged to deploy pre-commit into the systems python deployment. As a consequence this feature is more needed than ever before. While pre-commit may still be available, the use of local pre-commit-hooks (which allow to decouple from systems such as GitHub) no longer works.

Also, the workaround using the git.path setting does not work as I do not see that setting in current versions of vscode. Has this been renamed?

@Upgwades
Copy link

Adding onto #10165 (comment) for the specific issue around pytest local testing this worked for me when running git commit via VSCODE to run pytest without defining an extra script:

  - repo: local
    hooks:
      - id: pytest
        name: pytest
        language: system
        entry: poetry run pytest
        pass_filenames: false
        always_run: true

@lszomoru lszomoru mentioned this issue Mar 24, 2023
Closed
@nathanpovo nathanpovo mentioned this issue Mar 25, 2023
Open
@stefanbschneider
Copy link

stefanbschneider commented Apr 11, 2023 •
edited
Loading

To me, it's not even clear which environment the pre-commit hook inside VSCode uses? Conda's base env or sth else? It doesn't find one of the dependencies that is installed in the correct env. Ideally, the pre-commit would also use the env. As a workaround, I could also install the dependency elsewhere; but I don't even know where.

This is easily the most annoying part about VSCode atm...

@dvdstrzl dvdstrzl mentioned this issue May 3, 2023
Closed
@Paul-Durrant
Copy link

I have a pre-commit hook that calls mypy. It ought to work, but mypy isn't found, apparently because the remote virtual environment isn't being activated before git is called.

Terminals get the environment activated correctly. So commits work in the terminal.

I can, in an ugly way involving full paths, use the localgit suggestion above, so that git runs in my remote virtual environment. But I really didn't expect to have to do this.

@LudwigStumpp
Copy link

Adding onto #10165 (comment) for the specific issue around pytest local testing this worked for me when running git commit via VSCODE to run pytest without defining an extra script:

  - repo: local
    hooks:
      - id: pytest
        name: pytest
        language: system
        entry: poetry run pytest
        pass_filenames: false
        always_run: true

Works assuming one uses Poetry.

@gresavage
Copy link

How is this still open after 3+ years? Kind of absurd when you think about how simple the fix is and how many people are having the issue.

@brettcannon
Copy link
Member

@gresavage it's actually not simple as the SCM component of VS Code has no concept of Python interpreters or environments. So to make this work the SCM system in VS Code core would need to be expanded so we could somehow inject environment details so that git itself picks up. This would probably come about via environment variables and we are actively reworking our environment activation support in the terminal to use environment variables, so hopefully this work can then inform an API change in VS Code core for SCM execution and we can build one feature on top of the other's hard work.

Also note that this issue finally hit the top 5 most upvoted issues, but it's also half the number of upvotes of the first and second issue (and the first issue is actually newer than this one). So while we understand this impacts people, we still have other issues that the community has flagged as more important to them overall. We are definitely not ignoring this issue, but our team is only so big and we are expected to support all Python users in VS Code which is not an insignificant number based on our download count.

But if you believe I have misunderstood the complexity of what it will take to resolve this problem, then this extension and VS Code are open source and we do appreciate and accept community contributions. If you or anyone else wants to propose a fix and a PR for it we will happily review it.

@gresavage
Copy link

gresavage commented Aug 23, 2023 via email

@brettcannon
Copy link
Member

Until your comment it seemed like there was little to no activity from devs towards closing this issue.

As you can see by our issue count, there's unfortunately simply too many issues for us to keep all ancillary issues up-to-date while we are working on something. Plus we don't want to give people false hopes on when things will be completed.

Maybe some fleshed out CLI documentation

"CLI documentation" for which CLI? code?

@gresavage
Copy link

gresavage commented Aug 23, 2023 via email

@brettcannon
Copy link
Member

This is all relatively unrelated to the original thread but in particular it would be nice if the CLI

All feedback on the CLI should go to https://github.com/microsoft/vscode .

@kfigiela
Copy link

Not sure if this helps, but we had a similar problem, however, we're using pre-commit hooks via nix (https://github.com/cachix/pre-commit-hooks.nix). Nevertheless, I think our workaround could be adapted to other circumstances like python virtual environment. Essentially, we're "polluting" pre-commit hooks, so they'll load the necessary environment without needing VSCode to run in the environment. I hope this helps:

  1. We use direnv to load nix environment
  2. We enable nix in .envrc via use nix. That could be replaced with appropriate code to load python env without the need of nix.
  3. We update pre-commit hook scripts with a small shell script. This is all managed by nix in our case, so pre-commit hooks as well as the patch are installed automatically
    if [ -f .envrc ] && [ -f .git/hooks/pre-commit ]; then
      cat .git/hooks/pre-commit | grep --silent "direnv export bash" || sed --in-place '2 i eval "$(direnv export bash)"' .git/hooks/pre-commit
    fi

@gresavage
Copy link

This is all relatively unrelated to the original thread but in particular it would be nice if the CLI

All feedback on the CLI should go to https://github.com/microsoft/vscode .

very good, will do!

@kfigiela very good! That is actually pretty similar to what I've done as well - I also use sed to insert the commands for activating my conda environment in the shell opened by .git/hooks/pre-commit... I will have to look at automating it more as you have done.

My biggest gripe with doing it this way is that if the project/workspace interpreter is changed later on by the user then the script will need to be updated and then sed gets a little trickier if you want to clean up the prior statements. I have not yet been able to discover where VS Code (or VS Code Server, for that matter) store the setting of the workspace interpreter once it's been set through the command pallet (note the setting from command pallet takes precedence over the python.defaultInterpreter setting in settings.json). This is another example of where the documentation, overall API, and especially CLI could be improved

@brettcannon brettcannon mentioned this issue Dec 18, 2023
Closed
@kr-hansen
Copy link

I've been poking around on this and am considering trying to put together an Open Source Contribution (though I'll have some learning to do with VSCode extensions) to try resolving this, but have a couple questions I'm trying to track down/understand before I can do that.

  1. When going to the VSCode Output Tab with "Git" selected, is there a way for me to figure out what shell/terminal that is using? I don't have pre-commit on my system environment, so the fact it is starting it and failing by missing a requirement suggests it isn't using my base system environment.

  2. Re: Discussion on environment variables. I'm using pyenv, which sets an environment variable called VIRTUAL_ENV that seems to point to the version of python in the Terminal that I've assigned to my repo as expected. This may work to pass to SCM when Commit is hit from the UI, as I imagine this environment variable will be set appropriately. Not sure if that's true for other tools like conda. I'm open to trying this though as a first pass, if I can figure out what environment the Git Output in SCM is running in. Or is there something better that the Python Extensions surfaces that it can pass off to the SCM extension?

  3. In the meantime, I've found that having pre-commit in my environment, not installing it with pre-commit install and using the pre-commit extension actually does an ok job as a stop-gap. It doesn't block commits like pre-commit does if there are errors, but if I have the RunOnSave property set to all-hooks, it'll run my standard hooks I want and error in the appropriate environment on the Terminal tab on VSCode. If pre-commit isn't fully installed, than SCM works fine and doesn't get blocked. Using fast linters/formatters like ruff or black work well in this setup. Example of a failure:

failed_precommit_hook

Either way, I figured I'd surface what I'd learned and prompt with any potential help to try getting momentum on a potential solution going here.

@gresavage
Copy link

@kr-hansen thanks for the input. Here is what I can say in regards to (1) and (2)

  1. When going to the VSCode Output Tab with "Git" selected, is there a way for me to figure out what shell/terminal that is using? I don't have pre-commit on my system environment, so the fact it is starting it and failing by missing a requirement suggests it isn't using my base system environment.

In my case it is using /usr/bin/bash, the shebang at the beginning of the hook file should give an clue of which shell is being used. To be certain you can echo the SHELL environment variable from within your /<path>/<to>/<project>/.git/hooks/pre-commit file like so:

echo SHELL="${SHELL}"

This means that there is no project-specific environment configuration being done - just whatever is in ${HOME}/.bashrc

  1. Re: Discussion on environment variables. I'm using pyenv, which sets an environment variable called VIRTUAL_ENV that seems to point to the version of python in the Terminal that I've assigned to my repo as expected. This may work to pass to SCM when Commit is hit from the UI, as I imagine this environment variable will be set appropriately. Not sure if that's true for other tools like conda. I'm open to trying this though as a first pass, if I can figure out what environment the Git Output in SCM is running in. Or is there something better that the Python Extensions surfaces that it can pass off to the SCM extension?

With conda you should call the activate sub-command rather than setting environment variables directly:

conda activate <env_name>

@enricostragiotti enricostragiotti mentioned this issue Jul 5, 2024
Closed
@stefanbschneider
Copy link

Not a solution but a simple workaround: Start code from the terminal from within the environment that you want to use for pre-commit. It seems like VSCode then uses this environment for the system hooks.

@kieferrm kieferrm mentioned this issue Aug 4, 2024
Closed
44 tasks
@kieferrm kieferrm mentioned this issue Sep 10, 2024
Closed
20 tasks
@kieferrm kieferrm mentioned this issue Oct 6, 2024
Closed
27 tasks
@kieferrm kieferrm mentioned this issue Nov 3, 2024
Open
31 tasks
@KrisThielemans KrisThielemans mentioned this issue Nov 7, 2024
Merged
@zpoint zpoint mentioned this issue Nov 12, 2024
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-internal Label for non-user facing issues feature-request Request for new features or functionality needs proposal Need to make some design decisions
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests