Skip to content

Commit

Permalink
build: finish replacing paver assets (openedx#34554)
Browse files Browse the repository at this point in the history
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: openedx#34467
  • Loading branch information
kdmccormick authored May 6, 2024
1 parent fc68d34 commit 3f0f7ce
Show file tree
Hide file tree
Showing 9 changed files with 375 additions and 633 deletions.
37 changes: 25 additions & 12 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 #######################################

Expand Down Expand Up @@ -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 ##################################
Expand Down Expand Up @@ -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: []
Expand Down
52 changes: 17 additions & 35 deletions docs/decisions/0017-reimplement-asset-processing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
*******
Expand Down Expand Up @@ -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
********

Expand All @@ -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:


Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
=========
Expand All @@ -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
********
Expand Down
19 changes: 14 additions & 5 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 ###############################

Expand Down Expand Up @@ -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: []
Expand Down
124 changes: 48 additions & 76 deletions openedx/core/djangoapps/theming/management/commands/compile_sass.py
Original file line number Diff line number Diff line change
@@ -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 = []
Expand All @@ -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",
)

Expand All @@ -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',
Expand All @@ -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]
),
]),
)
Loading

0 comments on commit 3f0f7ce

Please sign in to comment.