[homebrew] 1.9.0 regression test failure #928
Loading…
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What I'm trying to do and why
👋 trying to build the latest release, but run into some regression test failure. The error log is as below:
error build log
full build log, https://github.com/Homebrew/homebrew-core/actions/runs/11654818120/job/32450978048?pr=196551
relates to https://github.com/Homebrew/homebrew-core/pull/196551
Steps to reproduce
No response
Actual behavior
No response
Expected behavior
No response
Other notes / implementation ideas
No response
borgmatic version
1.9.0
borgmatic installation method
No response
Borg version
No response
Python version
No response
Database version (if applicable)
No response
Operating system and version
No response
I can't easily tell from the build log... Is this test failure in one of borgmatic's tests or in some Homebrew-specific test suite? If the latter, can I get look at the test in question?
What I can tell from the error output is that the code under test is trying to write to borgmatic's new runtime data directory (which has replaced
~/.borgmatic
). The new path defaults to/run/user/$ID
and can be overridden with theXDG_RUNTIME_DIR
environment variable. So that might be one way to work around this for the time being.However it sounds like
XDG_RUNTIME_DIR
isn't set by default on macOS, and therefore borgmatic probably needs to use something mac-specific to determine the runtime directory location. Is there anything you recommend for that? Maybe theDARWIN_USER_TEMP_DIR
configuration string? Or perhaps Python'stempfile.gettempdir()
would be an easy way to accomplish this.yeah, it is a regression test, I did try to compare the test config, but did not see much differences btw 1.9.0 and 1.8.14, which is weird.
The difference is in the 1.9.0 borgmatic code that writes to the new runtime directory, which isn't apparently available on macOS. Anyway, I believe I've fixed in main by making borgmatic also read the
TMPDIR
orTEMP
environment variable to set its user runtime directory.TMPDIR
should be set on macOS andTEMP
is often set on other systems like Cygwin.I don't have a mac, so I haven't actually confirmed this works even if it should work. But please feel free to try borgmatic main and report back. Or you can wait until 1.9.1 if you prefer.
Thanks!
Sounds good, let me check out the patch, thanks!
the patch works for me, however I also notice the directory got changed from
#{testpath}/.borgmatic
to#{testpath}/./borgmatic
is that desired?
Yes! With Borg 1.4+ it means that the
testpath
doesn't become part of the path stored in the Borg archive, and instead the path just gets stored with the portion after the/./
, which in this case isborgmatic
. So for instance/run/user/0/./borgmatic/somefile
would get stored into the archive as justborgmatic/somefile
.got it, thanks for confirming the behavior 🍻
Fixed in borgmatic 1.9.1, just released!
thanks @witten!
Hi, just wanted to verify before i create a new issue, i'm still getting this error on macOS:
Is this error related to this issue? I tried both HomeBrew and via
pipx install borgmatic
, both give the same error for me.borgmatic 1.9.1
borg 1.4.0
macOS 15.1 (24B83)
Macbook M1
A static (predictable) file or directory name in a world-writable TMPDIR or TEMP isn't safe, some other user on the system could create it and prevent borgmatic from working. (That's presumably why the previous code used the home directory). If borgmatic wants to use /tmp for this, the directory name should have random characters appended and check that it could create the directory (mktemp -d style) to make it hard to race.
As far as I know, /run/user/$uid is a pam_systemd thing and unlikely to work on any other OS, or non-systemd Linux.
XDG_RUNTIME_DIR should be ok. /run/user/$UID should be ok if it already exists. If not then something in ~ is a better fallback (for 1.9.0 in the openbsd port I patched it to use ~/.cache, possibly not strictly correct but seemed like the best option out of what's available).
Ah, I see borgmatic uses ~/.local/state/borgmatic for checks now, maybe that would be a better place for these files too..
Not sure why you're still getting the error. Here's what the 1.9.1 code currently does to construct the runtime directory path:
XDG_RUNTIME_DIR
if set.TMPDIR
if set.TEMP
if set./run/user/
+ the current user's Unix uid.So are any of those environment variables set on your machine when running borgmatic?
I see what you're saying about risks with using a temporary directory. The whole point of moving the runtime directory from
~/.borgmatic
was to adhere better to actual OS standards rather than borgmatic just doing its own thing. So on Linux, there is an "official" place for runtime files (/run/user/$UID
). If the best option though on MacOS is a temp dir, then I think I'd rather use that (and to your point, use a random filename within it) than come up with my own convention in the home directory.And as for reusing
~/.local/state/borgmatic
, I think that directory (as per the XDG standard) is really intended for persistent user state rather than temporary runtime files.Also, according to various articles online,
$TMPDIR
on macOS is user-specific and already has a "random" filename. So there should be any cross-user temp file races, intentional or otherwise.btw I'm not the person who was having problems with TMPDIR (I'm running borgmatic on OpenBSD rather than MacOS). /tmp on other OS is not user specific (and often can't be, because it may be used to share files between users) and it seems plausible that some users may have it set in their environment (though I don't think it's usually done by default). The change in 1.9.1 means that if anyone does have one of those variables set, you're now preferring /tmp/borgmatic (with the easy conflict) over /run/user/$uid/borgmatic (in a user-specific dir that is already owned by the user).
I think for borgmatic it would be better to use, in order, XDG_RUNTIME_DIR, /run/user/$uid but only if it already exists, then some other kind of fallback (but with a random suffix if it ends up in a shared or possibly-shared dir).
btw there is an existing module on pypi that has some OS-specific knowledge of these dirs, "platformdirs", and user_runtime_dir is probably what you want from there, but sadly that has the same problem (it uses "/var/run/user/$uid" if it exists - which it doesn't by default at least on OpenBSD or NetBSD; unsure about FreeBSD - and "/tmp/runtime-$(id -u)/$appname/$version" as a fallback; there's nothing to create the directory safely or check that it's owned by the correct uid).
For the OpenBSD packages I think I'll continue patching to use ~/.cache/borgmatic for now.
@witten
No, they are not set.
Fyi, i'm running Borgmatic as a
root
user (a Global Daemon, see here.The env vars are only set for me when logged in as a normal user.
I also checked with
sudo -i
, and here environment variables are also not set.I don't know what the correct way is, to determine the TMP dir on macOS, so not sure if this is useful, but i did find:
getconf DARWIN_USER_TEMP_DIR
which when executed as
root
user outputs:/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/
So that seems correct...
PS: there also seems to be
DARWIN_USER_CACHE_DIR
.Looks like platformdirs uses ~/Library/Caches/TemporaryItems/$appname/$version for this purpose on MacOS. Not sure if that would work here or not?
@witten should we create/open a new issue, to make people aware?
PS. For anyone stumbling upon this issue on macOS running as
root
user, simply add this to your Borgmatic script to work around the error for now:I'll use #934 (recently opened) to track these issues.
Please continue to follow along on that ticket, and I'll post my thoughts there—hopefully taking into account the use cases raised here.
I will point out though that until this is fixed, you can set the borgmatic
user_runtime_directory
configuration option, so no tweaking of environment variables or code patching should be necessary.