build: finish replacing paver assets (#34554)

Together, these changes make it so that all features of the Paver-based
asset compilation system are supported with drop-in Paver-free
replacements. The remaining Paver asset functions are trivial wrappers,
which can be comfortably deleted before Sumac.

* Turn `./manage.py ... compile_sass` into a simple wrapper around `npm
  run compile-sass`
* Turn `paver webpack` into a simple wrapper around `npm run webpack`
* Turn `pavelib.assets:collect_assets` into a simple wrapper around
  `./manage.py ... collectstatic`
* Add/improve deprecation warnings for all Paver asset commands.
* Load defaults for asset-related Django settings from environment
  variables. This allows the build to work without Python. For the
  settings which will be removed in Sumac, I've added deprecation
  warnings.
* Change EDX_PLATFORM_THEME_DIRS env var to COMPREHENSIVE_THEME_DIRS.
  This simplifies the migration instructions, because all the new env
  vars now match their corresponding Django settings. This amends an
  ADR, but it should not be a breaking change because the  env var was
  recently added (since Quince) and nobody should be using it yet.
* Future-proof the static assets ADR with links. The linked pages will
  be kept up-to-date even if the ADR isn't.

Part of: #34467

cms/envs/common.py

@@ -39,6 +39,7 @@
 # pylint: disable=unused-import, useless-suppression, wrong-import-order, wrong-import-position
 
 import importlib.util
+import json
 import os
 import sys
 
@@ -1275,14 +1276,11 @@
 
 # Static content
 STATIC_URL = '/static/studio/'
-STATIC_ROOT = ENV_ROOT / "staticfiles" / 'studio'
+STATIC_ROOT = os.environ.get('STATIC_ROOT_CMS', ENV_ROOT / 'staticfiles' / 'studio')
 
 STATICFILES_DIRS = [
     COMMON_ROOT / "static",
     PROJECT_ROOT / "static",
-
-    # This is how you would use the textbook images locally
-    # ("book", ENV_ROOT / "book_images"),
 ]
 
 # Locale/Internationalization
@@ -1322,11 +1320,16 @@
 EMBARGO_SITE_REDIRECT_URL = None
 
 ##### custom vendor plugin variables #####
-# JavaScript code can access this data using `process.env.JS_ENV_EXTRA_CONFIG`
-# One of the current use cases for this is enabling custom TinyMCE plugins
-# (TINYMCE_ADDITIONAL_PLUGINS) and overriding the TinyMCE configuration
-# (TINYMCE_CONFIG_OVERRIDES).
-JS_ENV_EXTRA_CONFIG = {}
+
+# .. setting_name: JS_ENV_EXTRA_CONFIG
+# .. setting_default: {}
+# .. setting_description: JavaScript code can access this dictionary using `process.env.JS_ENV_EXTRA_CONFIG`
+#   One of the current use cases for this is enabling custom TinyMCE plugins
+#   (TINYMCE_ADDITIONAL_PLUGINS) and overriding the TinyMCE configuration (TINYMCE_CONFIG_OVERRIDES).
+# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer
+#   use Django settings. Please set the JS_ENV_EXTRA_CONFIG environment variable to an equivalent JSON
+#   string instead. For details, see: https://github.com/openedx/edx-platform/issues/31895
+JS_ENV_EXTRA_CONFIG = json.loads(os.environ.get('JS_ENV_EXTRA_CONFIG', '{}'))
 
 ############################### PIPELINE #######################################
 
@@ -1503,7 +1506,14 @@
         'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-worker-stats.json')
     }
 }
-WEBPACK_CONFIG_PATH = 'webpack.prod.config.js'
+
+# .. setting_name: WEBPACK_CONFIG_PATH
+# .. setting_default: "webpack.prod.config.js"
+# .. setting_description: Path to the Webpack configuration file. Used by Paver scripts.
+# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer
+#   use Django settings. Please set the WEBPACK_CONFIG_PATH environment variable instead. For details,
+#   see: https://github.com/openedx/edx-platform/issues/31895
+WEBPACK_CONFIG_PATH = os.environ.get('WEBPACK_CONFIG_PATH', 'webpack.prod.config.js')
 
 
 ############################ SERVICE_VARIANT ##################################
@@ -2180,8 +2190,11 @@
 
 # .. setting_name: COMPREHENSIVE_THEME_DIRS
 # .. setting_default: []
