Fixes three NameErrors which were introduced by the previous commit,
leading to it being reverted.
These NameErrors slipped through because the script was not tested on
any themes that had zero SCSS overrides.
Instead of using libsass-python's `sass` module, which is incompatible
with Python 3.11, we now directly use libsass-python's `_sass` module,
which directly binds to the underlying C++ libsass library.
This ensures that the Sass build will not a blocker for the edx-platform
Python 3.11 upgrade, which needs to land in Redwood.
For more details, see the docstring at scripts/compile_sass.py#L167
The implementation of `npm run watch-sass` was trying really hard
to recompile precisely only the Sass that needed to be recompiled, but in
order to do so, it had to spin up several `watchmedo` processes
per theme. These processes would trigger one another sometimes, leading
to infinite recompilation loops.
Rather than figure out all the dependency directions and messing with
`watchmedo`, I've opted to simplify the script to invoke a single
`watchmedo` process per theme. A single theme recompiles within
seconds, so I think this is a good compromise, one which makes the
script easier to reason about will help me move pass this legacy
assets work.
All CI used to go through scripts/generic-ci-tests.sh, which is a
wrapper around various `paver` test/linting/check invocations.
These days, most edx-platform CI checks just invoke their tools (pylint,
pycodestyle, pytest, etc.) directly.
In anticipation of the proposed Paver deprecation [1], let's remove
the parts of this script that aren't used any more, including several
`paver` command invocations. This should have no impact on CI.
Furthermore, we are able to remove the SHARD environment variable,
which was formely used to split unit and quality checks up into
smaller pieces. Unit tests and pylint checks now have their own
separate sharding logic, so there is only one "quality" shard remaining
(SHARD=4, ie generic quality checks), thus we don't need a SHARD
variable at all.
[1] https://github.com/openedx/edx-platform/issues/34467
Without this change, `npm run postinstall` (aka
scripts/copy-node-modules.sh) uses `set -x`, which echo steps to STDERR.
By default, it seems that GitHub Actions doesn't show STDERR.
Having the steps visible in CI was very useful while debugging some
Tutor build improvements, so I figured it would be good to upstream the
change.
This should probably be turned into an ADR at some point, but for
now I've just recreated the page on the public wiki. (The space it
was in had been deleted during the 2U/Axim split.)
Implements the `npm run watch` section of the assets ADR [1], plus some
modifications since I decided to switch from pywatchman to watchdog (see
ADR changes for justification). This will replace `paver watch_assets`
(edx-platform) and `openedx-assets watch-themes` (Tutor).
Specifically, this PR adds three experimental commands:
* `npm run watch-sass` : Watch for Sass changes with watchdog.
* `npm run watch-webpack` : Invoke Webpack-watch for JS changes.
* `npm run watch` : Invoke both `watch-sass` and `watch-webpack` simultaneously.
These commands are only intended to work in development mode. They have
been tested both on bare-metal edx-platform and through `tutor dev run`
on on Linux.
Before removing the "experimental" label, we need to:
* Test through Devstack on Linux.
* Test through Devstack and `tutor dev run` on macOS.
* Test on bare-metal macOS. Might not work, which is OK, but we should
document that.
* Document the commands in edx-platform's README.
* Confirm that this not only works through `tutor dev run`, but also as
a suitable replacement in the `watchthemes` service that Tutor runs
automatically as part of `tutor dev start`. Tweak if necessary.
References:
1. https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst
Part of: https://github.com/openedx/edx-platform/issues/31612
This PR implements much of the static assets rework ADR [1], including:
* `npm run build[-dev]`, and its subcommands,
* `npm run webpack[-dev]` and
* `npm run compile-sass[-dev]`.
This is backwards-compatible. `paver update_assets` should not be affected.
The new command warns that it is "experimental" for a few reasons:
* `npm run build` will fail in the webpack phase unless you first
run `xmodule_assets`. This will be changed soon [2].
* We have tested the new build, but not quite so thoroughly that we'd
recommend it as the production default yet. Once the xmodule_assets
work lands, we'll share this on the forums so early adopters can try it
out.
* The commands lack some top-level documentation. Once they stabilize more,
we'll add a section to the README that explains how and when to use `npm run
build` and its subcommands and its env vars.
* `npm run watch` is not yet implemented.
References:
1. https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst
2. https://github.com/openedx/edx-platform/pull/32685
Part of: https://github.com/openedx/edx-platform/issues/31604
During the review of ADR 17 [1], Régis pointed out [2] that the shell script
which replaces Paver's `process_npm_assets` could be automatically invoked as
an NPM post-install hook, ensuring that the step is seamlessly executed whenever
`npm install` is run. I had avoided using that suggestion, as I worried that it
would make it harder to move node_modules out of the edx-platform directory in
Tutor's openedx image.
Since then, two things have changed. Firstly, Tutor v16's new persistent mounts
interface [3] has lessened the importance of moving node_modules. Secondly, I
have realized that using a post-install hook would not preclude us from
modifying the underlying script (scripts/copy-node-modules.sh) to look in an
alternative location for node_modules, should that end up being something we
want to do.
This commit modifies the ADR based on those findings, stubs out Paver's
`process_npm_assets`, and adds the suggested post-install hook and replacement
Bash script.
References:
1. https://github.com/openedx/edx-platform/blob/master/docs/decisions/0017-reimplement-asset-processing.rst
2. https://github.com/openedx/edx-platform/pull/31790#discussion_r1122802492
3. https://github.com/overhangio/tutor/blob/master/CHANGELOG.md#v1600-2023-06-14
Part of: https://github.com/openedx/edx-platform/issues/31604
