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

revert: build: finish replacing paver #34700

Merged
merged 1 commit into from
May 6, 2024
Merged
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: 12 additions & 25 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
@@ -39,7 +39,6 @@
# pylint: disable=unused-import, useless-suppression, wrong-import-order, wrong-import-position

import importlib.util
import json
import os
import sys

@@ -1276,11 +1275,14 @@

# Static content
STATIC_URL = '/static/studio/'
STATIC_ROOT = os.environ.get('STATIC_ROOT_CMS', ENV_ROOT / 'staticfiles' / 'studio')
STATIC_ROOT = 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
@@ -1320,16 +1322,11 @@
EMBARGO_SITE_REDIRECT_URL = None

##### custom vendor plugin variables #####

# .. 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', '{}'))
# 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 = {}

############################### PIPELINE #######################################

@@ -1506,14 +1503,7 @@
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-worker-stats.json')
}
}

# .. 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')
WEBPACK_CONFIG_PATH = 'webpack.prod.config.js'


############################ SERVICE_VARIANT ##################################
@@ -2190,11 +2180,8 @@

# .. setting_name: COMPREHENSIVE_THEME_DIRS
# .. setting_default: []
# .. 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_description: See LMS annotation.
COMPREHENSIVE_THEME_DIRS = []

# .. setting_name: COMPREHENSIVE_THEME_LOCALE_PATHS
# .. setting_default: []
52 changes: 35 additions & 17 deletions docs/decisions/0017-reimplement-asset-processing.rst
Original file line number Diff line number Diff line change
@@ -11,14 +11,16 @@ Overview
Status
******

**Accepted**
**Provisional**

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.
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.

Related deprecation tickets:
The status will be moved to *Accepted* upon completion of reimplementation. Related work:

* `[DEPR]: Asset processing in Paver <https://github.com/openedx/edx-platform/issues/31895>`_
* `[DEPR]: Paver <https://github.com/openedx/edx-platform/issues/34467>`_
* `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>`_


Context
*******
@@ -90,6 +92,7 @@ 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
********

@@ -111,9 +114,6 @@ 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,9 +226,6 @@ 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.
@@ -269,7 +266,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``
- ``COMPREHENSIVE_THEME_DIRS``
- ``EDX_PLATFORM_THEME_DIRS``

Migration
=========
@@ -288,16 +285,37 @@ 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.

**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>`_.
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``.

non-Tutor migration guide
-------------------------

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>`_.
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.


See also
********
19 changes: 5 additions & 14 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
@@ -1928,7 +1928,7 @@ def _make_mako_template_dirs(settings):

# Static content
STATIC_URL = '/static/'
STATIC_ROOT = os.environ.get('STATIC_ROOT_LMS', ENV_ROOT / "staticfiles")
STATIC_ROOT = ENV_ROOT / "staticfiles"
STATIC_URL_BASE = '/static/'

STATICFILES_DIRS = [
@@ -2822,14 +2822,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring
'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-worker-stats.json')
}
}

# .. 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')
WEBPACK_CONFIG_PATH = 'webpack.prod.config.js'

########################## DJANGO DEBUG TOOLBAR ###############################

@@ -4556,11 +4549,9 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring

# .. setting_name: COMPREHENSIVE_THEME_DIRS
# .. setting_default: []
# .. 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_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_name: COMPREHENSIVE_THEME_LOCALE_PATHS
# .. setting_default: []
124 changes: 76 additions & 48 deletions openedx/core/djangoapps/theming/management/commands/compile_sass.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
"""
Management command for compiling sass.

DEPRECATED in favor of `npm run compile-sass`.
"""
import shlex

from django.core.management import BaseCommand
from django.conf import settings

from pavelib.assets import run_deprecated_command_wrapper
from django.core.management import BaseCommand, CommandError
from paver.easy import call_task

from openedx.core.djangoapps.theming.helpers import get_theme_base_dirs, get_themes, is_comprehensive_theming_enabled
from pavelib.assets import ALL_SYSTEMS


class Command(BaseCommand):
"""
Compile theme sass and collect theme assets.
"""

help = "DEPRECATED. Use 'npm run compile-sass' instead."
help = 'Compile and collect themed assets...'

# NOTE (CCB): This allows us to compile static assets in Docker containers without database access.
requires_system_checks = []
@@ -29,7 +28,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=["lms", "cms"],
'system', type=str, nargs='*', default=ALL_SYSTEMS,
help="lms or studio",
)

@@ -56,7 +55,7 @@ def add_arguments(self, parser):
'--force',
action='store_true',
default=False,
help="DEPRECATED. Full recompilation is now always forced.",
help="Force full compilation",
)
parser.add_argument(
'--debug',
@@ -65,48 +64,77 @@ def add_arguments(self, parser):
help="Disable Sass compression",
)

def handle(self, *args, **options):
@staticmethod
def parse_arguments(*args, **options): # pylint: disable=unused-argument
"""
Handle compile_sass command.
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
"""
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 = []
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:
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)]
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,
),
*(
arg
for theme in themes
for arg in ["--theme", theme]
)

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"
),
]),
)

call_task(
'pavelib.assets.compile_sass',
options={'system': system, 'theme_dirs': theme_dirs, 'themes': themes, 'force': force, 'debug': debug},
)
Loading
Loading