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.

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

bundled borg is preferred over system borg (macOS) #2100

Closed
2 tasks done
ThomasWaldmann opened this issue Oct 18, 2024 · 40 comments
Closed
2 tasks done

bundled borg is preferred over system borg (macOS) #2100

ThomasWaldmann opened this issue Oct 18, 2024 · 40 comments
Labels
status:needs details Needs more details before it can be solved.

Comments

@ThomasWaldmann
Copy link
Collaborator

Description

After stumbling over #2099 I also noticed that vorta seems to prefer the borg that comes bundled with it, even if the user has installed another borg on their system.

In my case, I have:

tw@MacBook-Pro ~ % which borg       
/opt/homebrew/bin/borg
tw@MacBook-Pro ~ % borg -V
borg 1.4.0

And that borg is the Apple Silicon binary, which would be better than the currently bundled Intel binary inside vorta.

Reproduction

  • I tried to reproduce the issue.
  • I was able to reproduce the issue.

OS

macOS 15

Version of Vorta

0.10.0 beta1

What did you install Vorta with?

Binary

Version of Borg

1.4.0

Logs

No response

@ThomasWaldmann ThomasWaldmann changed the title bundled borg is preferred over system borg bundled borg is preferred over system borg (macOS) Oct 18, 2024
@m3nu
Copy link
Contributor

m3nu commented Oct 18, 2024

Not seeing that. It's using the Homebrew version for me.

Screenshot 2024-10-18 at 19 18 47

@m3nu m3nu added the status:needs details Needs more details before it can be solved. label Oct 18, 2024
@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Oct 18, 2024

I just restarted vorta, still the same: it wants to use the bundled borg.

2024-10-18 21:05:20,068 - vorta.i18n - DEBUG - Loading translation succeeded for ['en-DE', 'en-Latn-DE', 'de-DE', 'de-Latn-DE', 'de'].
2024-10-18 21:05:20,072 - vorta.scheduler - WARNING - Failed to connect to DBUS interface to detect sleep/resume events
2024-10-18 21:05:20,228 - root - DEBUG - Not a private SSH key file: authorized_keys
2024-10-18 21:05:20,229 - root - DEBUG - Not a private SSH key file: ykpub
2024-10-18 21:05:20,243 - vorta.views.source_tab - DEBUG - Added item number 0 from 1
2024-10-18 21:05:20,612 - root - INFO - Using DarwinNetworkStatus NetworkStatusMonitor implementation.
2024-10-18 21:05:20,688 - vorta.borg.jobs_manager - DEBUG - Add job for site default
2024-10-18 21:05:20,688 - vorta.borg.jobs_manager - DEBUG - Start job on site: default
2024-10-18 21:05:20,689 - vorta.borg.borg_job - INFO - Running command /Applications/Vorta.app/Contents/Resources/borg-dir/borg.exe --version
2024-10-18 21:05:21,204 - vorta.borg.jobs_manager - DEBUG - Finish job for site: default
2024-10-18 21:05:21,204 - vorta.borg.jobs_manager - DEBUG - No more jobs for site: default
2024-10-18 21:05:25,290 - vorta.scheduler - DEBUG - Refreshing all scheduler timers
2024-10-18 21:05:25,291 - vorta.scheduler - INFO - Setting timer for profile 1
2024-10-18 21:05:25,291 - vorta.scheduler - DEBUG - Scheduling next run for 2024-10-19 00:03:32.628913

@ThomasWaldmann
Copy link
Collaborator Author

Just checked out vorta master branch and installed it to a venv:

That one finds the homebrew borg:

2024-10-18 21:48:17,192 - vorta.borg.borg_job - INFO - Running command /opt/homebrew/bin/borg --version

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Oct 18, 2024

    @classmethod
    def prepare_bin(cls):
        """Find packaged borg binary. Prefer globally installed."""

        borg_in_path = shutil.which('borg')

        if borg_in_path:
            return borg_in_path
        elif sys.platform == 'darwin':
            # macOS: Look in pyinstaller bundle
            from Foundation import NSBundle

            mainBundle = NSBundle.mainBundle()

            bundled_borg = os.path.join(mainBundle.bundlePath(), 'Contents', 'Resources', 'borg-dir', 'borg.exe')
            if os.path.isfile(bundled_borg):
                return bundled_borg
        return None