-# .. setting_description: See LMS annotation.
-COMPREHENSIVE_THEME_DIRS = []
+# .. setting_description: A list of paths to directories, each of which will
+#   be searched for comprehensive themes. Do not override this Django setting directly.
+#   Instead, set the COMPREHENSIVE_THEME_DIRS environment variable, using colons (:) to
+#   separate paths.
+COMPREHENSIVE_THEME_DIRS = os.environ.get("COMPREHENSIVE_THEME_DIRS", "").split(":")
 
 # .. setting_name: COMPREHENSIVE_THEME_LOCALE_PATHS
 # .. setting_default: []

docs/decisions/0017-reimplement-asset-processing.rst

@@ -11,16 +11,14 @@ Overview
 Status
 ******
 
-**Provisional**
+**Accepted**
 
-This was `originally authored <https://github.com/openedx/edx-platform/pull/31790>`_ in March 2023. We `modified it in July 2023 <https://github.com/openedx/edx-platform/pull/32804>`_ based on learnings from the implementation process.
+This was `originally authored <https://github.com/openedx/edx-platform/pull/31790>`_ in March 2023. We `modified it in July 2023 <https://github.com/openedx/edx-platform/pull/32804>`_ based on learnings from the implementation process, and then `modified and it again in May 2024 <https://github.com/openedx/edx-platform/pull/34554>`_ to make the migration easier for operators to understand.
 
-The status will be moved to *Accepted* upon completion of reimplementation. Related work:
+Related deprecation tickets:
 
 * `[DEPR]: Asset processing in Paver <https://github.com/openedx/edx-platform/issues/31895>`_
-* `Process edx-platform assets without Paver <https://github.com/openedx/edx-platform/issues/31798>`_
-* `Process edx-platform assets without Python <https://github.com/openedx/edx-platform/issues/31800>`_
-
+* `[DEPR]: Paver <https://github.com/openedx/edx-platform/issues/34467>`_
 
 Context
 *******
@@ -92,7 +90,6 @@ Three particular issues have surfaced in Developer Experience Working Group disc
 
 All of these potential solutions would involve refactoring or entirely replacing parts of the current asset processing system.
 
-
 Decision
 ********
 
@@ -114,6 +111,9 @@ Reimplementation Specification
 Commands and stages
 -------------------
 
+**May 2024 update:** See the `static assets reference <../references/static-assets.rst>`_ for
+the latest commands.
+
 The three top-level edx-platform asset processing actions are *build*, *collect*, and *watch*. The build action can be further broken down into five stages. Here is how those actions and stages will be reimplemented:
 
 
@@ -226,6 +226,9 @@ The three top-level edx-platform asset processing actions are *build*, *collect*
 Build Configuration
 -------------------
 
+**May 2024 update:** See the `static assets reference <../references/static-assets.rst>`_ for
+the latest configuration settings.
+
 To facilitate a generally Python-free build reimplementation, we will require that certain Django settings now be specified as environment variables, which can be passed to the build like so::
 
   MY_ENV_VAR="my value" npm run build    # Set for the whole build.
@@ -266,7 +269,7 @@ Some of these options will remain as Django settings because they are used in ed
    * - ``COMPREHENSIVE_THEME_DIRS``
      - Directories that will be searched when compiling themes.
      - ``COMPREHENSIVE_THEME_DIRS``
-     - ``EDX_PLATFORM_THEME_DIRS``
+     - ``COMPREHENSIVE_THEME_DIRS``
 
 Migration
 =========
@@ -285,37 +288,16 @@ As a consequence of this ADR, Tutor will either need to:
 * reimplement the script as a thin wrapper around the new asset processing commands, or
 * deprecate and remove the script.
 
-Either way, the migration path is straightforward:
-
-.. list-table::
-   :header-rows: 1
-
-   * - Existing Tutor-provided command
-     - New upstream command
-   * - ``openedx-assets build``
-     - ``npm run build``
-   * - ``openedx-assets npm``
-     - ``scripts/copy-node-modules.sh  # (automatically invoked by 'npm install'!)``
-   * - ``openedx-assets xmodule``
-     - (no longer needed)
-   * - ``openedx-assets common``
-     - ``npm run compile-sass -- --skip-themes``
-   * - ``openedx-assets themes``
-     - ``npm run compile-sass -- --skip-default``
-   * - ``openedx-assets webpack``
-     - ``npm run webpack``
-   * - ``openedx-assets collect``
-     - ``./manage.py [lms|cms] collectstatic --noinput``
-   * - ``openedx-assets watch-themes``
-     - ``npm run watch``
-
-The options accepted by ``openedx-assets`` will all be valid inputs to ``scripts/build-assets.sh``.
+**May 2024 update:** The ``openedx-assets`` script will be removed from Tutor,
+with migration instructions documented in
+`Tutor's changelog <https://github.com/overhangio/tutor/blob/master/CHANGELOG.md>`_.
 
 non-Tutor migration guide
 -------------------------
 
