René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"HIDE_WINDOWS_*_ERRORS environment variables are interpreted strangely","articleBody":"### Summary\r\n\r\nAs noted in #1697:\r\n\r\n\u003e The `HIDE_WINDOWS_KNOWN_ERRORS` and `HIDE_WINDOWS_FREEZE_ERRORS` variables are intended to be affected by the same-named environment variables, but this does not work properly. They are `True` by default, but the only way to use environment variables to set them to a false value is *to define the environment variable with an empty value* (since everything else, as a string, is truthy). In addition to being very unlikely to have been the intended behavior, this is also hard to do on Windows when using typical shells, because in both `cmd.exe` and PowerShell setting an environment variable as empty actually removes the variable altogether.\r\n\r\n### Details\r\n\r\nThe attributes have type `bool` outside native Windows systems (where they are `False`) and in native Windows systems where the environment variables don't exist (they are `True`):\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/8107cbfeb1d56b2587d347c7f3d509ebef079950/git/util.py#L122-L126\r\n\r\nOtherwise they are the `str` values of the same-named environment variables. They are always used as conditions, and as such, the only ways to set them `False` on a native Windows system--besides editing that code, which in practice seems to be what is done--is to write to the attributes with Python code or to set the environment variables to the empty string.\r\n\r\nIn `bash` on Windows, such as Git Bash, this is easy to do, though not at all obvious that it should be done and also very unintuitive. In shells meant for Windows, `cmd.exe` and PowerShell, setting an environment variable to the empty string causes it to be automatically *unset*, and extra effort is required to actually set the empty string.\r\n\r\n### The tricky part\r\n\r\n`HIDE_WINDOWS_KNOWN_ERRORS` is defined in `git.util` [**and listed in `__all__`**](https://github.com/gitpython-developers/GitPython/blob/8107cbfeb1d56b2587d347c7f3d509ebef079950/git/util.py#L114). So it is a breaking change to remove or rename it. Because code that uses the GitPython library is explicitly allowed to assume the attribute will continue to exist, it's worth considering the subjective question of what, if anything, users can reasonably assume *about* it.\r\n\r\nAlthough comments shown above explains its original purpose, its semantics are not really documented anywhere, and nothing makes any statements about how it is set. Furthermore, if people are setting nonempty values that intuitively feel like they express falsehood--such as `0`--then they probably expect that this is setting it to `False` or some falsy value. Therefore, I think it's reasonable to regard the current behavior as a bug and fix it, even though actually removing or even officially deprecating the attribute should wait until later because it currently affects the behavior of `git.util.rmtree`, through [the callback](https://github.com/gitpython-developers/GitPython/blob/8107cbfeb1d56b2587d347c7f3d509ebef079950/git/util.py#L185-L196) it passes to `shutil.rmtree`.\r\n\r\nIn contrast, `HIDE_WINDOWS_FREEZE_ERRORS` is not listed in `__all__` and could be renamed, removed, or arbitrarily changed at any time. But as long as `HIDE_WINDOWS_KNOWN_ERRORS` remains in `git.util`, I don't think the presence `HIDE_WINDOWS_FREEZE_ERRORS` should be seen as any special problem.\r\n\r\nAlthough I don't advocate formally deprecating the `HIDE_WINDOWS_KNOWN_ERRORS` *attribute* just yet, the environment variable is another story. In addition to being parsed differently, the presence of either environment variable could cause a warning to be printed when `git.util` is loaded (which happens when the `git` module is loaded).\r\n\r\n### Context\r\n\r\nThis relates to the deeper issue #790. Fixing this narrower issue won't solve the problem that raising `SkipTest` as a result of failing to delete files can produce unpredictable effects in production--because it signals a condition that would typically be signaled by `PermissionError` or some other exception derived from `OSError`, and would bypass exception handling meant for that--as well as masking bugs in applications or other libraries that use GitPython by causing their automated tests to be skipped. But solving that will carry its own challenge. The main problem is not how to provide the functionality to the test suite, but instead that there may be code using GitPython that has come to rely on exceptions from deleting files being wrapped in `SkipTest`.\r\n\r\nAlthough I definitely think #790 *should* be solved, and there are various approaches to mitigating disruption from doing so, I think it is worth fixing this narrower issue of how the environment variables are interpreted now. I've included such a fix in #1700 (which also would fix #1699).","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2023-10-09T11:11:17.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":0},"url":"https://github.com/1698/GitPython/issues/1698"}