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.

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

VirtualEnv's pip install -r requirements.txt fails because git behaves differently when shell=False passed to subprocess.run() #365

Open
andyk opened this issue Apr 18, 2022 · 3 comments

Comments

@andyk
Copy link
Contributor

andyk commented Apr 18, 2022

Including PATH in the env we pass to the subprocess.run causes my git to use /usr/local/git/etc/gitconfig which refers to files that don't exist in my homedir (~/.gitcinclude, `~/.gitignore, etc.).

agentos/pcs/virtual_env.py

Lines 214 to 219 in 4030b81

component_env = {
"SYSTEMROOT": os.environ.get("SYSTEMROOT", ""), # win32
"VIRTUAL_ENV": str(self.venv_path),
"PATH": f"{str(bin_path)}:{os.environ.get('PATH')}",
}
subprocess.run(cmd, env=component_env)

I verified this by running the following after activating virtualenv created by agentos for the agent:

python
>>> import subprocess
>>> subprocess.run(['/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin/python', '-m', 'pip', 'install', '-r', '/Users/andyk/Development/agentos/example_agents/papag/requirements.txt'], env={'SYSTEMROOT': '', 'VIRTUAL_ENV': '/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226', 'PATH': '/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin:/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/Users/andyk/.gem/ruby/2.7.0/bin:/opt/homebrew/Caskroom/miniforge/base/envs/agentos_dev2/bin:/opt/homebrew/Caskroom/miniforge/base/condabin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin:/Applications/Postgres.app/Contents/Versions/latest/bin'})

which fails with:

...
Cloning https://github.com/ikostrikov/pytorch-a2c-ppo-acktr-gail to /private/tmp/pip-req-build-0tvyih8o
  Running command git clone --filter=blob:none --quiet https://github.com/ikostrikov/pytorch-a2c-ppo-acktr-gail /private/tmp/pip-req-build-0tvyih8o
  fatal: failed to expand user dir in: '~/.gitignore'

the ~/.gitignore' in the error is referring to a line in /usr/local/git/etc/gitconfig.

And then I run it again, but remove /usr/local/bin from the PATH in the command:

python
>>> import subprocess
>>> subprocess.run(['/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin/python', '-m', 'pip', 'install', '-r', '/Users/andyk/Development/agentos/example_agents/papag/requirements.txt'], env={'SYSTEMROOT': '', 'VIRTUAL_ENV': '/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226', 'PATH': '/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin:/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/Users/andyk/.gem/ruby/2.7.0/bin:/opt/homebrew/Caskroom/miniforge/base/envs/agentos_dev2/bin:/opt/homebrew/Caskroom/miniforge/base/condabin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin:/Applications/Postgres.app/Contents/Versions/latest/bin'})

...and the pip install runs fine.

Running the pip command directly in my bash shell works fine. That is, the following works fine (note that PATH is the same):

> echo $PATH
/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/Users/andyk/.gem/ruby/2.7.0/bin:/opt/homebrew/Caskroom/miniforge/base/envs/agentos_dev2/bin:/opt/homebrew/Caskroom/miniforge/base/condabin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin:/Applications/Postgres.app/Contents/Versions/latest/bin
> agentos run agent --entry-point learn --arg-set-file a2c_pong_args.yaml

I believe the problem is that we are currently passing shell=False to subprocess.run(), and so git is behaving differently for that subprocess than it does for me inside my shell (i.e., git seems to be using that conf file /usr/local/git/etc/gitconfig but not for my shell.

I verified this by running the same python command inside the virtualenv as before, except i set shell=True and it succeeds:

python
>>> import subprocess
>>> subprocess.run(['/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin/python', '-m', 'pip', 'install', '-r', '/Users/andyk/Development/agentos/example_agents/papag/requirements.txt'], env={'SYSTEMROOT': '', 'VIRTUAL_ENV': '/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226', 'PATH': '/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin:/Users/andyk/.agentos/cache/requirements_cache/python3.9/6ca95402f3200676689cbf632041895fb1b022e6c898c9bfc4439e4fcb0bb226/bin:/opt/homebrew/bin:/opt/homebrew/sbin:/Users/andyk/.gem/ruby/2.7.0/bin:/opt/homebrew/Caskroom/miniforge/base/envs/agentos_dev2/bin:/opt/homebrew/Caskroom/miniforge/base/condabin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin:/Applications/Postgres.app/Contents/Versions/latest/bin'}, shell=True)
@andyk andyk changed the title pip install -r command fails on my M1 because PATH is passed to env= arg VirtualEnv's pip install -r requirements.txt fails becuase git behaves differently when shell=False passed to subprocess.run()` Apr 18, 2022
@andyk andyk changed the title VirtualEnv's pip install -r requirements.txt fails becuase git behaves differently when shell=False passed to subprocess.run()` VirtualEnv's pip install -r requirements.txt fails because git behaves differently when shell=False passed to subprocess.run()` Apr 18, 2022
@andyk andyk changed the title VirtualEnv's pip install -r requirements.txt fails because git behaves differently when shell=False passed to subprocess.run()` VirtualEnv's pip install -r requirements.txt fails because git behaves differently when shell=False passed to subprocess.run() Apr 18, 2022
@andyk
Copy link
Contributor Author

andyk commented Apr 18, 2022

@nickjalbert any reason we should not pass shell=True to subprocess.run() in order to make sure our command behaves more like the user's shell?

@andyk andyk moved this to Todo in Sprint 0.2.1 Apr 18, 2022
@andyk
Copy link
Contributor Author

andyk commented Apr 19, 2022

Adding shell=True seems to break a few tests, perhaps most importantly it breaks the test_vm_repl test inside of the test_venv_management.py file. I think the way the test fails might be more clear to understand if #364 was fixed.

@nickjalbert
Copy link
Contributor

nickjalbert commented Apr 19, 2022

Hmm, I don't know why shell=True breaks tests. Off the top of my head, I don't know why it should behave differently (and, consequently, it seems like it should be fine to run with shell=True). Something to investigate!

@andyk andyk removed this from Sprint 0.2.1 Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants