[CT-3508] Consolidate `system.rmdir` and `system.rmtree`
- Dominant language
- Rust
- Stars
- 13.8k
- Forks
- 2.6k
- Avg merge
- 21h 31m
- Merged PRs (30d)
- 56
Description
### Housekeeping
- [X] I am a maintainer of dbt-core
### Short description
[`system.rmdir`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L223-L235) and [`system.rmtree`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L565-L570) look basically the same.
One uses [`_windows_rmdir_readonly`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L203-L209) and the other uses [`chmod_and_retry`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L508-L519) (which also look very similar).
Can we consolidate all of these for simplicity?
### Potential solution
It might be as simple as just replacing the implementation of one with a call to the other.
And then removing whichever of `_windows_rmdir_readonly` or `chmod_and_retry` is unused after that.
### Acceptance criteria
- There is no longer duplicated code within [`system.rmdir`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L223-L235) and [`system.rmtree`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L565-L570)
- There is no longer duplicated code within [`_windows_rmdir_readonly`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L203-L209) and [`chmod_and_retry`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L508-L519)
### Impact to Other Teams
No impact anticipated.
### Will backports be required?
No backports required.
### Context
Discovered while investigating https://github.com/dbt-labs/dbt-core/issues/9304.
#### `system.rmdir`
[`system.rmdir` ](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L223) was introduced in [#487](https://github.com/dbt-labs/dbt-core/pull/487/files#diff-20374548a72c09013ab95950949b7cd3d59f6d1a0faa406f231c9a6d2603322c) (2017-07-14) It was updated within [#542](https://github.com/dbt-labs/dbt-core/pull/542/files#diff-20374548a72c09013ab95950949b7cd3d59f6d1a0faa406f231c9a6d2603322c) (2018-02-27) to handle Windows files that git marks as read-only.
It is in use in these locations:
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/deps/git.py#L121
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/deps/local.py#L62
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/deps/tarball.py#L88
#### `system.rmtree`
[`system.rmtree`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L565) was added in [#1110](https://github.com/dbt-labs/dbt-core/pull/1110/files#diff-20374548a72c09013ab95950949b7cd3d59f6d1a0faa406f231c9a6d2603322c) (2018-11-14). It also handles read-only Windows files.
It is directly in use in these locations:
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/deps/base.py#L43
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/task/deps.py#L229
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L559
And it is _indirectly_ used (via [`system.move`](https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/clients/system.py#L526)) here:
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/deps/git.py#L123
- https://github.com/dbt-labs/dbt-core/blob/c2bc2f009bbeeb46b3c69d082ab4d485597898af/core/dbt/deps/tarball.py#L89
Contributor guide
Assessment
This issue has not been assessed yet.