Title: Fix bugs affecting exception wrapping in rmtree callback by EliahKagan · Pull Request #1700 · gitpython-developers/GitPython · GitHub
Open Graph Title: Fix bugs affecting exception wrapping in rmtree callback by EliahKagan · Pull Request #1700 · gitpython-developers/GitPython
X Title: Fix bugs affecting exception wrapping in rmtree callback by EliahKagan · Pull Request #1700 · gitpython-developers/GitPython
Description: Fixes #1698 Fixes #1699 May close #1571 This takes the approaches I suggested in #1698 and #1699 for those two issues. It also adds tests for both the already-working behavior and the new behavior. For the tests of new behavior, I have manually verified they failed in the expected way, and now pass, on native Windows. (Other results can be seen on CI.) For interpreting the HIDE_WINDOWS_KNOWN_ERRORS and HIDE_WINDOWS_FREEZE_ERRORS environment variables: I have kept True as the default (though we may be able to change that in the near future, even before fully addressing #790). 0, false, and no, as well as the empty string, are treated as False. This is case-insensitive and ignoring leading and trailing whitespace. While other values are currently treated as True, values other than possibly padded and possibly case-varying 1, yes, or true show a warning about being unrecognized. This warning is in addition to, not instead of, the warning that is shown from the environment variable being present at all. The attributes from those variables are always bool, now, too. They are never strings. When limiting the exceptions the rmtree callback catches and reraises as unittest.SkipTest to PermissionError, I made a couple other changes as well: I eliminated the duplicated logic, which technically is an additional behavioral change, because in some cases a PermissionError was wrapped in a SkipTest that was itself wrapped in another SkipTest. But I don't think this will cause any problems because even if someone relied on that, the only thing they would likely do would have been to follow the chain to the first non-SkipTest. As noted in #1571, passing a callback as the onerror parameter to shutil.rmtree is deprecated starting in Python 3.12, and the documentation indicates that it is planned for removal in 3.14. So I have used the new onexc parameter on 3.12 (and later). Because this is not available on 3.11 and earlier, it continues to use onerror in those versions. This was easy to implement, because in this case the same callback function naturally works for both, as it does not inspect the error information it receives. #1571 may or may not be resolved by this, which I think is to a large extent subjective. That issue does two things. It documents a problem from 3.12.0.alpha7, which I believe was fixed, as I commented there. It also notes the deprecated status of shutil.rmtree's onerror argument and advocates that onexc be used instead. This was originally with the idea that using onerror caused or contributed to the observed test failures, but even though that was probably not the case, the issue might have been taken to stand for that as well. I also fixed the type hints on the callback, renamed the callback itself as handler so it is no longer named after one of those two keyword arguments but not the other (which I think could cause confusing), and renamed its parameters to more closely match the documentation. As it is a local function that is not returned, this does not raise any compatibility issues. In a similar vein, I've included a number of low-stakes refactorings, mostly just to imports, in modules I was already modifying heavily, mainly in test_util. This also adds a missing xfail and updates commented references to the callback-based skipping logic that already existed. Most tests that get skipped in this way, of which there are about eight, do not have comments like this, and I have not added them. The tests that did are those that had previously been marked with @skipIf decorators. Finally, I've updated the git.util.rmtree docstring, which will affect generated documentation. This relates to, but only somewhat mitigates and definitely does not solve, #790.
Open Graph Description: Fixes #1698 Fixes #1699 May close #1571 This takes the approaches I suggested in #1698 and #1699 for those two issues. It also adds tests for both the already-working behavior and the new behavior....
X Description: Fixes #1698 Fixes #1699 May close #1571 This takes the approaches I suggested in #1698 and #1699 for those two issues. It also adds tests for both the already-working behavior and the new behavior....
Opengraph URL: https://github.com/gitpython-developers/GitPython/pull/1700
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/checks(.:format) |
| route-controller | pull_requests |
| route-action | checks |
| fetch-nonce | v2:5ed7f995-bc12-ee66-1ed6-c8d2c5a10128 |
| current-catalog-service-hash | 87dc3bc62d9b466312751bfd5f889726f4f1337bdff4e8be7da7c93d6c00a25a |
| request-id | 8792:218736:A2437A:D95EE8:69686C30 |
| html-safe-nonce | afa32725a00d254efc2f74a3480c803feeb2eb1524469613f2489b0f8f0e49cc |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiI4NzkyOjIxODczNjpBMjQzN0E6RDk1RUU4OjY5Njg2QzMwIiwidmlzaXRvcl9pZCI6IjgyNTI5NzM2MjE3NTU1Mzg0ODAiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | e62da51422c61a46db3da8f5cff4f7e81abb9c581da98dadd6e21ce6e2f4e7a8 |
| hovercard-subject-tag | pull_request:1547659758 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,checks,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/gitpython-developers/GitPython/pull/1700/checks |
| twitter:image | https://avatars.githubusercontent.com/u/1771172?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/1771172?s=400&v=4 |
| og:image:alt | Fixes #1698 Fixes #1699 May close #1571 This takes the approaches I suggested in #1698 and #1699 for those two issues. It also adds tests for both the already-working behavior and the new behavior.... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 50f46dc2d6192249fd8ebf20e76c800f4f2596d4a5f3ab63dd63a754df154f54 |
| turbo-cache-control | no-preview |
| go-import | github.com/gitpython-developers/GitPython git https://github.com/gitpython-developers/GitPython.git |
| octolytics-dimension-user_id | 503709 |
| octolytics-dimension-user_login | gitpython-developers |
| octolytics-dimension-repository_id | 1126087 |
| octolytics-dimension-repository_nwo | gitpython-developers/GitPython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 1126087 |
| octolytics-dimension-repository_network_root_nwo | gitpython-developers/GitPython |
| turbo-body-classes | logged-out env-production page-responsive full-width full-width-p-0 |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | fef287f17234b4529a4b112a3d47fe8551e32ddd |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width