From 60cbd0f588982262d2b3c9655db5cbdf163aba12 Mon Sep 17 00:00:00 2001 From: Buck Evan Date: Wed, 8 Mar 2017 10:17:01 -0800 Subject: [PATCH 1/3] we need to cd to the final destination even if source fails --- aactivator.py | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/aactivator.py b/aactivator.py index cb58c01..80fe2d8 100755 --- a/aactivator.py +++ b/aactivator.py @@ -246,23 +246,38 @@ def security_check(path): ) +def commands(commands, always=None): + """ + run a set of commands, stopping on the first error + The `always` is run whether there was an error or not + """ + cmd = ' &&\n'.join(commands) + if always: + cmd += '\n' + always + return cmd + + def command_for_path(cmd, path, pwd): if path == pwd: return cmd else: - return ' &&\n'.join(( - 'OLDPWD_bak="$OLDPWD"', - 'cd ' + quote(path), - cmd, - 'cd "$OLDPWD_bak"', - 'cd ' + quote(pwd), - 'unset OLDPWD_bak', - )) + return commands( + ( + 'OLDPWD_bak="$OLDPWD"', + 'cd ' + quote(path), + cmd, + ), + always=commands(( + 'cd "$OLDPWD_bak"', + 'cd ' + quote(pwd), + 'unset OLDPWD_bak', + )) + ) def aactivate(path, pwd): return command_for_path( - ' &&\n'.join(( + commands(( 'aactivator security-check ' + ACTIVATE, 'source ./' + ACTIVATE, 'export %s=%s' % (ENVIRONMENT_VARIABLE, quote(path)), @@ -278,15 +293,18 @@ def deaactivate(path, pwd): if os.path.exists(deactivate_path): return command_for_path( - ' &&\n'.join(( - 'aactivator security-check ' + DEACTIVATE, - 'source ./' + DEACTIVATE, - )) + '\n' + unset, + commands( + ( + 'aactivator security-check ' + DEACTIVATE, + 'source ./' + DEACTIVATE, + ), + always=unset, + ), path, pwd, ) else: - return ' &&\n'.join(( + return commands(( unset, error_command('Cannot deactivate. File missing: {0}'.format(deactivate_path)) )) @@ -315,7 +333,7 @@ def get_output(environ, pwd='.', get_input=sys.stdin.readline, arg0='/path/to/aa result.append(deaactivate(activated_env, pwd)) if activate_path: result.append(aactivate(activate_path, pwd)) - return ' &&\n'.join(result) + return commands(result) def aactivator(args, env): From dfb7adf6b14b5eab8a2734d226e8d1f95fc9ae70 Mon Sep 17 00:00:00 2001 From: Buck Evan Date: Sat, 13 May 2017 11:30:19 -0700 Subject: [PATCH 2/3] touchup --- aactivator.py | 4 +++- tests/unit_test.py | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/aactivator.py b/aactivator.py index 80fe2d8..9bb1a21 100755 --- a/aactivator.py +++ b/aactivator.py @@ -248,7 +248,9 @@ def security_check(path): def commands(commands, always=None): """ - run a set of commands, stopping on the first error + Generate a bash script to run a set of commands, stopping + on the first error. + The `always` is run whether there was an error or not """ cmd = ' &&\n'.join(commands) diff --git a/tests/unit_test.py b/tests/unit_test.py index 5ed13bc..a1f0921 100644 --- a/tests/unit_test.py +++ b/tests/unit_test.py @@ -116,7 +116,7 @@ def test_get_output_sourced_not_in_directory(tmpdir, venv_path, active_env): cd {venv_path} && aactivator security-check .deactivate.sh && source ./.deactivate.sh -unset AACTIVATOR_ACTIVE && +unset AACTIVATOR_ACTIVE cd "$OLDPWD_bak" && cd {tmpdir} && unset OLDPWD_bak'''.format(venv_path=str(venv_path), tmpdir=str(tmpdir)) @@ -151,7 +151,7 @@ def test_get_output_sourced_deeper_venv(tmpdir, venv_path, active_env): cd {venv_path} && aactivator security-check .deactivate.sh && source ./.deactivate.sh -unset AACTIVATOR_ACTIVE && +unset AACTIVATOR_ACTIVE cd "$OLDPWD_bak" && cd {venv_path}/deeper && unset OLDPWD_bak && @@ -192,7 +192,7 @@ def test_get_output_change_venv(tmpdir, venv_path, active_env): cd {venv_path} && aactivator security-check .deactivate.sh && source ./.deactivate.sh -unset AACTIVATOR_ACTIVE && +unset AACTIVATOR_ACTIVE cd "$OLDPWD_bak" && cd {venv_path}2 && unset OLDPWD_bak && @@ -306,3 +306,13 @@ def test_no_is_remembered_until_cd_out(venv_path, tmpdir, no_config, yes_config) # cd to a parent directory directory (and back) should assert yes_config().find_allowed('/') is None assert yes_config().find_allowed(str(child_dir)) == str(venv_path) + + +def test_commands(): + assert aactivator.commands(('true',)) == 'true' + assert aactivator.commands(('foo', 'bar')) == '''\ +foo && +bar''' + assert aactivator.commands(('try',), 'finally') == '''\ +try +finally''' From feaa14feb6eb0f226a7c0c855af1e601c83f5f9d Mon Sep 17 00:00:00 2001 From: Buck Evan Date: Sat, 13 May 2017 11:31:39 -0700 Subject: [PATCH 3/3] integration test for the problem --- tests/integration_test.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/integration_test.py b/tests/integration_test.py index e438464..e44ea22 100644 --- a/tests/integration_test.py +++ b/tests/integration_test.py @@ -397,3 +397,35 @@ def test_cd_dash(venv_path, tmpdir, shell): venv2=str(venv2), ) run_test(shell, test, tmpdir) + + +def test_broken_activate(venv_path, tmpdir, shell): + """When a activate script doesn't work, aactivator keeps trying it, + but you can still cd within the project. + """ + make_venv_in_tempdir(tmpdir) + venv_path.join('.activate.sh').write("echo '(activation failure)'; false") + + test = '''\ +TEST> eval "$(aactivator init)" +TEST> cd {venv_path}/child-dir +aactivator will source .activate.sh and .deactivate.sh at {venv_path}. +Acceptable? (y)es (n)o (N)ever: INPUT> y +aactivator will remember this: ~/.cache/aactivator/allowed +(activation failure) +TEST> pwd +{venv_path}/child-dir +(activation failure) +TEST> cd .. +(activation failure) +TEST> pwd +{venv_path} +(activation failure) +TEST> cd .. +TEST> echo ok +ok +''' + test = test.format( + venv_path=str(venv_path), + ) + run_test(shell, test, tmpdir)