Skip to content

Conversation

@taoufik07
Copy link
Contributor

Custom envs can't be passed to normalize_cmd which causes find_executable to default to os.environ -- works as expected when injecting directly os.environ.


def find_executable(
exe: str, _environ: Mapping[str, str] | None = None,
exe: str, *, _environ: Mapping[str, str] | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's public it shouldn't be underscored. env is probably what it sounds be called to match subprocess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@taoufik07 taoufik07 force-pushed the fix_cmd_norm_env branch 2 times, most recently from 411ef96 to 338de08 Compare January 5, 2023 20:39
homedir = tempdir_factory.get()
ret, out = git_commit(
env={
**os.environ,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm why did this need to change?

Copy link
Contributor Author

@taoufik07 taoufik07 Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was missing PATHEXT for windows so it couldn't find git. I pushed the right change, it should be good now.

@taoufik07 taoufik07 force-pushed the fix_cmd_norm_env branch 3 times, most recently from f7fecc1 to ada060c Compare January 5, 2023 22:56
Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asottile asottile enabled auto-merge January 6, 2023 14:28
@asottile asottile merged commit dc667ab into pre-commit:main Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants