[question] How to interpret `env` param in `Repo.clone_from`

[pared]

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?

[Byron]

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?

[pared]

1. Probably anything that could be done do API would be breaking change in this case. How about a little update on the env description 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).?

2. Out of curiosity: what were the reasons to implement git.Git.update_envrionment this way? It would seem easier to make self._environment=copy(env).

[Byron]

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.

[pared]

I am closing this one as a kind of fix has been merged.