-Operators using distributions other than Tutor should refer to the upstream edx-platform changes described above in **Reimplementation Specification**, and adapt them accordingly to their distribution.
-
+A migration guide for site operators who are directly referencing Paver will be
+included in the
+`Paver deprecation ticket <https://github.com/openedx/edx-platform/issues/34467>`_.
 
 See also
 ********

lms/envs/common.py

@@ -1928,7 +1928,7 @@ def _make_mako_template_dirs(settings):
 
 # Static content
 STATIC_URL = '/static/'
-STATIC_ROOT = ENV_ROOT / "staticfiles"
+STATIC_ROOT = os.environ.get('STATIC_ROOT_LMS', ENV_ROOT / "staticfiles")
 STATIC_URL_BASE = '/static/'
 
 STATICFILES_DIRS = [
@@ -2822,7 +2822,14 @@ def _make_locale_paths(settings):  # pylint: disable=missing-function-docstring
         'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-worker-stats.json')
     }
 }
-WEBPACK_CONFIG_PATH = 'webpack.prod.config.js'
+
+# .. setting_name: WEBPACK_CONFIG_PATH
+# .. setting_default: "webpack.prod.config.js"
+# .. setting_description: Path to the Webpack configuration file. Used by Paver scripts.
+# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer
+#   use Django settings. Please set the WEBPACK_CONFIG_PATH environment variable instead. For details,
+#   see: https://github.com/openedx/edx-platform/issues/31895
+WEBPACK_CONFIG_PATH = os.environ.get('WEBPACK_CONFIG_PATH', 'webpack.prod.config.js')
 
 ########################## DJANGO DEBUG TOOLBAR ###############################
 
@@ -4549,9 +4556,11 @@ def _make_locale_paths(settings):  # pylint: disable=missing-function-docstring
 
 # .. setting_name: COMPREHENSIVE_THEME_DIRS
 # .. setting_default: []
-# .. setting_description: A list of directories containing themes folders,
-#   each entry should be a full path to the directory containing the theme folder.
-COMPREHENSIVE_THEME_DIRS = []
+# .. setting_description: A list of paths to directories, each of which will
+#   be searched for comprehensive themes. Do not override this Django setting directly.
+#   Instead, set the COMPREHENSIVE_THEME_DIRS environment variable, using colons (:) to
+#   separate paths.
+COMPREHENSIVE_THEME_DIRS = os.environ.get("COMPREHENSIVE_THEME_DIRS", "").split(":")
 
 # .. setting_name: COMPREHENSIVE_THEME_LOCALE_PATHS
 # .. setting_default: []

openedx/core/djangoapps/theming/management/commands/compile_sass.py

@@ -1,21 +1,22 @@
 """
 Management command for compiling sass.
-"""
 
+DEPRECATED in favor of `npm run compile-sass`.
+"""
+import shlex
 
-from django.core.management import BaseCommand, CommandError
-from paver.easy import call_task
+from django.core.management import BaseCommand
+from django.conf import settings
 
-from openedx.core.djangoapps.theming.helpers import get_theme_base_dirs, get_themes, is_comprehensive_theming_enabled
-from pavelib.assets import ALL_SYSTEMS
+from pavelib.assets import run_deprecated_command_wrapper
 
 
 class Command(BaseCommand):
     """
     Compile theme sass and collect theme assets.
     """
 
-    help = 'Compile and collect themed assets...'
+    help = "DEPRECATED. Use 'npm run compile-sass' instead."
 
     # NOTE (CCB): This allows us to compile static assets in Docker containers without database access.
     requires_system_checks = []
@@ -28,7 +29,7 @@ def add_arguments(self, parser):
                 parser (django.core.management.base.CommandParser): parsed for parsing command line arguments.
         """
         parser.add_argument(
-            'system', type=str, nargs='*', default=ALL_SYSTEMS,
+            'system', type=str, nargs='*', default=["lms", "cms"],
             help="lms or studio",
         )
 
@@ -55,7 +56,7 @@ def add_arguments(self, parser):
             '--force',
             action='store_true',
             default=False,
-            help="Force full compilation",
+            help="DEPRECATED. Full recompilation is now always forced.",
         )
         parser.add_argument(
             '--debug',
@@ -64,77 +65,48 @@ def add_arguments(self, parser):
             help="Disable Sass compression",
         )
 