Guess we need some additional logging in the else-branch for os.environ["PATH"] because the shutil.which obviously could not find my homebrew borg (and it is found if I use which in a terminal).

@m3nu
Copy link
Contributor

m3nu commented Oct 18, 2024

vorta.spec has the path that's being searched. This is an Apple app setting. Currently it's

/usr/local/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/homebrew/bin

@ThomasWaldmann
Copy link
Collaborator Author

That might be, but something is broken there.

My borg is in /opt/homebrew/bin and does not get found by shutil.which.

@maximede
Copy link

Same issue on my side.
If that helps, vorta was installed with brew too

@m3nu
Copy link
Contributor

m3nu commented Oct 22, 2024

I think in the case of Thomas, it was Borg that was installed from homebrew.

Is there a difference when using Vorta downloaded from Github? (Homebrew also downloads from there, so it should be the same file). In which path is your Borg binary? Also in /opt/homebrew/bin?

@maximede
Copy link

Same for me, both borg and vorta are installed from homebrew

# which borg
/opt/homebrew/bin/borg

@m3nu
Copy link
Contributor

m3nu commented Oct 22, 2024

Still can't reproduce this with fresh settings.

Screenshot 2024-10-22 at 13 47 33

@ThomasWaldmann
Copy link
Collaborator Author

BTW, IIRC finding the homebrew borg from vorta has worked for me at some time in the past. I remember installing the Apple Silicon version of borg from homebrew, so that Vorta does not use the bundled Intel version of borg. At that time, I verified that the Apple Silicon version gets invoked by vorta. But that was quite a while ago.

Recently, upgraded macOS and also vorta (see top post).

@ThomasWaldmann
Copy link
Collaborator Author

BTW, I reproduced the issue (see top post) with vorta 0.9.1 installed via brew.

It also wants to use the bundled borg and not the one from homebrew.

@ThomasWaldmann
Copy link
Collaborator Author

Just found out that the borgbackup-fuse from homebrew does not work correctly, it misses packaging. Guess that could be the reason why vorta falls back to the bundled borg.

borgbackup/homebrew-tap#36

@ThomasWaldmann
Copy link
Collaborator Author

Hmm, I just uninstalled borgbackup-fuse, brew installed borgbackup, upgraded and restarted vorta and still get this:

Screenshot 2024-11-10 at 16 08 18

@m3nu
Copy link
Contributor

m3nu commented Nov 10, 2024

Here it always gives me the Homebrew version, if it exists. Very mysterious.

@greigdp
Copy link
Contributor

greigdp commented Nov 11, 2024

Curious - I noticed an Intel-based borg binary was being used, rather than the available homebrew-provided ARM-native binary. I am using the non-fuse (regular) borg homebrew package, and the mac-packaged Vorta app.

Looking at the code that detects a borg binary, when I run this from an (admittedly separate) Python install, I see the correct Homebrew-based borg binary is found.

