-
-
Notifications
You must be signed in to change notification settings - Fork 954
-
Hi!
Problem: recently I stumbled upon a problem when cloning when git used LD_LIBRARY_PATH that I thought I deleted.
It turned out, that here environment is updated with provided env. I thought that env param will be "copied" as a working environment, rather than used to update some pre-existing one.
My problem was fixed by setting the variable value to an empty string.
In docs of execute method, env param is described as follows:
env – A dictionary of environment variables to be passed to subprocess.Popen.
This sounds like the env will be used "as is".
Same name of parameter in clone_from led me to believe that the behaviour will be similar there.
Question:
Is this behavior intentional?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
When following the link provided behind docs now, I see env – Optional dictionary containing the desired environment variables. in the clone_from(...) method. Inarguably this description is not very precise, and to me it doesn't sound like it would use the given environment dictionary as is.
However, I can definitely see why one would be surprised though that env in clone_from behaves differently from env in execute for instance. The parameter name could have been better.
Changing env in clone_from to something more specific would be a breaking change, as is changing the name in any other method.
Is there anything else that you can imagine doing to improve this?
Replies: 4 comments
-
When following the link provided behind docs now, I see env – Optional dictionary containing the desired environment variables. in the clone_from(...) method. Inarguably this description is not very precise, and to me it doesn't sound like it would use the given environment dictionary as is.
However, I can definitely see why one would be surprised though that env in clone_from behaves differently from env in execute for instance. The parameter name could have been better.
Changing env in clone_from to something more specific would be a breaking change, as is changing the name in any other method.
Is there anything else that you can imagine doing to improve this?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
-
Probably anything that could be done do API would be breaking change in this case. How about a little update on the
envdescription for clone_from? Something that would explain that provided variables will be added, or will replace existing ones? Like
env- optional dictionary containing environmental variables that will be added to the execution environment of git commands (or will replace existing ones).? -
Out of curiosity: what were the reasons to implement
git.Git.update_envrionmentthis way? It would seem easier to makeself._environment=copy(env).
Beta Was this translation helpful? Give feedback.
All reactions
-
Thanks for your reply!
About 1), I think you would be most qualified to submit a PR with a documentation update, you could write the docs you would have liked to see, and the contribution would be much appreciated :).
About 2): Since the capability was contributed, I could only guess. And doing just that, I guess the author saw this usage as primary use case, maybe even one that is most common for them.
Personally I think it would have been best if a more descriptive name would have been found in place of env, which indeed sounds like it is used as definitive environment.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 2
-
I am closing this one as a kind of fix has been merged.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1