Deprecation: remove "use system packages" (python.system_packages c…

…onfig key and UI checkbox) (#10562) * Docs: do not mention `python.system_packages` This option doesn't work on builds using `build.os` because the Python binary used it's not the one from the system, but another one compiled via `asdf`. This means that creating a virtualenv with access to the "system packages" has no effect at all. As a first step, I'm removing this option from the documentation to avoid confusions. Next, we should probably our deprecation policy to contact users ands remove it from the code as well. Closes #10500 * Build: remove "use system package" option - Removed from the UI - Removed from the v1 and v2 config file * Do not remove the DB field for now We should remove this field after the deploy. * Remove `use_system_site_packages` leftovers * Display custom error messages based on the config key * Apply suggestions from code review Co-authored-by: Eric Holscher <[email protected]> * Minor code style changes * Update readthedocs/config/config.py Co-authored-by: Anthony <[email protected]> --------- Co-authored-by: Eric Holscher <[email protected]> Co-authored-by: Anthony <[email protected]>
readthedocs · Aug 22, 2023 · bcac962 · bcac962
1 parent 97b3b25
commit bcac962
Show file tree

Hide file tree

Showing 20 changed files with 56 additions and 341 deletions.
diff --git a/docs/user/api/v3.rst b/docs/user/api/v3.rst
@@ -697,7 +697,6 @@ Build details
                             "requirements": ".../stable/tools/docs-requirements.txt"
                         }
                     ],
