From b9b6a7fe9d49f5f5892ba9b9a58dc07003de7a73 Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Tue, 11 Jul 2023 17:25:55 -0400 Subject: [PATCH] build: `npm run compile-sass` TODO update ADR TODO make compile-requirements TODO testing TODO: will copy in commit message from PR when squash-merging Part of: TODO --- .../0017-reimplement-asset-processing.rst | 24 +- package.json | 6 +- requirements/constraints.txt | 7 +- requirements/edx/assets.in | 5 + requirements/edx/assets.txt | 3 + requirements/edx/base.txt | 6 - requirements/edx/development.in | 1 + requirements/edx/doc.txt | 6 - requirements/edx/kernel.in | 1 - requirements/edx/paver.in | 1 - requirements/edx/paver.txt | 3 - requirements/edx/testing.txt | 4 - scripts/compile_sass | 413 ++++++++++++++++++ 13 files changed, 445 insertions(+), 35 deletions(-) create mode 100644 requirements/edx/assets.in create mode 100644 requirements/edx/assets.txt create mode 100755 scripts/compile_sass diff --git a/docs/decisions/0017-reimplement-asset-processing.rst b/docs/decisions/0017-reimplement-asset-processing.rst index 8405d605fed4..5eb4ecafde94 100644 --- a/docs/decisions/0017-reimplement-asset-processing.rst +++ b/docs/decisions/0017-reimplement-asset-processing.rst @@ -128,9 +128,11 @@ The three top-level edx-platform asset processing actions are *build*, *collect* A Python-defined task that calls out to each build stage. - - ``scripts/build-assets.sh`` + - ``npm clean-install && npm run build`` + + Simple NPM wrappers around the build stages. The wrappers will be written in Bash and tested on both GNU+Linux and macOS. - A Bash script that contains all build stages, with subcommands available for running each stage separately. Its command-line interface inspired by Tutor's ``openedx-assets`` script. The script will be runnable on any POSIX system, including macOS and Ubuntu and it will linted for common shell scripting mistakes using `shellcheck `_. + These commands are a "one stop shop" for building assets, but more efficiency-oriented users may choose to run build stages individually. * - + **Build stage 1: Copy npm-installed assets** from node_modules to other folders in edx-platform. They are used by certain especially-old legacy LMS & CMS frontends that are not set up to work with npm directly. @@ -174,27 +176,25 @@ The three top-level edx-platform asset processing actions are *build*, *collect* Note: libsass is pinned to a 2015 version with a non-trivial upgrade path. Installing it requires compiling a large C extension, noticeably affecting Docker image build time. - - ``scripts/build-assets.sh css`` + - ``scripts/compile_sass`` - Bash reimplementation, calling ``node-sass`` and ``rtlcss``. + TODO describe - The initial implementation of build-assets.sh may use ``sassc``, a CLI provided by libsass, instead of node-sass. Then, ``sassc`` can be replaced by ``node-sass`` as part of a subsequent `edx-platform frontend framework upgrade effort `_. + TODO mention `edx-platform frontend framework upgrade effort `_. * - + **Build stage 5: Compile themes' SCSS** into CSS for legacy LMS/CMS frontends. The default SCSS is used as a base, and theme-provided SCSS files are used as overrides. Themes are searched for from some number of operator-specified theme directories. - ``./manage.py [lms|cms] compile_sass``, or - ``paver compile_sass --theme-dirs ...`` + ``paver compile_sass --theme-dirs X Y --themes A B`` The management command is a wrapper around the paver task. The former looks up the list of theme search directories from Django settings and site configuration; the latter requires them to be supplied as arguments. - ``./manage.py [lms|cms] compile_sass``, or - ``scripts/build-assets.sh themes --theme-dirs ...`` - - The management command will remain available, but it will need to be updated to point at the Bash script, which will replace the paver task (see build stage 4 for details). + - ``scripts/compile_sass --theme-dir X --theme-dir Y --theme A --theme B`` - The overall asset *build* action will use the Bash script; this means that list of theme directories will need to be provided as arguments, but it ensures that the build can remain Python-free. + The management command will remain available, but it will need to be updated to point at ``compile_sass``, which will replace the paver task (see build stage 4 for details). * - **Collect** the built static assets from edx-platform to another location (the ``STATIC_ROOT``) so that they can be efficiently served *without* Django's webserver. This step, by nature, requires Python and Django in order to find and organize the assets, which may come from edx-platform itself or from its many installed Python and NPM packages. This is only needed for **production** environments, where it is usually desirable to serve assets with something efficient like NGINX. @@ -313,9 +313,9 @@ Either way, the migration path is straightforward: * - ``openedx-assets xmodule`` - (no longer needed) * - ``openedx-assets common`` - - ``scripts/build-assets.sh css`` + - ``compile_sass --skip-themes`` * - ``openedx-assets themes`` - - ``scripts/build-assets.sh themes`` + - ``compile_sass --skip-default`` * - ``openedx-assets webpack [--env=dev]`` - ``npm run webpack[-dev]`` * - ``openedx-assets collect`` diff --git a/package.json b/package.json index 5927c685b1c1..d8385ce8344b 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,12 @@ "repository": "https://github.com/openedx/edx-platform", "scripts": { "postinstall": "scripts/copy-node-modules.sh", + "build": "npm run webpack && npm run compile-sass", + "build-dev": "npm run webpack-dev && npm run compile-sass-dev", "webpack": "NODE_ENV=${NODE_ENV:-production} \"$(npm bin)/webpack\" --progress --config=${WEBPACK_CONFIG_PATH:-webpack.prod.config.js}", - "webpack-dev": "NODE_ENV=development WEBPACK_CONFIG_PATH=webpack.dev.config.js npm run webpack" + "webpack-dev": "NODE_ENV=development WEBPACK_CONFIG_PATH=webpack.dev.config.js npm run webpack", + "compile-sass": "compile_sass", + "compile-sass-dev": "compile_sass --env=dev" }, "dependencies": { "@babel/core": "7.19.0", diff --git a/requirements/constraints.txt b/requirements/constraints.txt index c96ddb4ba00a..f82824d5ec6f 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -110,6 +110,11 @@ drf-yasg<1.21.6 # Adding pin to avoid any major upgrade djangorestframework<3.15.0 - # tests failing with greater version. Fix this in separate ticket. pillow<10.0.0 + +# Our legacy Sass code is incompatible with anything except this ancient libsass version. +# Here is a ticket to upgrade, but it's of debatable importance given that we are rapidly moving +# away from legacy LMS/CMS frontends: +# https://github.com/openedx/edx-platform/issues/31616 +libsass==0.10.0 diff --git a/requirements/edx/assets.in b/requirements/edx/assets.in new file mode 100644 index 000000000000..84dcb11611f5 --- /dev/null +++ b/requirements/edx/assets.in @@ -0,0 +1,5 @@ +-c constraints.txt + +click +libsass +nodeenv diff --git a/requirements/edx/assets.txt b/requirements/edx/assets.txt new file mode 100644 index 000000000000..4693a51a3eda --- /dev/null +++ b/requirements/edx/assets.txt @@ -0,0 +1,3 @@ +click==8.1.5 +libsass==0.10.0 +nodeenv==1.8.0 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 7201a2b19d9f..30f4d5f79288 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -676,10 +676,6 @@ lazy==1.5 # xblock learner-pathway-progress==1.3.4 # via -r requirements/edx/bundled.in -libsass==0.10.0 - # via - # -r requirements/edx/paver.txt - # ora2 loremipsum==1.0.5 # via ora2 lti-consumer-xblock==9.5.5 @@ -747,8 +743,6 @@ newrelic==8.8.1 # edx-django-utils nltk==3.8.1 # via chem -nodeenv==1.8.0 - # via -r requirements/edx/kernel.in numpy==1.22.4 # via # chem diff --git a/requirements/edx/development.in b/requirements/edx/development.in index ffdc299eddd9..306f3d77df4a 100644 --- a/requirements/edx/development.in +++ b/requirements/edx/development.in @@ -13,6 +13,7 @@ -r ../pip-tools.txt # pip-tools and its dependencies, for managing requirements files -r testing.txt # Dependencies for running the various test suites -r doc.txt # Dependencies for building the documentation locally. +-r assets.txt # Dependencies for rebuilding static assets. click # Used for perf_tests utilities in modulestore django-debug-toolbar # A set of panels that display debug information about the current request/response diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index d9aa9770b8dc..ba46533b3a47 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -802,10 +802,6 @@ lazy==1.5 # xblock learner-pathway-progress==1.3.4 # via -r requirements/edx/base.txt -libsass==0.10.0 - # via - # -r requirements/edx/base.txt - # ora2 loremipsum==1.0.5 # via # -r requirements/edx/base.txt @@ -887,8 +883,6 @@ nltk==3.8.1 # via # -r requirements/edx/base.txt # chem -nodeenv==1.8.0 - # via -r requirements/edx/base.txt numpy==1.22.4 # via # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 43395d9f7efe..d6e12cab3add 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -108,7 +108,6 @@ mako # Primary template language used for server- Markdown # Convert text markup to HTML; used in capa problems, forums, and course wikis mongoengine # Object-document mapper for MongoDB, used in the LMS dashboard mysqlclient # Driver for the default production relational database -nodeenv # Utility for managing Node.js environments; we use this for deployments and testing oauthlib # OAuth specification support for authenticating via LTI or other Open edX services olxcleaner openedx-calc # Library supporting mathematical calculations for Open edX diff --git a/requirements/edx/paver.in b/requirements/edx/paver.in index caaf349592d2..d0c4b7dadda8 100644 --- a/requirements/edx/paver.in +++ b/requirements/edx/paver.in @@ -12,7 +12,6 @@ edx-opaque-keys # Create and introspect course and xblock identities lazy # Lazily-evaluated attributes for Python objects -libsass==0.10.0 # Python bindings for the LibSass CSS compiler markupsafe # XML/HTML/XHTML Markup safe strings mock # Stub out code with mock objects and make assertions about how they have been used path # Easier manipulation of filesystem paths diff --git a/requirements/edx/paver.txt b/requirements/edx/paver.txt index 34b6b91243b2..c75a8993bb72 100644 --- a/requirements/edx/paver.txt +++ b/requirements/edx/paver.txt @@ -16,8 +16,6 @@ idna==3.4 # via requests lazy==1.5 # via -r requirements/edx/paver.in -libsass==0.10.0 - # via -r requirements/edx/paver.in markupsafe==2.1.3 # via -r requirements/edx/paver.in mock==5.1.0 @@ -43,7 +41,6 @@ requests==2.31.0 # via -r requirements/edx/paver.in six==1.16.0 # via - # libsass # paver # python-memcached stevedore==5.1.0 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 6dab6bac6bc0..551914b9405d 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -869,10 +869,6 @@ lazy-object-proxy==1.9.0 # via astroid learner-pathway-progress==1.3.4 # via -r requirements/edx/base.txt -libsass==0.10.0 - # via - # -r requirements/edx/base.txt - # ora2 loremipsum==1.0.5 # via # -r requirements/edx/base.txt diff --git a/scripts/compile_sass b/scripts/compile_sass new file mode 100755 index 000000000000..d78b5a66606b --- /dev/null +++ b/scripts/compile_sass @@ -0,0 +1,413 @@ +#!/usr/bin/env python +""" +Defines a CLI for compiling Sass (both default and themed) into CSS. + +Should be run from the root of edx-platform. +Requirements for this scripts are stored in requirements/edx/compile-sass.in. + +Usage (from a provisioned environment like Tutor or Devstack): + + # Just run the script, referring to --help for details. + scripts/compile_sass --help + +Usage (standalone): + + # Active a venv if you haven't already. + python -m venv venv + . venv/bin/activate + + # Install prereqs. + pip install -r requirements/edx/compile-sass.txt + + # Run the script (refer to --help for details). + scripts/compile_sass --help + +This script is intentionally implemented in a very simplistic way. It prefers repetition +over abstraction, and its dependencies are minimal (just click and libsass-python, ideally). +We do this because: + +* If and when we migrate from libsass-python to something less ancient like node-sass or dart-sass, + we will want to re-write this script in Bash or JavaScript so that it can work without any + backend tooling. By keeping the script dead simple, that will be easier. This is the same reason + why we did not append '.py' to the name of this script. +* The features this script supports (legacy frontends & comprehensive theming) are on the way out, + in favor of micro-frontends, branding, and Paragon design tokens. We're not sure how XBlock + view styling will fit into that, but it probably can be much simpler than comprehensive theming. + So, we don't need this script to be modular and extensible. We just need it to be obvious, robust, + and easy to maintain until we can delete it. + +Why no .py extension? + +See docs/decisions/0017-reimplement-asset-processing.rst for more details. +""" +from __future__ import annotations + +import glob +import subprocess +from pathlib import Path + +import click + + +@click.option( + "-T", + "--theme-dir", + "theme_dirs", + metavar="PATH", + multiple=True, + envvar="EDX_PLATFORM_THEME_DIRS", + type=click.Path( + exists=True, file_okay=False, readable=True, writable=True, path_type=Path + ), + help=( + "Consider sub-dirs of PATH as themes. " + "Multiple theme dirs are accepted. " + "If none are provided, we look at colon-separated paths on the EDX_PLATFORM_THEME_DIRS env var." + ), +) +@click.option( + "-t", + "--theme", + "themes", + metavar="NAME", + multiple=True, + type=str, + help=( + "A theme to compile. " + "NAME should be a sub-dir of a PATH provided by --theme-dir. " + "Multiple themes are accepted. " + "If none are provided, all available themes are compiled." + ), +) +@click.option( + "--skip-default", + is_flag=True, + help="Don't compile default Sass.", +) +@click.option( + "--skip-themes", + is_flag=True, + help="Don't compile any themes (overrides --theme* options).", +) +@click.option( + "--skip-lms", + is_flag=True, + help="Don't compile any LMS Sass.", +) +@click.option( + "--skip-cms", + is_flag=True, + help="Don't compile any CMS Sass.", +) +@click.option( + "--env", + type=click.Choice(["dev", "prod"]), + default="prod", + help="Optimize CSS for this environment. Defaults to 'prod'.", +) +@click.option( + "--dry", + is_flag=True, + help="Print what would be compiled, but don't compile it.", +) +@click.option( + "-h", + "--help", + "show_help", + is_flag=True, + help="Print this help.", +) +@click.command() +@click.pass_context +def main( + context: click.Context, + theme_dirs: list[Path], + themes: list[str], + skip_default: bool, + skip_themes: bool, + skip_lms: bool, + skip_cms: bool, + env: str, + dry: bool, + show_help: bool, +) -> None: + """ + Compile Sass for edx-platform and its themes. + + Default Sass is compiled unless explicitly skipped. + Additionally, any number of themes may be specified using --theme-dir and --theme. + + Default CSS is compiled to css/ directories in edx-platform. + Themed CSS is compiled to css/ directories in their source themes. + """ + + def compile_sass_dir( + message: str, + source: Path, + dest: Path, + includes: list[Path], + tolerate_missing: bool = False, + ) -> None: + """ + TODO + """ + use_dev_settings = env == "development" + click.echo(f" {message}...") + click.echo(f" Source: {source}") + click.echo(f" Destination: {dest}") + if not source.is_dir(): + if tolerate_missing: + click.echo(f" Skipped because source directory does not exist.") + return + else: + raise FileNotFoundError(f"missing Sass source dir: {source}") + click.echo(f" Include paths:") + for include in includes: + click.echo(f" {include}") + if not dry: + # Import sass late so that this script can be dry-run without installing + # libsass, which takes a while as it must be compiled from its C source. + import sass + + dest.mkdir(parents=True, exist_ok=True) + sass.compile( + dirname=(str(source), str(dest)), + include_paths=[str(include_path) for include_path in includes], + source_comments=use_dev_settings, + output_style=("nested" if use_dev_settings else "compressed"), + ) + click.echo(f" Compiled.") + # For Sass files without explicit RTL versions, generate + # an RTL version of the CSS using the rtlcss library. + for sass_path in glob.glob(str(source) + "/**/*.scss"): + if Path(sass_path).name.startswith("_"): + # Don't generate RTL CSS for partials + continue + if sass_path.endswith("-rtl.scss"): + # Don't generate RTL CSS if the file is itself an RTL version + continue + if Path(sass_path.replace(".scss", "-rtl.scss")).exists(): + # Don't generate RTL CSS if there is an explicit Sass version for RTL + continue + click.echo(" Generating missing right-to-left Sass:") + source_css_file = sass_path.replace(str(source), str(dest)).replace( + ".scss", ".css" + ) + target_css_file = source_css_file.replace(".css", "-rtl.css") + click.echo(f" {source_css_file} -> ") + click.echo(f" {target_css_file}") + if not dry: + subprocess.run(["rtlcss", source_css_file, target_css_file]) + + if show_help: + click.echo(context.get_help()) + return + if skip_lms and skip_cms: + click.echo("Skipping both LMS and CMS... nothing to do! Will exit.") + return + + # Build a list of theme paths: + if skip_themes: + theme_paths = [] + else: + theme_paths = [ + theme_dir / theme + # For every theme dir, + for theme_dir in theme_dirs + for theme in ( + # for every theme name (if theme names provided), + themes + or + # or for every subdir of theme dirs (if no theme name provided), + (theme_dir_entry.name for theme_dir_entry in theme_dir.iterdir()) + ) + # consider the path a theme if it has a lms/ or cms/ subdirectory. + if (theme_dir / theme / "lms").is_dir() or (theme_dir / theme / "cms").is_dir() + ] + + # We expect this script to be run from the edx-platform root. + repo = Path(".") + if not (repo / "xmodule").is_dir(): + # Sanity check: If the xmodule/ folder is missing, we're definitely not at the root + # of edx-platform, so save the user some headache by exiting early. + raise Exception(f"{__file__} must be run from the root of edx-platform") + + # Every Sass compilation will use have these directories as lookup paths, + # regardless of theme. + common_includes = [ + repo / "common" / "static", + repo / "common" / "static" / "sass", + repo / "node_modules" / "@edx", + repo / "node_modules", + ] + + if not skip_default: + click.echo(f"Compiling default Sass...") + if not skip_lms: + compile_sass_dir( + "Compiling default LMS Sass", + repo / "lms" / "static" / "sass", + repo / "lms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + ) + compile_sass_dir( + "Compiling default certificate Sass", + repo / "lms" / "static" / "certificates" / "sass", + repo / "lms" / "static" / "certificates" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for default LMS", + repo / "xmodule" / "assets", + repo / "lms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + if not skip_cms: + compile_sass_dir( + "Compiling default CMS Sass", + repo / "cms" / "static" / "sass", + repo / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass", + ], + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for default CMS", + repo / "xmodule" / "assets", + repo / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + click.echo(f"Done compiling default Sass!") + + for theme in theme_paths: + click.echo(f"Compiling Sass for theme at {theme}...") + if not skip_lms: + compile_sass_dir( + "Compiling default LMS Sass with themed partials", + repo / "lms" / "static" / "sass", + theme / "lms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling themed LMS Sass as overrides to CSS from previous step", + theme / "lms" / "static" / "sass", + theme / "lms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling themed certificate Sass", + theme / "lms" / "static" / "certificates" / "sass", + theme / "lms" / "static" / "certificates" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + theme / "lms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for themed LMS", + repo / "xmodule" / "assets", + theme / "lms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + if not skip_cms: + compile_sass_dir( + "Compiling default CMS Sass with themed partials", + repo / "cms" / "static" / "sass", + theme / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling themed CMS Sass as overrides to CSS from previous step", + theme / "cms" / "static" / "sass", + theme / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for themed CMS", + repo / "xmodule" / "assets", + theme / "cms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + click.echo(f"Done compiling Sass for theme at {theme}!") + + click.echo() + click.echo("Successfully compiled:") + if not skip_default: + click.echo(f" - {repo.absolute()} (default Sass)") + for theme in theme_paths: + click.echo(f" - {theme}") + if skip_lms: + click.echo(f"(skipped LMS)") + if skip_cms: + click.echo(f"(skipped CMS)") + + +if __name__ == "__main__": + main()