➜  ~ python3.13
Python 3.13.0 (main, Oct  7 2024, 05:02:14) [Clang 16.0.0 (clang-1600.0.26.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import shutil
>>> shutil.which('borg')
'/opt/homebrew/bin/borg'

So the fallback path that uses the built-in binary shouldn't be invoked.

Despite this, Vorta still uses the built-in borg binary.

I will try to set up a development environment and run Vorta from a virtualenv to see if there is something relating to the packaging for Mac OS that is affecting the behaviour of shutil. Are those seeing the homebrew binary used running Vorta from a source tree, or the regular shipped release .app file?

@ThomasWaldmann
Copy link
Collaborator Author

I really think we need a debug log line printing the PATH there. Something is not as expected there and that would be a first step confirming that.

@ThomasWaldmann
Copy link
Collaborator Author

@greigdp See #2100 (comment) - I could not debug the issue because in a source based install it worked as expected (and I could not build the binary).

@greigdp
Copy link
Contributor

greigdp commented Dec 1, 2024

@greigdp See #2100 (comment) - I could not debug the issue because in a source based install it worked as expected (and I could not build the binary).

Interesting - I have tried a few things there to reproduce the issue, and have noticed one potential quirk.

If you launch the shipped signed binary of Vorta directly by double-clicking the .app bundle, the embedded borg binary is shown in About.

If you launch the Vorta binary manually from the shell (/Applications/Vorta.app/Contents/MacOS/vorta-darwin), it will find and use the homebrew-provided binary.

Modifying the binary to add logging to the prepare_bin() function, I added (around the shutil.which statement):

logger.info('$PATH is set to: %s', os.popen("echo $PATH").read())
borg_in_path = shutil.which('borg')
logger.info('borg_in_path was equal to: %s', borg_in_path)

This showed that, from the perspective of the code running in my build, $PATH was set to:

/usr/bin:/bin:/usr/sbin:/sbin

From the pyinstaller spec, it looks like the PATH for the binary is meant to be set to a suitable value (https://github.com/borgbase/vorta/blob/master/package/vorta.spec#L81) - just this doesn't seem to be the PATH that is in the runtime environment experienced by the app.

If I run the vorta-darwin binary directly (i.e. my one built here, with my additional debugging lines), the PATH is much longer and includes, as you'd expect, /opt/homebrew/bin:/opt/homebrew/sbin at the start, as it's being inherited from the shell that spawned the command, and the homebrew binary is found.

Therefore I think the root issue here is, as you expected, the PATH variable, and how it is (or is not) meant to be set when running the app from a double-click on the .App bundle.

@ThomasWaldmann
Copy link
Collaborator Author

@greigdp Can you use os.environ["PATH"]?

Should show the current PATH as in the vorta process and not create another process for the "echo $PATH".

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Dec 1, 2024

Can confirm:

  • when vorta is started by "Vorta.app" being a "login item", it get the wrong, bundled borg-dir/borg.exe (intel)
  • when I invoke /Applications/Vorta.app/Contents/MacOS/vorta-darwin from a terminal, it says it will use the correct homebrew borg (apple silicon)

@ThomasWaldmann
Copy link
Collaborator Author

ThomasWaldmann commented Dec 1, 2024

This makes it use the homebrew borg for me:

tw@MacBook-Pro ~ % defaults write com.apple.dt.Xcode UseSanitizedBuildSystemEnvironment -bool NO

Source: https://stackoverflow.com/a/26586170

Update: only works until logout or reboot, seems like that setting is not persisted.

@greigdp
Copy link
Contributor

greigdp commented Dec 1, 2024

@greigdp Can you use os.environ["PATH"]?

Should show the current PATH as in the vorta process and not create another process for the "echo $PATH".

I got the same path result as before using os.environ["PATH"] - /usr/bin:/bin:/usr/sbin:/sbin.

@ThomasWaldmann
Copy link
Collaborator Author

Any macOS developer / admin reading? It can't be that hard to modify the PATH so such stuff just works, can it?

@greigdp
Copy link
Contributor

greigdp commented Dec 8, 2024

Any macOS developer / admin reading? It can't be that hard to modify the PATH so such stuff just works, can it?

I've managed to create a workaround that at least proves this can be fixed. It might not be ideal from a cross-platform perspective, but it looks like something in Mac OS isn't behaving as you'd expect - this part of the pyinstaller spec is meant to set the Environment that the app runs in.

For whatever reason (maybe a change in a newer Mac OS release?), this doesn't appear to work any more, given the wrong path being used.

I changed this (blank) line of code to contain the following line of code, as a proof of concept. This should manually set the PATH environment variable at the time it is required, to the value that's meant to be set by the pyinstaller spec file.

os.environ["PATH"]="/usr/local/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/opt/homebrew/bin"

After doing this, and rebuilding the app, I see a path of /opt/homebrew/bin/borg shown in the about page of the GUI, which matches up with my manual logging (that I still had in place).

@m3nu
Copy link
Contributor

m3nu commented Dec 8, 2024

Nice one! Looks like a pretty clear solution and I'm surprised it works and the spec file stopped working.

Wanna do a mini PR or should I?

@greigdp
Copy link
Contributor

greigdp commented Dec 8, 2024

Nice one! Looks like a pretty clear solution and I'm surprised it works and the spec file stopped working.

Wanna do a mini PR or should I?

I guess we should "platform-gate" this so it only makes this PATH change on Mac platform? Don't mind if you do the PR or I do.

@m3nu
Copy link
Contributor

m3nu commented Dec 9, 2024

It will be easier to credit you, if you add the PR.

Yes, I'ld only make the change on macOS. We mostly do if sys.platform == 'darwin': in other places. Would also add a short comment on why it's needed.

Once the PR is up, I'll do test builds for intel and arm, so everyone can confirm the fix.

@greigdp
Copy link
Contributor

greigdp commented Dec 9, 2024

It will be easier to credit you, if you add the PR.

Yes, I'ld only make the change on macOS. We mostly do if sys.platform == 'darwin': in other places. Would also add a short comment on why it's needed.

Once the PR is up, I'll do test builds for intel and arm, so everyone can confirm the fix.

Makes sense, will make a PR shortly.

@m3nu
Copy link
Contributor

m3nu commented Dec 10, 2024

Thanks for the PR, @greigdp . Here a test build for Arm using the PR branch:

https://files.pf7.net/vorta/Vorta-greigdp-fix-2100-arm.dmg

One thing I was wondering: Should we append to PATH instead of replacing it. Normal Mac users are unlikely to care about it, but it may be surprising when e.g. using a Borg from a virtual env. I hardly ever do this myself, so I don't mind. Maybe we just log that PATH was replaced?

@greigdp
Copy link
Contributor

greigdp commented Dec 10, 2024

Thanks for the PR, @greigdp . Here a test build for Arm using the PR branch:

https://files.pf7.net/vorta/Vorta-greigdp-fix-2100-arm.dmg

One thing I was wondering: Should we append to PATH instead of replacing it. Normal Mac users are unlikely to care about it, but it may be surprising when e.g. using a Borg from a virtual env. I hardly ever do this myself, so I don't mind. Maybe we just log that PATH was replaced?

I have pushed an updated PR, which takes the existing PATH and appends to it. This feels neater.

@m3nu
Copy link
Contributor

m3nu commented Dec 10, 2024

Works for me.

And the test build picks the right Borg version in your situation?

@greigdp
Copy link
Contributor

greigdp commented Dec 10, 2024

Works for me.

And the test build picks the right Borg version in your situation?

Yes, confirmed by checking ps aux that the running Borg process is an ARM-based Python binary, pulling in Borg from Homebrew:

/opt/homebrew/Cellar/[email protected]/3.12.8/Frameworks/Python.framework/Versions/3.12/Resources/Python.app/Contents/MacOS/Python /opt/homebrew/bin/borg info --log-json --json [....]

@m3nu
Copy link
Contributor

m3nu commented Dec 11, 2024

Great! Also solves if for you, @ThomasWaldmann ? Test build: https://files.pf7.net/vorta/Vorta-greigdp-fix-2100-arm.dmg

The binary found should also show in About.

@ThomasWaldmann
Copy link
Collaborator Author

@m3nu works for me!

both vorta and borg show as kind "Apple" in activity monitor, path shown in vorta points to the homebrew borg.

@m3nu
Copy link
Contributor

m3nu commented Dec 11, 2024

Good to hear. I think we should also append /usr/local/bin, which is Homebrew for Intel? Then it should match the functionality of the spec file: https://github.com/borgbase/vorta/blob/master/package/vorta.spec#L81C39-L81C53

@m3nu
Copy link
Contributor

m3nu commented Dec 11, 2024

Added Intel Homebrew and changed it slightly so we don't fail if there is no PATH at all. Will merge tonight, if no objections.

https://github.com/borgbase/vorta/pull/2166/files

@ThomasWaldmann
Copy link
Collaborator Author

Fixed by #2166.

@ThomasWaldmann
Copy link
Collaborator Author

Fixed in 0.10.3. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:needs details Needs more details before it can be solved.
Projects
None yet
Development

No branches or pull requests

5 participants