Fix failing test on OpenBSD #7652
Conversation
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.
```
=================================== FAILURES ===================================
_____________________________ test_get_runtime_dir _____________________________
path = '/run/user/55/borg', mode = 511, pretty_deadly = True
def ensure_dir(path, mode=stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO, pretty_deadly=True):
"""
Ensures that the dir exists with the right permissions.
1) Make sure the directory exists in a race-free operation
2) If mode is not None and the directory has been created, give the right
permissions to the leaf directory. The current umask value is masked out first.
3) If pretty_deadly is True, catch exceptions, reraise them with a pretty
message.
Returns if the directory has been created and has the right permissions,
An exception otherwise. If a deadly exception happened it is reraised.
"""
try:
> os.makedirs(path, mode=mode, exist_ok=True)
build/lib.openbsd-7.3-amd64-cpython-310/borg/helpers/fs.py:37:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
...
...
```
I think `/run/user` is something that is created by systemd, which is not used
on OpenBSD. Despite of
https://github.com/borgbackup/borg/blob/master/src/borg/platformflags.py#L4,
proposal is to test if we are on OpenBSD, and act accordingly. Reason for this
shortcut is that adding `is_openbsd` to `platformflags.py` results in more
code, and I'm not sure this makes sense for a unit test.
|
#7638 could be relevant. |
|
@bket thanks for the PR, but the fix needed is in |
|
@ThomasWaldmann, forgot to mention: initially I was seeing more failing tests. After setting |
|
I guess we need to wait for someone fixing the problem in After we know how it should behave, we can fix our test for its behaviour. |
| assert get_runtime_dir() == os.path.join("/run/user", str(os.getuid()), "borg") | ||
| if not sys.platform.startswith("openbsd"): | ||
| assert get_runtime_dir() == os.path.join("/run/user", str(os.getuid()), "borg") |
There was a problem hiding this comment.
i've seen some fix in platformdirs - can you re-test / adapt the test code with a current version?
There was a problem hiding this comment.
retested with platformdirs-3.8.0, and the issue is still there. Pull request still works for me, and is used locally
There was a problem hiding this comment.
well, the linux test would still fail, but it now returns a valid directory for openbsd i guess, so the test could be adapted to check that?
There was a problem hiding this comment.
Included with platformdirs-3.8.0 is tox-dev/platformdirs@a02fb04, which causes platformdirs.user_runtime_dir to default to /var/run/user/{getuid()} on several BSD's, including OpenBSD. /var/run/user is not part of OpenBS's base as shown by hier(7), and is therefore not a valid directory. Additionally, it seems that XDG's runtime directory is normally set by pam_systemd, which is not supported by OpenBSD.
I'm guessing that the recent change to platformdirs has not been tested on OpenBSD. Way forward is to discuss this with upstream of platformdirs.
I think it makes sense to address this issue in OpenBSD's port of borgbackup (until it has been resolved in platformdirs), and close this PR.
@ThomasWaldmann, What do you think?
There was a problem hiding this comment.
Guess getting platformdirs fixed would benefit a lot of software using that.
And if it is fixed there, no change in borg is needed.
A borgbackup-2.0.0b6 test fails on OpenBSD with the message below.
I think
/run/useris something that is created by systemd, which is not used on OpenBSD. Despite ofhttps://github.com/borgbackup/borg/blob/master/src/borg/platformflags.py#L4, proposal is to test if we are on OpenBSD, and act accordingly. Reason for this shortcut is that adding
is_openbsdtoplatformflags.pyresults in more code, and I'm not sure this makes sense for a unit test.