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

[andyk]

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.).

https://github.com/agentos-project/agentos/blob/4030b81ef1db4b86614e3d8e861eb8d95845edce/pcs/virtual_env.py#L214-L219

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]

@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]

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]

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!