Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: finish replacing paver assets #34554

Merged
merged 1 commit into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading