Skip to content

Commit

Permalink
Fix environment population for build package workaround
Browse files Browse the repository at this point in the history
The fix in eb03bc1 calls `subprocess.run` with an empty
dict as environment to escape snap environment when installing
packages on the host system.  This does unfortunately not work in
all circumstances.

Fixes #644
  • Loading branch information
fnordahl committed Oct 9, 2022
1 parent eb03bc1 commit efb0fba
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 13 deletions.
3 changes: 2 additions & 1 deletion charmtools/build/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,8 @@ def workaround_charmcraft_maybe_ensure_build_packages(self):
or os.environ.get('CHARMCRAFT_PART_NAME', None))):
log.warning('Probably running as root in charmcraft, proactively '
'installing the `git` and `virtualenv` packages.')
subprocess.run(('apt', '-y', 'install', 'git', 'virtualenv'), check=True, env={})
subprocess.run(('apt', '-y', 'install', 'git', 'virtualenv'),
check=True, env=utils.host_env())

def generate(self):
layers = self.fetch()
Expand Down
14 changes: 2 additions & 12 deletions charmtools/build/tactics.py
Original file line number Diff line number Diff line change
Expand Up @@ -1064,24 +1064,14 @@ def __str__(self):
return "Building wheelhouse in {}".format(directory)

def _get_env(self):
"""Get environment appropriate for executing external commands.
"""Get environment for executing commands outside snap context.
:returns: Dictionary with environment variables
:rtype: Dict[str,str]
"""
if self.use_python_from_snap:
return os.environ.copy()

env = os.environ.copy()
for key in ('PREFIX', 'PYTHONHOME', 'PYTHONPATH',
'GIT_TEMPLATE_DIR', 'GIT_EXEC_PATH'):
if key in env:
del(env[key])
env['PATH'] = ':'.join([
element
for element in env['PATH'].split(':')
if not element.startswith('/snap/charm/')])
return env
return utils.host_env()

def combine(self, existing):
"" # suppress inherited doc
Expand Down
18 changes: 18 additions & 0 deletions charmtools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -633,3 +633,21 @@ def validate_display_name(entity, linter):
linter.err('display-name: not in valid format. '
'Only letters, numbers, dashes, and hyphens are permitted.')
return


def host_env():
"""Get environment appropriate for executing commands outside snap context.
:returns: Dictionary with environment variables
:rtype: Dict[str,str]
"""
env = os.environ.copy()
for key in ('PREFIX', 'PYTHONHOME', 'PYTHONPATH',
'GIT_TEMPLATE_DIR', 'GIT_EXEC_PATH'):
if key in env:
del(env[key])
env['PATH'] = ':'.join([
element
for element in env['PATH'].split(':')
if not element.startswith('/snap/charm/')])
return env
17 changes: 17 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import print_function

import unittest
from unittest import TestCase
from charmtools import utils
from six import StringIO
Expand Down Expand Up @@ -43,3 +44,19 @@ def react(db):
self.assertIn("Beta", output)
self.assertIn("@when('db.ready'", output)
self.assertIn("bar", output)

@unittest.mock.patch("os.environ")
def test_host_env(self, mock_environ):
mock_environ.copy.return_value = {
'PREFIX': 'fake-prefix',
'PYTHONHOME': 'fake-pythonhome',
'PYTHONPATH': 'fake-pythonpath',
'GIT_TEMPLATE_DIR': 'fake-git-template-dir',
'GIT_EXEC_PATH': 'fake-git-exec-path',
'SOME_OTHER_KEY': 'fake-some-other-key',
'PATH': '/snap/charm/current/bin:/usr/bin:'
'/snap/charm/current/usr/bin:/bin',
}
self.assertDictEqual(
{'SOME_OTHER_KEY': 'fake-some-other-key', 'PATH': '/usr/bin:/bin'},
utils.host_env())

0 comments on commit efb0fba

Please sign in to comment.