From a07cf9e699b0c1ca6d8039af3e92318d84c367d1 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 26 Nov 2024 16:20:06 +0000 Subject: [PATCH] Revert temporary reversion of 1.9.4.dev0. revert Temporary revert of 1.9.4.dev0 changeset so we can re-build 1.9.3 (which never actually got built). revert Fix library error when running within a PyInstaller bundle (#926). --- NEWS | 3 ++ borgmatic/hooks/command.py | 34 ++++++++++++++----- pyproject.toml | 2 +- tests/unit/hooks/test_command.py | 58 ++++++++++++++++++++++++++------ 4 files changed, 76 insertions(+), 21 deletions(-) diff --git a/NEWS b/NEWS index 6ac9be77..212ce5e5 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.9.4.dev0 + * #926: Fix library error when running within a PyInstaller bundle. + 1.9.3 * #261 (beta): Add a ZFS hook for snapshotting and backing up ZFS datasets. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/snapshot-your-filesystems/ diff --git a/borgmatic/hooks/command.py b/borgmatic/hooks/command.py index 8eafb31e..d03fc1da 100644 --- a/borgmatic/hooks/command.py +++ b/borgmatic/hooks/command.py @@ -2,8 +2,9 @@ import logging import os import re import shlex +import sys -from borgmatic import execute +import borgmatic.execute logger = logging.getLogger(__name__) @@ -27,6 +28,20 @@ def interpolate_context(config_filename, hook_description, command, context): return command +def make_environment(current_environment, sys_module=sys): + ''' + Given the existing system environment as a map from environment variable name to value, return + (in the same form) any extra environment variables that should be used when running command + hooks. + ''' + # Detect whether we're running within a PyInstaller bundle. If so, set or clear LD_LIBRARY_PATH + # based on the value of LD_LIBRARY_PATH_ORIG. This prevents library version information errors. + if getattr(sys_module, 'frozen', False) and hasattr(sys_module, '_MEIPASS'): + return {'LD_LIBRARY_PATH': current_environment.get('LD_LIBRARY_PATH_ORIG', '')} + + return {} + + def execute_hook(commands, umask, config_filename, description, dry_run, **context): ''' Given a list of hook commands to execute, a umask to execute with (or None), a config filename, @@ -65,14 +80,15 @@ def execute_hook(commands, umask, config_filename, description, dry_run, **conte try: for command in commands: - if not dry_run: - execute.execute_command( - [command], - output_log_level=( - logging.ERROR if description == 'on-error' else logging.WARNING - ), - shell=True, - ) + if dry_run: + continue + + borgmatic.execute.execute_command( + [command], + output_log_level=(logging.ERROR if description == 'on-error' else logging.WARNING), + shell=True, + extra_environment=make_environment(os.environ), + ) finally: if original_umask: os.umask(original_umask) diff --git a/pyproject.toml b/pyproject.toml index d081573f..ce0322cc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "1.9.3" +version = "1.9.4.dev0" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ] diff --git a/tests/unit/hooks/test_command.py b/tests/unit/hooks/test_command.py index 2ba7ac13..4a3ca045 100644 --- a/tests/unit/hooks/test_command.py +++ b/tests/unit/hooks/test_command.py @@ -35,12 +35,32 @@ def test_interpolate_context_escapes_interpolated_variables(): ) +def test_make_environment_without_pyinstaller_does_not_touch_environment(): + assert module.make_environment({}, sys_module=flexmock()) == {} + + +def test_make_environment_with_pyinstaller_clears_LD_LIBRARY_PATH(): + assert module.make_environment({}, sys_module=flexmock(frozen=True, _MEIPASS='yup')) == { + 'LD_LIBRARY_PATH': '' + } + + +def test_make_environment_with_pyinstaller_and_LD_LIBRARY_PATH_ORIG_copies_it_into_LD_LIBRARY_PATH(): + assert module.make_environment( + {'LD_LIBRARY_PATH_ORIG': '/lib/lib/lib'}, sys_module=flexmock(frozen=True, _MEIPASS='yup') + ) == {'LD_LIBRARY_PATH': '/lib/lib/lib'} + + def test_execute_hook_invokes_each_command(): flexmock(module).should_receive('interpolate_context').replace_with( lambda config_file, hook_description, command, context: command ) - flexmock(module.execute).should_receive('execute_command').with_args( - [':'], output_log_level=logging.WARNING, shell=True + flexmock(module).should_receive('make_environment').and_return({}) + flexmock(module.borgmatic.execute).should_receive('execute_command').with_args( + [':'], + output_log_level=logging.WARNING, + shell=True, + extra_environment={}, ).once() module.execute_hook([':'], None, 'config.yaml', 'pre-backup', dry_run=False) @@ -50,11 +70,18 @@ def test_execute_hook_with_multiple_commands_invokes_each_command(): flexmock(module).should_receive('interpolate_context').replace_with( lambda config_file, hook_description, command, context: command ) - flexmock(module.execute).should_receive('execute_command').with_args( - [':'], output_log_level=logging.WARNING, shell=True + flexmock(module).should_receive('make_environment').and_return({}) + flexmock(module.borgmatic.execute).should_receive('execute_command').with_args( + [':'], + output_log_level=logging.WARNING, + shell=True, + extra_environment={}, ).once() - flexmock(module.execute).should_receive('execute_command').with_args( - ['true'], output_log_level=logging.WARNING, shell=True + flexmock(module.borgmatic.execute).should_receive('execute_command').with_args( + ['true'], + output_log_level=logging.WARNING, + shell=True, + extra_environment={}, ).once() module.execute_hook([':', 'true'], None, 'config.yaml', 'pre-backup', dry_run=False) @@ -66,8 +93,12 @@ def test_execute_hook_with_umask_sets_that_umask(): ) flexmock(module.os).should_receive('umask').with_args(0o77).and_return(0o22).once() flexmock(module.os).should_receive('umask').with_args(0o22).once() - flexmock(module.execute).should_receive('execute_command').with_args( - [':'], output_log_level=logging.WARNING, shell=True + flexmock(module).should_receive('make_environment').and_return({}) + flexmock(module.borgmatic.execute).should_receive('execute_command').with_args( + [':'], + output_log_level=logging.WARNING, + shell=True, + extra_environment={}, ) module.execute_hook([':'], 77, 'config.yaml', 'pre-backup', dry_run=False) @@ -77,7 +108,8 @@ def test_execute_hook_with_dry_run_skips_commands(): flexmock(module).should_receive('interpolate_context').replace_with( lambda config_file, hook_description, command, context: command ) - flexmock(module.execute).should_receive('execute_command').never() + flexmock(module).should_receive('make_environment').and_return({}) + flexmock(module.borgmatic.execute).should_receive('execute_command').never() module.execute_hook([':', 'true'], None, 'config.yaml', 'pre-backup', dry_run=True) @@ -90,8 +122,12 @@ def test_execute_hook_on_error_logs_as_error(): flexmock(module).should_receive('interpolate_context').replace_with( lambda config_file, hook_description, command, context: command ) - flexmock(module.execute).should_receive('execute_command').with_args( - [':'], output_log_level=logging.ERROR, shell=True + flexmock(module).should_receive('make_environment').and_return({}) + flexmock(module.borgmatic.execute).should_receive('execute_command').with_args( + [':'], + output_log_level=logging.ERROR, + shell=True, + extra_environment={}, ).once() module.execute_hook([':'], None, 'config.yaml', 'on-error', dry_run=False)