-
-
Notifications
You must be signed in to change notification settings - Fork 754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use platformdirs #7300
use platformdirs #7300
Conversation
8500282
to
5659b9d
Compare
5659b9d
to
0fdb35e
Compare
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #7300 +/- ##
==========================================
- Coverage 83.62% 83.50% -0.13%
==========================================
Files 67 67
Lines 11668 11679 +11
Branches 2122 1982 -140
==========================================
- Hits 9757 9752 -5
- Misses 1342 1355 +13
- Partials 569 572 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
0fdb35e
to
6c7efbe
Compare
src/borg/testsuite/helpers.py
Outdated
assert get_config_dir(legacy=False) == get_config_dir(legacy=True) | ||
# TODO fails on macOS: assert '/Users/tw/Library/Preferences/borg' == '/Users/tw/.config/borg' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on macOS, it looks like platformdirs uses a different default: not ~/.config/<appname>
.
this might affect a lot of macOS borg users (defaults, nothing special here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on Linux, this works ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on cygwin, this works ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
win32 fail: assert 'C:/Users/runneradmin/AppData/Local/borg/borg' == 'D:\a\_temp\msys64\home\runneradmin/.config/borg'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it is using the HOME
env var, which inside MSYS2 is set to the home directory of the MSYS2 installation.
>>> os.path.expanduser("~")
'C:/Users/Rayyan'
>>> os.environ.get("HOME")
'C:\\msys64\\home\\Rayyan'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i excluded that "legacy compat test" on win32, no need to have it there.
BTW, that .../Local/borg/borg
path is correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RayyanAnsari seen this?
On native win32, guess we can ignore legacy vs. non-legacy compatibility as nobody uses borg < 2 there. |
it's better readable.
…e into a temp dir XDG_*_HOME is not honoured on macOS and on Windows if we use platformdirs.
8bf19e2
to
bde0f11
Compare
there is no old borg < 2.0 there anyway.
Test run looks good. @RayyanAnsari maybe give the whole PR a review, please. Are the tests complete, do they make sense? Something missing? |
Needs requirement: I'll fix the requirement... |
we don't want to suddenly/unexpectedly break stuff for borg users just because platformdirs does a breaking release. at platformdirs 2.0.0 macOS config dir changed. at platformdirs 3.0.0 macOS config dir changed again. at platformdirs 4.0.0 (future) - who knows?
TODO: #7332 |
25909ee
to
516c070
Compare
@RayyanAnsari merged! thanks for working on this! |
Make sure the result of get_cache_dir matches pre and post borgbackup#7300 where desired
Make sure the result of get_cache_dir matches pre and post borgbackup#7300 where desired
fix config dir compatibility issue, fixes #7445 - add tests - make sure the result of get_cache_dir matches pre and post #7300 where desired - harmonize implementation of config_dir_compat and cache_dir_compat tests Co-authored-by: nain <[email protected]>
fixes #7281