[homebrew] 1.9.0 regression test failure #928

Closed
opened 2024-11-04 04:06:33 +00:00 by chenrui · 19 comments

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
  ==> /home/linuxbrew/.linuxbrew/Cellar/borgmatic/1.9.0/bin/borgmatic --config /tmp/borgmatic-test-20241103-14940-o1dpfg/config.yml
  local: Error running actions for repository
  [Errno 13] Permission denied: '/run/user/1000'
  /tmp/borgmatic-test-20241103-14940-o1dpfg/config.yml: An error occurred
  
  summary:
  /tmp/borgmatic-test-20241103-14940-o1dpfg/config.yml: An error occurred
  local: Error running actions for repository
  [Errno 13] Permission denied: '/run/user/1000'

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

### 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: <details> <summary>error build log</summary> ``` ==> /home/linuxbrew/.linuxbrew/Cellar/borgmatic/1.9.0/bin/borgmatic --config /tmp/borgmatic-test-20241103-14940-o1dpfg/config.yml local: Error running actions for repository [Errno 13] Permission denied: '/run/user/1000' /tmp/borgmatic-test-20241103-14940-o1dpfg/config.yml: An error occurred summary: /tmp/borgmatic-test-20241103-14940-o1dpfg/config.yml: An error occurred local: Error running actions for repository [Errno 13] Permission denied: '/run/user/1000' ``` </details> 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_
Owner

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 the XDG_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 the DARWIN_USER_TEMP_DIR configuration string? Or perhaps Python's tempfile.gettempdir() would be an easy way to accomplish this.

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 the `XDG_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 the `DARWIN_USER_TEMP_DIR` configuration string? Or perhaps Python's ` tempfile.gettempdir()` would be an easy way to accomplish this.
witten added the
bug
label 2024-11-04 05:11:52 +00:00
Author

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.

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.
Owner

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 or TEMP environment variable to set its user runtime directory. TMPDIR should be set on macOS and TEMP 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!

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` or `TEMP` environment variable to set its user runtime directory. `TMPDIR` should be set on macOS and `TEMP` 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!
Author

Sounds good, let me check out the patch, thanks!

Sounds good, let me check out the patch, thanks!
Author

the patch works for me, however I also notice the directory got changed from #{testpath}/.borgmatic to #{testpath}/./borgmatic

       info --json #{repo_path}
       init --encryption repokey --debug #{repo_path}
       --version
-      create #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/.borgmatic #{config_path}
+      create #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/./borgmatic #{config_path}
       prune --keep-daily 7 --glob-archives {hostname}-* #{repo_path}
       compact #{repo_path}
       info --json #{repo_path}
       check --glob-archives {hostname}-* #{repo_path}
       --version
-      create --json #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/.borgmatic #{config_path}
+      create --json #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/./borgmatic #{config_path}
       prune --keep-daily 7 --glob-archives {hostname}-* #{repo_path}
       compact #{repo_path}
       info --json #{repo_path}

is that desired?

the patch works for me, however I also notice the directory got changed from `#{testpath}/.borgmatic` to `#{testpath}/./borgmatic` ```diff info --json #{repo_path} init --encryption repokey --debug #{repo_path} --version - create #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/.borgmatic #{config_path} + create #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/./borgmatic #{config_path} prune --keep-daily 7 --glob-archives {hostname}-* #{repo_path} compact #{repo_path} info --json #{repo_path} check --glob-archives {hostname}-* #{repo_path} --version - create --json #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/.borgmatic #{config_path} + create --json #{repo_path}::{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f} /etc /home #{testpath}/./borgmatic #{config_path} prune --keep-daily 7 --glob-archives {hostname}-* #{repo_path} compact #{repo_path} info --json #{repo_path} ``` is that desired?
Owner

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 is borgmatic. So for instance /run/user/0/./borgmatic/somefile would get stored into the archive as just borgmatic/somefile.

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 is `borgmatic`. So for instance `/run/user/0/./borgmatic/somefile` would get stored into the archive as just `borgmatic/somefile`.
Author

got it, thanks for confirming the behavior 🍻

got it, thanks for confirming the behavior 🍻
Owner

Fixed in borgmatic 1.9.1, just released!

Fixed in borgmatic 1.9.1, just released!
Author

thanks @witten!

thanks @witten!

Hi, just wanted to verify before i create a new issue, i'm still getting this error on macOS:

ssh://user@host.xx/./backups/mybackup: Removing SQLite data source dumps
    backupserver: Error running actions for repository
    [Errno 30] Read-only file system: '/run'

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

Hi, just wanted to verify before i create a new issue, i'm still getting this error on macOS: ``` ssh://user@host.xx/./backups/mybackup: Removing SQLite data source dumps backupserver: Error running actions for repository [Errno 30] Read-only file system: '/run' ``` 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).

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..

Ah, I see borgmatic uses ~/.local/state/borgmatic for checks now, maybe that would be a better place for these files too..
Owner

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:

  • Use XDG_RUNTIME_DIR if set.
  • Otherwise, use TMPDIR if set.
  • Otherwise, use TEMP if set.
  • Otherwise, use a hard-coded /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.

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: - Use `XDG_RUNTIME_DIR` if set. - Otherwise, use `TMPDIR` if set. - Otherwise, use `TEMP` if set. - Otherwise, use a hard-coded `/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.
Owner

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.

Also, according to [various articles online](https://iboysoft.com/wiki/tmp-folder-mac.html), `$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.

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

So are any of those environment variables set on your machine when running borgmatic?

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.

@witten > So are any of those environment variables set on your machine when running borgmatic? No, they are not set. Fyi, i'm running Borgmatic as a `root` user (a Global Daemon, [see here](https://www.launchd.info/). 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?

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:

export TMPDIR=$(getconf DARWIN_USER_CACHE_DIR)
@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: ```bash export TMPDIR=$(getconf DARWIN_USER_CACHE_DIR) ```
Owner

@witten should we create/open a new issue, to make people aware?

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.

> @witten should we create/open a new issue, to make people aware? 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.
Sign in to join this conversation.
No Milestone
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#928
No description provided.