From 845b0563120d649a84e83eeb68241431ecc43f3e Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Fri, 12 Apr 2024 11:33:58 -0400 Subject: [PATCH 1/5] Revert "fix: `--theme-dirs` argument to `compile_sass` management command" This reverts commit 5fe131c85890857673e41e12a144ec213c91af26. --- openedx/core/djangoapps/theming/helpers.py | 8 +++----- .../theming/management/commands/compile_sass.py | 2 +- openedx/core/djangoapps/theming/tests/test_helpers.py | 1 - 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/theming/helpers.py b/openedx/core/djangoapps/theming/helpers.py index b88bbdca4382..4d1df6418082 100644 --- a/openedx/core/djangoapps/theming/helpers.py +++ b/openedx/core/djangoapps/theming/helpers.py @@ -268,11 +268,9 @@ def get_themes(themes_dir=None): """ if not is_comprehensive_theming_enabled(): return [] - if themes_dir: - themes_dirs = [themes_dir] - else: - themes_dirs = get_theme_base_dirs_unchecked() - return get_themes_unchecked(themes_dirs, settings.PROJECT_ROOT) + if themes_dir is None: + themes_dir = get_theme_base_dirs_unchecked() + return get_themes_unchecked(themes_dir, settings.PROJECT_ROOT) def get_theme_base_dirs_unchecked(): diff --git a/openedx/core/djangoapps/theming/management/commands/compile_sass.py b/openedx/core/djangoapps/theming/management/commands/compile_sass.py index 765ef98aeac3..0e193fb10174 100644 --- a/openedx/core/djangoapps/theming/management/commands/compile_sass.py +++ b/openedx/core/djangoapps/theming/management/commands/compile_sass.py @@ -90,7 +90,7 @@ def parse_arguments(*args, **options): # pylint: disable=unused-argument 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)}) + 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()} diff --git a/openedx/core/djangoapps/theming/tests/test_helpers.py b/openedx/core/djangoapps/theming/tests/test_helpers.py index 95829a16b416..955e723df145 100644 --- a/openedx/core/djangoapps/theming/tests/test_helpers.py +++ b/openedx/core/djangoapps/theming/tests/test_helpers.py @@ -44,7 +44,6 @@ def test_get_themes(self): Theme('red-theme', 'red-theme', get_theme_base_dir('red-theme'), settings.PROJECT_ROOT), Theme('stanford-style', 'stanford-style', get_theme_base_dir('stanford-style'), settings.PROJECT_ROOT), Theme('test-theme', 'test-theme', get_theme_base_dir('test-theme'), settings.PROJECT_ROOT), - Theme('empty-theme', 'empty-theme', get_theme_base_dir('empty-theme'), settings.PROJECT_ROOT), ] actual_themes = get_themes() self.assertCountEqual(expected_themes, actual_themes) From 3d49eac3d59986d39dc69399a7d634591abb786f Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Fri, 12 Apr 2024 11:33:58 -0400 Subject: [PATCH 2/5] Revert "test: compile built-in themes as part of static assets check" This reverts commit 0d52e370f5222dfa7bef267bf1b28bcf6b9a81c2. --- .github/workflows/static-assets-check.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index 369151170a44..5aedfdf52f73 100644 --- a/.github/workflows/static-assets-check.yml +++ b/.github/workflows/static-assets-check.yml @@ -82,5 +82,5 @@ jobs: CMS_CFG: lms/envs/minimal.yml run: | - paver update_assets lms --theme-dirs ./themes - paver update_assets cms --theme-dirs ./themes + paver update_assets lms + paver update_assets cms From a1b9f82a301da40c47da4defa978aa85c4905ecc Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Fri, 12 Apr 2024 11:33:58 -0400 Subject: [PATCH 3/5] Revert "test: add a dummy empty theme" This reverts commit c546bd870ab681ed8b0f4a217dc0cf4b4d7ed78c. --- themes/empty-theme/README.rst | 7 ------- themes/empty-theme/cms/static/sass/.gitignore | 2 -- themes/empty-theme/cms/templates/.gitignore | 2 -- themes/empty-theme/lms/static/sass/.gitignore | 2 -- themes/empty-theme/lms/templates/.gitignore | 2 -- 5 files changed, 15 deletions(-) delete mode 100644 themes/empty-theme/README.rst delete mode 100644 themes/empty-theme/cms/static/sass/.gitignore delete mode 100644 themes/empty-theme/cms/templates/.gitignore delete mode 100644 themes/empty-theme/lms/static/sass/.gitignore delete mode 100644 themes/empty-theme/lms/templates/.gitignore diff --git a/themes/empty-theme/README.rst b/themes/empty-theme/README.rst deleted file mode 100644 index 541e9ac28457..000000000000 --- a/themes/empty-theme/README.rst +++ /dev/null @@ -1,7 +0,0 @@ -Empty Theme -########### - -This is a dummy theme to ensure that the build succeeds on a folder that is -*structured* like a theme, but contains no actual Sass modules nor HTML -templates. Compiling and enabling this theme should be functionally and -visually equivalent to the default theme. diff --git a/themes/empty-theme/cms/static/sass/.gitignore b/themes/empty-theme/cms/static/sass/.gitignore deleted file mode 100644 index 7f1d3dd29328..000000000000 --- a/themes/empty-theme/cms/static/sass/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# This empty gitignore file ensures that this otherwise-empty -# directory is included in version control. diff --git a/themes/empty-theme/cms/templates/.gitignore b/themes/empty-theme/cms/templates/.gitignore deleted file mode 100644 index 7f1d3dd29328..000000000000 --- a/themes/empty-theme/cms/templates/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# This empty gitignore file ensures that this otherwise-empty -# directory is included in version control. diff --git a/themes/empty-theme/lms/static/sass/.gitignore b/themes/empty-theme/lms/static/sass/.gitignore deleted file mode 100644 index 7f1d3dd29328..000000000000 --- a/themes/empty-theme/lms/static/sass/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# This empty gitignore file ensures that this otherwise-empty -# directory is included in version control. diff --git a/themes/empty-theme/lms/templates/.gitignore b/themes/empty-theme/lms/templates/.gitignore deleted file mode 100644 index 7f1d3dd29328..000000000000 --- a/themes/empty-theme/lms/templates/.gitignore +++ /dev/null @@ -1,2 +0,0 @@ -# This empty gitignore file ensures that this otherwise-empty -# directory is included in version control. From 021c9985f425d23bde41e6e62285d82b1dfd73c3 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Fri, 12 Apr 2024 11:33:58 -0400 Subject: [PATCH 4/5] Revert "fix: NameErrors in compile_sass.py" This reverts commit 2cd3dc844dd2ea38cfeda7a70973a192f69041b6. --- scripts/compile_sass.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/compile_sass.py b/scripts/compile_sass.py index 41a2d56b3bda..97f800a34fe7 100755 --- a/scripts/compile_sass.py +++ b/scripts/compile_sass.py @@ -259,7 +259,7 @@ def compile_sass_dir( click.secho(f" Done.", fg="green") # 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_root) + "/**/*.scss"): + for sass_path in glob.glob(str(source) + "/**/*.scss"): if Path(sass_path).name.startswith("_"): # Don't generate RTL CSS for partials continue @@ -270,7 +270,7 @@ def compile_sass_dir( # Don't generate RTL CSS if there is an explicit Sass version for RTL continue click.echo(" Generating missing right-to-left CSS:") - source_css_file = sass_path.replace(str(source_root), str(target_root)).replace( + source_css_file = sass_path.replace(str(source), str(dest)).replace( ".scss", ".css" ) target_css_file = source_css_file.replace(".css", "-rtl.css") From 50d6b9e6b5ce587a1a46ef926e32477ef15e6880 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Fri, 12 Apr 2024 11:33:58 -0400 Subject: [PATCH 5/5] Revert "revert: revert: build: remove dependency on Python `sass` module" This reverts commit f57d412c714e92a7446c794716815dfe6a7a66ac. --- scripts/compile_sass.py | 124 +++++++++------------------------------- 1 file changed, 28 insertions(+), 96 deletions(-) diff --git a/scripts/compile_sass.py b/scripts/compile_sass.py index 97f800a34fe7..5698b2eaa6cb 100755 --- a/scripts/compile_sass.py +++ b/scripts/compile_sass.py @@ -42,9 +42,7 @@ from __future__ import annotations import glob -import os import subprocess -import sys from pathlib import Path import click @@ -155,108 +153,42 @@ def main( def compile_sass_dir( message: str, - source_root: Path, - target_root: Path, + source: Path, + dest: Path, includes: list[Path], tolerate_missing: bool = False, ) -> None: """ Compile a directory of Sass into a target CSS directory, and generate any missing RTL CSS. - Structure of source dir is mirrored in target dir. - IMPLEMENTATION NOTES: - ===================== - - libsass is a C++ library for compiling Sass (ref: https://github.com/sass/libsass). - - libsass-python is a small PyPI package wrapping libsass, including: - * The `_sass` module, which provides direct Python bindings for the C++ library. - (ref: https://github.com/sass/libsass-python/blob/0.10.0/pysass.cpp) - * The `sass` module, which adds some friendly Pythonic wrapper functions around `_sass`, - notably `sass.compile_dirname(...)`. - (ref: https://github.com/sass/libsass-python/blob/0.10.0/sass.py#L198-L201) - - Our legacy Sass code only works with a super old version of libsass (3.3.2,) which is provided to us by a super - old version of libsass-python (0.10.0). In this super old libsass-python version: - * the `sass` module DOESN'T support Python 3.11+, but - * the `_sass` module DOES support Python 3.11+. - - Upgrading our Sass to work with newer a libsass version would be arduous and would potentially break - comprehensive themes, so we don't want to do that. Forking libsass-python at v0.10.0 and adding Python 3.11+ - support would mean adding another repo to the openedx org. Rather than do either of those, we've decided to - hack around the problem by just reimplementing what we need of `sass.compile_dirname` here, directly on top - of the `_sass` C++ binding module. - - Eventually, we may eschew libsass-python altogether by switching to SassC@3.3.2, a direct CLI for libsass@3.3.2. - (ref: https://github.com/sass/sassc). This would be nice because it would allow us to remove Python from the - Sass build pipeline entirely. However, it would mean explicitly compiling & installing both libsass and SassC - within the edx-platform Dockerfile, which has its own drawbacks. + Structure of source dir is mirrored in target dir. """ - # Constants from libsass-python - SASS_STYLE_NESTED = 0 - _SASS_STYLE_EXPANDED = 1 - _SASS_STYLE_COMPACT = 2 - SASS_STYLE_COMPRESSED = 3 - SASS_COMMENTS_NONE = 0 - SASS_COMMENTS_LINE_NUMBERS = 1 - - # Defaults from libass-python - precision = 5 - source_map_filename = None - custom_functions = [] - importers = None - - use_dev_settings: bool = NORMALIZED_ENVS[env] == "development" - fs_encoding: str = sys.getfilesystemencoding() or sys.getdefaultencoding() - output_style: int = SASS_STYLE_NESTED if use_dev_settings else SASS_STYLE_COMPRESSED - source_comments: int = SASS_COMMENTS_LINE_NUMBERS if use_dev_settings else SASS_COMMENTS_NONE - include_paths: bytes = os.pathsep.join(str(include) for include in includes).encode(fs_encoding) - + use_dev_settings = NORMALIZED_ENVS[env] == "development" click.secho(f" {message}...", fg="cyan") - click.secho(f" Source: {source_root}") - click.secho(f" Target: {target_root}") - if not source_root.is_dir(): + click.secho(f" Source: {source}") + click.secho(f" Target: {dest}") + if not source.is_dir(): if tolerate_missing: - click.secho(f" Skipped because source directory does not exist.", fg="yellow") + click.secho(f" Skipped because source directory does not exist.", fg="yellow") return else: - raise FileNotFoundError(f"missing Sass source dir: {source_root}") - click.echo(f" Include paths:") + raise FileNotFoundError(f"missing Sass source dir: {source}") + click.echo(f" Include paths:") for include in includes: - click.echo(f" {include}") - - click.echo(f" Files:") - for dirpath, _, filenames in os.walk(str(source_root)): - for filename in filenames: - if filename.startswith('_'): - continue - if not filename.endswith(('.scss', '.sass')): - continue - source = Path(dirpath) / filename - target = (target_root / source.relative_to(source_root)).with_suffix('.css') - click.echo(f" {source} -> {target}") - 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. - from _sass import compile_filename # pylint: disable=protected-access - success, output, _ = compile_filename( - str(source).encode(fs_encoding), - output_style, - source_comments, - include_paths, - precision, - source_map_filename, - custom_functions, - importers, - ) - output_text = output.decode('utf-8') - if not success: - raise Exception(f"Failed to compile {source}: {output_text}") - target.parent.mkdir(parents=True, exist_ok=True) - with open(target, 'w', encoding="utf-8") as target_file: - target_file.write(output_text) - - click.secho(f" Done.", fg="green") + 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.secho(f" Compiled.", fg="green") # 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"): @@ -269,16 +201,16 @@ def compile_sass_dir( 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 CSS:") + click.echo(" Generating missing right-to-left CSS:") 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: {source_css_file}") - click.echo(f" Target: {target_css_file}") + click.echo(f" Source: {source_css_file}") + click.echo(f" Target: {target_css_file}") if not dry: subprocess.run(["rtlcss", source_css_file, target_css_file]) - click.secho(" Generated.", fg="green") + click.secho(" Generated.", fg="green") # Information click.secho(f"USING ENV: {NORMALIZED_ENVS[env]}", fg="blue")