-                    "use_system_site_packages": false
                 },
                 "conda": null,
                 "build": {

diff --git a/docs/user/config-file/v1.rst b/docs/user/config-file/v1.rst
@@ -167,23 +167,6 @@ the highest supported minor version will be selected.
     python:
        version: 3.5
 
-python.use_system_site_packages
-```````````````````````````````
-
-* Default: ``false``
-* Type: Boolean
-
-When true, it gives the virtual environment access to the global site-packages directory.
-Depending on the :ref:`config-file/v1:build.image`,
-Read the Docs includes some libraries like scipy, numpy, etc.
-See :doc:`/builds` for more details.
-
-.. code-block:: yaml
-
-    python:
-       use_system_site_packages: true
-
-
 python.setup_py_install
 ```````````````````````
 

diff --git a/docs/user/config-file/v2.rst b/docs/user/config-file/v2.rst
@@ -130,7 +130,6 @@ Configuration of the Python environment to be used.
            - docs
        - method: pip
          path: another/package
-     system_packages: true
 
 python.version
 ``````````````
@@ -228,20 +227,6 @@ With the previous settings, Read the Docs will execute the next commands:
 
    pip install .[docs]
 
-python.system_packages
-``````````````````````
-
-Give the virtual environment access to the global site-packages directory.
-
-:Type: ``bool``
-:Default: ``false``
-
-.. warning::
-
-  If you are using a :ref:`Conda <config-file/v2:conda>` environment
-  to manage the build, this setting will not have any effect, since
-  the virtual environment creation is managed by Conda.
-
 conda
 ~~~~~
 
@@ -869,8 +854,6 @@ Changes
 - The settings ``python.setup_py_install`` and ``python.pip_install`` were replaced by ``python.install``.
   And now it accepts a path to the package.
   See :ref:`config-file/v2:Packages`.
-- The setting ``python.use_system_site_packages`` was renamed to ``python.system_packages``.
-  See :ref:`config-file/v2:python.system_packages`.
 - The build will fail if there are invalid keys (strict mode).
 
 .. warning::

diff --git a/readthedocs/api/v2/serializers.py b/readthedocs/api/v2/serializers.py
@@ -84,7 +84,6 @@ class Meta(ProjectSerializer.Meta):
             "container_mem_limit",
             "container_time_limit",
             "install_project",
-            "use_system_packages",
             "skip",
             "requirements_file",
             "python_interpreter",

diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py
@@ -2,6 +2,7 @@
 
 """Build configuration for rtd."""
 
+import collections
 import copy
 import os
 import re
@@ -128,14 +129,44 @@ class InvalidConfig(ConfigError):
 
     """Error for a specific key validation."""
 
-    message_template = 'Invalid "{key}": {error}'
+    # Define the default message to show on ``InvalidConfig``
+    default_message_template = 'Invalid configuration option "{key}"'
+
+    # Create customized message for based on each particular ``key``
+    message_templates = collections.defaultdict(lambda: "{default_message}: {error}")
+
+    # Redirect the user to the blog post when using
+    # `python.system_packages` or `python.use_system_site_packages`
+    message_templates.update(
+        {
+            "python.system_packages": "{default_message}. "
+            "This configuration key has been deprecated and removed. "
+            "Refer to https://blog.readthedocs.com/use-system-packages-deprecated/ to read more about this change and how to upgrade your config file."  # noqa
+        }
+    )
+    # Use same message for `python.use_system_site_packages`
+    message_templates.update(
+        {
+            "python.use_system_site_packages": message_templates.get(
+                "python.system_packages"
+            )
+        }
+    )
 
     def __init__(self, key, code, error_message, source_file=None):
         self.key = key
         self.code = code
         self.source_file = source_file
-        message = self.message_template.format(
-            key=self._get_display_key(),
+
+        display_key = self._get_display_key()
+        default_message = self.default_message_template.format(
+            key=display_key,
+            code=code,
+            error=error_message,
+        )
+        message = self.message_templates[display_key].format(
+            default_message=default_message,
+            key=display_key,
             code=code,
             error=error_message,
         )
@@ -211,18 +242,10 @@ def __init__(self, env_config, raw_config, source_file, base_path=None):
 
     def error(self, key, message, code):
         """Raise an error related to ``key``."""
-        if not os.path.isdir(self.source_file):
-            source = os.path.relpath(self.source_file, self.base_path)
-            error_message = '{source}: {message}'.format(
-                source=source,
-                message=message,
-            )
-        else:
-            error_message = message
         raise InvalidConfig(
             key=key,
             code=code,
-            error_message=error_message,
+            error_message=message,
             source_file=self.source_file,
         )
 
@@ -507,10 +530,8 @@ def validate_build(self):
     def validate_python(self):
         """Validates the ``python`` key, set default values it's necessary."""
         install_project = self.defaults.get('install_project', False)
-        use_system_packages = self.defaults.get('use_system_packages', False)
         version = self.defaults.get('python_version', '2')
         python = {
-            'use_system_site_packages': use_system_packages,
             'install_with_pip': False,
             'extra_requirements': [],
             'install_with_setup': install_project,
@@ -526,13 +547,6 @@ def validate_python(self):
                     code=PYTHON_INVALID,
                 )
 
-            # Validate use_system_site_packages.
-            if 'use_system_site_packages' in raw_python:
-                with self.catch_validation_error('python.use_system_site_packages'):
-                    python['use_system_site_packages'] = validate_bool(
-                        raw_python['use_system_site_packages'],
-                    )
-
             # Validate pip_install.
             if 'pip_install' in raw_python:
                 with self.catch_validation_error('python.pip_install'):
@@ -663,7 +677,6 @@ def python(self):
         return Python(
             version=python['version'],
             install=python_install,
-            use_system_site_packages=python['use_system_site_packages'],
         )
 
     @property
@@ -967,7 +980,6 @@ def validate_python(self):
         Fall back to the defaults of:
         - ``requirements``
         - ``install`` (only for setup.py method)
-        - ``system_packages``
 
         .. note::
            - ``version`` can be a string or number type.
@@ -1011,13 +1023,6 @@ def validate_python(self):
             for index in range(len(raw_install))
         ]
 
-        with self.catch_validation_error('python.system_packages'):
-            system_packages = self.pop_config(
-                'python.system_packages',
-                False,
-            )
-            python['use_system_site_packages'] = validate_bool(system_packages)
-
         return python
 
     def validate_python_install(self, index):
@@ -1275,18 +1280,14 @@ def validate_keys(self):
         This should be called after all the validations are done and all keys
         are popped from `self._raw_config`.
         """
-        msg = (
-            'Invalid configuration option: {}. '
-            'Make sure the key name is correct.'
-        )
         # The version key isn't popped, but it's
         # validated in `load`.
         self.pop_config('version', None)
         wrong_key = '.'.join(self._get_extra_key(self._raw_config))
         if wrong_key:
             self.error(
-                wrong_key,
-                msg.format(wrong_key),
+                key=wrong_key,
+                message="Make sure the key name is correct.",
                 code=INVALID_KEY,
             )
 
@@ -1354,7 +1355,6 @@ def python(self):
         return Python(
             version=python.get('version'),
             install=python_install,
-            use_system_site_packages=python['use_system_site_packages'],
         )
 
     @property

diff --git a/readthedocs/config/models.py b/readthedocs/config/models.py
@@ -80,8 +80,7 @@ def __init__(self, **kwargs):
 
 
 class Python(Base):
-
-    __slots__ = ('version', 'install', 'use_system_site_packages')
+    __slots__ = ("version", "install")
 
 
 class PythonInstallRequirements(Base):