-    @staticmethod
-    def parse_arguments(*args, **options):  # pylint: disable=unused-argument
-        """
-        Parse and validate arguments for compile_sass command.
-
-        Args:
-            *args: Positional arguments passed to the update_assets command
-            **options: optional arguments passed to the update_assets command
-        Returns:
-            A tuple containing parsed values for themes, system, source comments and output style.
-            1. system (list): list of system names for whom to compile theme sass e.g. 'lms', 'cms'
-            2. theme_dirs (list): list of Theme objects
-            3. themes (list): list of Theme objects
-            4. force (bool): Force full compilation
-            5. debug (bool): Disable Sass compression
-        """
-        system = options.get("system", ALL_SYSTEMS)
-        given_themes = options.get("themes", ["all"])
-        theme_dirs = options.get("theme_dirs", None)
-
-        force = options.get("force", True)
-        debug = options.get("debug", True)
-
-        if theme_dirs:
-            available_themes = {}
-            for theme_dir in theme_dirs:
-                available_themes.update({t.theme_dir_name: t for t in get_themes(theme_dir)})
-        else:
-            theme_dirs = get_theme_base_dirs()
-            available_themes = {t.theme_dir_name: t for t in get_themes()}
-
-        if 'no' in given_themes or 'all' in given_themes:
-            # Raise error if 'all' or 'no' is present and theme names are also given.
-            if len(given_themes) > 1:
-                raise CommandError("Invalid themes value, It must either be 'all' or 'no' or list of themes.")
-        # Raise error if any of the given theme name is invalid
-        # (theme name would be invalid if it does not exist in themes directory)
-        elif (not set(given_themes).issubset(list(available_themes.keys()))) and is_comprehensive_theming_enabled():
-            raise CommandError(
-                "Given themes '{themes}' do not exist inside any of the theme directories '{theme_dirs}'".format(
-                    themes=", ".join(set(given_themes) - set(available_themes.keys())),
-                    theme_dirs=theme_dirs,
-                ),
-            )
-
-        if "all" in given_themes:
-            themes = list(available_themes.values())
-        elif "no" in given_themes:
-            themes = []
-        else:
-            # convert theme names to Theme objects, this will remove all themes if theming is disabled
-            themes = [available_themes.get(theme) for theme in given_themes if theme in available_themes]
-
-        return system, theme_dirs, themes, force, debug
-
     def handle(self, *args, **options):
         """
         Handle compile_sass command.
         """
-        system, theme_dirs, themes, force, debug = self.parse_arguments(*args, **options)
-        themes = [theme.theme_dir_name for theme in themes]
-
-        if options.get("themes", None) and not is_comprehensive_theming_enabled():
-            # log a warning message to let the user know that asset compilation for themes is skipped
-            self.stdout.write(
-                self.style.WARNING(
-                    "Skipping theme asset compilation: enable theming to process themed assets"
+        systems = set(
+            {"lms": "lms", "cms": "cms", "studio": "cms"}[sys]
+            for sys in options.get("system", ["lms", "cms"])
+        )
+        theme_dirs = options.get("theme_dirs", settings.COMPREHENSIVE_THEME_DIRS) or []
+        themes_option = options.get("themes", [])  # '[]' means 'all'
+        if not settings.ENABLE_COMPREHENSIVE_THEMING:
+            compile_themes = False
+            themes = []
+        elif "no" in themes_option:
+            compile_themes = False
+            themes = []
+        elif "all" in themes_option:
+            compile_themes = True
+            themes = []
+        else:
+            compile_themes = True
+            themes = themes_option
+        run_deprecated_command_wrapper(
+            old_command="./manage.py [lms|cms] compile_sass",
+            ignored_old_flags=list(set(["force"]) & set(options)),
+            new_command=shlex.join([
+                "npm",
+                "run",
+                ("compile-sass-dev" if options.get("debug") else "compile-sass"),
+                "--",
+                *(["--skip-lms"] if "lms" not in systems else []),
+                *(["--skip-cms"] if "cms" not in systems else []),
+                *(["--skip-themes"] if not compile_themes else []),
+                *(
+                    arg
+                    for theme_dir in theme_dirs
+                    for arg in ["--theme-dir", str(theme_dir)]
                 ),
-            )
-
-        call_task(
-            'pavelib.assets.compile_sass',
-            options={'system': system, 'theme_dirs': theme_dirs, 'themes': themes, 'force': force, 'debug': debug},
+                *(
+                    arg
+                    for theme in themes
+                    for arg in ["--theme", theme]
+                ),
+            ]),
         )