utils: fix_env: dont delete LD_LIBRARY_PATH from env #2541
Conversation
dvc/utils/__init__.py
Outdated
| env.pop(lp_key, None) | ||
| # We used to delete "LD_LIBRARY_PATH" key. We cannot do that | ||
| # because GitPython uses env to "update" his own internal state. | ||
| # When there is no key in env, this value is not updated and |
There was a problem hiding this comment.
I guess this part of the explanation is about why setting None doesn't work, no?
There was a problem hiding this comment.
I wouldn't say that.
This explains why we prefer to set the empty string for "LD_LIBRARY_PATH" over deleting it from the env dictionary.
There was a problem hiding this comment.
Also please add a link to that gitpython issue.
There was a problem hiding this comment.
And it is worth noting that this is only relevant for clone_from really, where env behaves weirdly, but other things work fine. Hm, it also makes me think whether we should do needed modifications for clone_from specifically and not here. I don't see how this approach would backstub us in the future yet, but it sure makes me a bit nervous 😃
There was a problem hiding this comment.
Discussed with @pared that it would make sense to indeed move this logic to 'dvc/scm/git/init.py:Git.clone' to be safe. It could look something like
env = fix_env()
if is_binary():
env["LD_LIBRARY_PATH"] = ""
Co-Authored-By: Ruslan Kuprieiev <[email protected]>
Have you followed the guidelines in our
Contributing document?
Does your PR affect documented changes or does it add new functionality
that should be documented? If yes, have you created a PR for
dvc.org documenting it or at
least opened an issue for it? If so, please add a link to it.
Fixes #2471