Skip to content
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.

Sign up for GitHub

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

Fix failing test on OpenBSD #7652

Closed
wants to merge 1 commit into from
Closed

Fix failing test on OpenBSD #7652

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/borg/testsuite/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,8 @@ def test_get_runtime_dir(monkeypatch):
else:
monkeypatch.delenv("XDG_RUNTIME_DIR", raising=False)
monkeypatch.delenv("BORG_RUNTIME_DIR", raising=False)
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")
Comment on lines -803 to +804
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've seen some fix in platformdirs - can you re-test / adapt the test code with a current version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retested with platformdirs-3.8.0, and the issue is still there. Pull request still works for me, and is used locally

Copy link
Member

@ThomasWaldmann ThomasWaldmann Jul 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess getting platformdirs fixed would benefit a lot of software using that.
And if it is fixed there, no change in borg is needed.

monkeypatch.setenv("XDG_RUNTIME_DIR", "/var/tmp/.cache")
assert get_runtime_dir() == os.path.join("/var/tmp/.cache", "borg")
monkeypatch.setenv("BORG_RUNTIME_DIR", "/var/tmp")
Expand Down