Do not mount deps folder when running in virtual env (#14993)

* Do not mount deps folder when inside virtual env

* Add tests

* Fix package test

homeassistant/bootstrap.py

@@ -1,5 +1,4 @@
 """Provide methods to bootstrap a Home Assistant instance."""
-import asyncio
 import logging
 import logging.handlers
 import os
@@ -17,7 +16,7 @@ from homeassistant.components import persistent_notification
 from homeassistant.const import EVENT_HOMEASSISTANT_CLOSE
 from homeassistant.setup import async_setup_component
 from homeassistant.util.logging import AsyncHandler
-from homeassistant.util.package import async_get_user_site, get_user_site
+from homeassistant.util.package import async_get_user_site, is_virtual_env
 from homeassistant.util.yaml import clear_secret_cache
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.signal import async_register_signal_handling
@@ -53,8 +52,9 @@ def from_config_dict(config: Dict[str, Any],
         if config_dir is not None:
             config_dir = os.path.abspath(config_dir)
             hass.config.config_dir = config_dir
-            hass.loop.run_until_complete(
-                async_mount_local_lib_path(config_dir, hass.loop))
+            if not is_virtual_env():
+                hass.loop.run_until_complete(
+                    async_mount_local_lib_path(config_dir))
 
     # run task
     hass = hass.loop.run_until_complete(
@@ -197,7 +197,9 @@ async def async_from_config_file(config_path: str,
     # Set config dir to directory holding config file
     config_dir = os.path.abspath(os.path.dirname(config_path))
     hass.config.config_dir = config_dir
-    await async_mount_local_lib_path(config_dir, hass.loop)
+
+    if not is_virtual_env():
+        await async_mount_local_lib_path(config_dir)
 
     async_enable_logging(hass, verbose, log_rotate_days, log_file,
                          log_no_color)
@@ -211,9 +213,8 @@ async def async_from_config_file(config_path: str,
     finally:
         clear_secret_cache()
 
-    hass = await async_from_config_dict(
+    return await async_from_config_dict(
         config_dict, hass, enable_log=False, skip_pip=skip_pip)
-    return hass
 
 
 @core.callback
@@ -308,23 +309,13 @@ def async_enable_logging(hass: core.HomeAssistant,
             "Unable to setup error log %s (access denied)", err_log_path)
 
 
-def mount_local_lib_path(config_dir: str) -> str:
-    """Add local library to Python Path."""
-    deps_dir = os.path.join(config_dir, 'deps')
-    lib_dir = get_user_site(deps_dir)
-    if lib_dir not in sys.path:
-        sys.path.insert(0, lib_dir)
-    return deps_dir
-
-
-async def async_mount_local_lib_path(config_dir: str,
-                                     loop: asyncio.AbstractEventLoop) -> str:
+async def async_mount_local_lib_path(config_dir: str) -> str:
     """Add local library to Python Path.
 
     This function is a coroutine.
     """
     deps_dir = os.path.join(config_dir, 'deps')
-    lib_dir = await async_get_user_site(deps_dir, loop=loop)
+    lib_dir = await async_get_user_site(deps_dir)
     if lib_dir not in sys.path:
         sys.path.insert(0, lib_dir)
     return deps_dir

homeassistant/scripts/__init__.py

@@ -1,5 +1,6 @@
 """Home Assistant command line scripts."""
 import argparse
+import asyncio
 import importlib
 import logging
 import os
@@ -7,10 +8,10 @@ import sys
 
 from typing import List
 
-from homeassistant.bootstrap import mount_local_lib_path
+from homeassistant.bootstrap import async_mount_local_lib_path
 from homeassistant.config import get_default_config_dir
 from homeassistant import requirements
-from homeassistant.util.package import install_package
+from homeassistant.util.package import install_package, is_virtual_env
 
 
 def run(args: List) -> int:
@@ -38,7 +39,11 @@ def run(args: List) -> int:
     script = importlib.import_module('homeassistant.scripts.' + args[0])
 
     config_dir = extract_config_dir()
-    mount_local_lib_path(config_dir)
+
+    if not is_virtual_env():
+        asyncio.get_event_loop().run_until_complete(
+            async_mount_local_lib_path(config_dir))
+
     pip_kwargs = requirements.pip_kwargs(config_dir)
 
     logging.basicConfig(stream=sys.stdout, level=logging.INFO)

homeassistant/util/package.py

@@ -77,32 +77,16 @@ def check_package_exists(package: str) -> bool:
     return any(dist in req for dist in env[req.project_name])
 
 
-def _get_user_site(deps_dir: str) -> tuple:
-    """Get arguments and environment for subprocess used in get_user_site."""
-    env = os.environ.copy()
-    env['PYTHONUSERBASE'] = os.path.abspath(deps_dir)
-    args = [sys.executable, '-m', 'site', '--user-site']
-    return args, env
-
-
-def get_user_site(deps_dir: str) -> str:
-    """Return user local library path."""
-    args, env = _get_user_site(deps_dir)
-    process = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env)
-    stdout, _ = process.communicate()
-    lib_dir = stdout.decode().strip()
-    return lib_dir
-
-
-async def async_get_user_site(deps_dir: str,
-                              loop: asyncio.AbstractEventLoop) -> str:
+async def async_get_user_site(deps_dir: str) -> str:
     """Return user local library path.
 
     This function is a coroutine.
     """
-    args, env = _get_user_site(deps_dir)
+    env = os.environ.copy()
+    env['PYTHONUSERBASE'] = os.path.abspath(deps_dir)
+    args = [sys.executable, '-m', 'site', '--user-site']
     process = await asyncio.create_subprocess_exec(
-        *args, loop=loop, stdin=asyncio.subprocess.PIPE,
+        *args, stdin=asyncio.subprocess.PIPE,
         stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.DEVNULL,
         env=env)
     stdout, _ = await process.communicate()

tests/test_bootstrap.py

@@ -9,7 +9,7 @@ import homeassistant.config as config_util
 from homeassistant import bootstrap
 import homeassistant.util.dt as dt_util
 
-from tests.common import patch_yaml_files, get_test_config_dir
+from tests.common import patch_yaml_files, get_test_config_dir, mock_coro
 
 ORIG_TIMEZONE = dt_util.DEFAULT_TIME_ZONE
 VERSION_PATH = os.path.join(get_test_config_dir(), config_util.VERSION_FILE)
@@ -52,3 +52,55 @@ def test_home_assistant_core_config_validation(hass):
         }
     }, hass)
     assert result is None
+
+
+def test_from_config_dict_not_mount_deps_folder(loop):
+    """Test that we do not mount the deps folder inside from_config_dict."""
+    with patch('homeassistant.bootstrap.is_virtual_env', return_value=False), \
+        patch('homeassistant.core.HomeAssistant',
+              return_value=Mock(loop=loop)), \
+        patch('homeassistant.bootstrap.async_mount_local_lib_path',
+              return_value=mock_coro()) as mock_mount, \
+        patch('homeassistant.bootstrap.async_from_config_dict',
+              return_value=mock_coro()):
+
+        bootstrap.from_config_dict({}, config_dir='.')
+        assert len(mock_mount.mock_calls) == 1
+
+    with patch('homeassistant.bootstrap.is_virtual_env', return_value=True), \
+        patch('homeassistant.core.HomeAssistant',
+              return_value=Mock(loop=loop)), \
+        patch('homeassistant.bootstrap.async_mount_local_lib_path',
+              return_value=mock_coro()) as mock_mount, \
+        patch('homeassistant.bootstrap.async_from_config_dict',
+              return_value=mock_coro()):
+
+        bootstrap.from_config_dict({}, config_dir='.')
+        assert len(mock_mount.mock_calls) == 0
+
+
+async def test_async_from_config_file_not_mount_deps_folder(loop):
+    """Test that we not mount the deps folder inside async_from_config_file."""
+    hass = Mock(async_add_job=Mock(side_effect=lambda *args: mock_coro()))
+
+    with patch('homeassistant.bootstrap.is_virtual_env', return_value=False), \
+        patch('homeassistant.bootstrap.async_enable_logging',
+              return_value=mock_coro()), \
+        patch('homeassistant.bootstrap.async_mount_local_lib_path',
+              return_value=mock_coro()) as mock_mount, \
+        patch('homeassistant.bootstrap.async_from_config_dict',
+              return_value=mock_coro()):
+
+        await bootstrap.async_from_config_file('mock-path', hass)
+        assert len(mock_mount.mock_calls) == 1
+
+    with patch('homeassistant.bootstrap.is_virtual_env', return_value=True), \
+        patch('homeassistant.bootstrap.async_enable_logging',
+              return_value=mock_coro()), \
+        patch('homeassistant.bootstrap.async_mount_local_lib_path',
+              return_value=mock_coro()) as mock_mount, \
+        patch('homeassistant.bootstrap.async_from_config_dict',
+              return_value=mock_coro()):
+
+        await bootstrap.async_from_config_file('mock-path', hass)
+        assert len(mock_mount.mock_calls) == 0

tests/util/test_package.py

@@ -201,20 +201,8 @@ def test_check_package_zip():
     assert not package.check_package_exists(TEST_ZIP_REQ)
 
 
-def test_get_user_site(deps_dir, lib_dir, mock_popen, mock_env_copy):
-    """Test get user site directory."""
-    env = mock_env_copy()
-    env['PYTHONUSERBASE'] = os.path.abspath(deps_dir)
-    args = [sys.executable, '-m', 'site', '--user-site']
-    ret = package.get_user_site(deps_dir)
-    assert mock_popen.call_count == 1
-    assert mock_popen.call_args == call(
-        args, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env)
-    assert ret == lib_dir
-
-
 @asyncio.coroutine
-def test_async_get_user_site(hass, mock_env_copy):
+def test_async_get_user_site(mock_env_copy):
     """Test async get user site directory."""
     deps_dir = '/deps_dir'
     env = mock_env_copy()
@@ -222,10 +210,10 @@ def test_async_get_user_site(hass, mock_env_copy):
     args = [sys.executable, '-m', 'site', '--user-site']
     with patch('homeassistant.util.package.asyncio.create_subprocess_exec',
                return_value=mock_async_subprocess()) as popen_mock:
-        ret = yield from package.async_get_user_site(deps_dir, hass.loop)
+        ret = yield from package.async_get_user_site(deps_dir)
     assert popen_mock.call_count == 1
     assert popen_mock.call_args == call(
-        *args, loop=hass.loop, stdin=asyncio.subprocess.PIPE,
+        *args, stdin=asyncio.subprocess.PIPE,
         stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.DEVNULL,
         env=env)
     assert ret == os.path.join(deps_dir, 'lib_dir')