build: restructure "vendor" directories to improve build

[kdmccormick]

Context

(Part of: openedx/wg-devops#21)

Our overall goal is to minimize Docker image rebuild time while also maintaining Docker image correctness for edx-platform developers.

As of recently, Tutor lets edx-platform developers use their local edx-platform repository as input into the Docker image build. Before this, developers would either have to:

• sacrifice speed by pushing their code to a remote git repository+branch and then tell Tutor to build from that repository+branch, or
• sacrifice correctness by bind-mounting edx-platform into a container running an image build from master (this is also what Devstack does).

Tutor achieves this improvement by bind-mounting several source items (files or directories) from edx-platform into the Docker build context during each build step. When choosing which source items to bind-mount at each build step, we have several constraints:

1. To achieve correctness, we must bind-mount all items that are significant to the build step.
2. To achieve correctness, we must not bind-mount items that were previously modified by the build (because bind-mounting would overwrite the modifications).
3. To maximize speed, we avoid bind-mounting items that are not significant to the build step.
4. Finally, Docker mandates that source directories cannot be written to while they are bind-mounted.

For example, look at the build step that installs NPM packages:

1. We bind-mount package.json, package-lock.json, and copy-node-modules.sh as source items, as they all affect npm clean-install.
2. This step generates node_modules. In future build steps, we cannot bind-mount node_modules (or any parent directory) from edx-platform, because that would clobber the output of npm clean-install.
3. We do not bind-mount Python requirements, JS sources, etc., as changes to them should not affect npm clean-install.
4. None of the bind-mounted files are modified by npm clean-install. Note that running npm install would fail because it could modify package-lock.json.

The problem is that the current structure of edx-platform, particularly the "vendor" folders, makes it impossible to follow these constraints when building static assets. Tutor currently works around this by sacrificing some correctness: static assets sources are not bind-mounted, so local changes to static assets will not be reflected in the image build.

This PR fixes the structure of edx-platform vendor folders. This will benefit Tutor users, but it will generally benefit anyone who is trying to write a finely-tuned Dockerfile for edx-platform.

Change Descriptions

Changes Visualized

 BEFORE                                                         >>   AFTER
=======================================================================================================================================
 edx-platform                                                   >>   edx-platform
 ├── lms                                                        >>   ├── lms
 │   └── static                                                 >>   │   └── static
 │       └── css                                                >>   │       └── css
 │           ├── ...                                            >>   │           ├── ...
 │           ├── ...  (Generated:                               >>   │           ├── ...  (Generated:
 │           ├── ...   Compiled LMS CSS)                        >>   │           ├── ...   Compiled LMS CSS)
 │           ├── ...                                            >>   │           └── ...
 │           └── vendor                                         >>   │
 │               ├── ...                                        >>   │
 │               ├── ... (Source:                               >>   │
 │               ├── ...  LMS "vendored-in" CSS libs)           >>   │
 │               └── ...                                        >>   │
 └── common                                                     >>   └── common
     └── static                                                 >>       └── static
         ├── common                                             >>           ├── common
         │   ├── js                                             >>           │   ├── js
         │   │   ├── ...                                        >>           │   │   ├── ...
         │   │   ├── ...(Source:                                >>           │   │   ├── ...(Source:
         │   │   ├── ... LMS+CMS JS to be Webpacked)            >>           │   │   ├── ... LMS+CMS JS to be Webpacked)
         │   │   ├── ...                                        >>           │   │   ├── ...
         │   │   └── vendor                                     >>           │   │   └── vendor ==[symlink]==> ../../node_copies/js
         │   │       ├── ...                                    >>           │   │
         │   │       ├── ... (Generated/Source:                 >>           │   │
         │   │       ├── ...  JS copied from node_modules)      >>           │   │
         │   │       └── ...                                    >>           │   │
         │   └── css                                            >>           │   └── css                      
         │       └── vendor                                     >>           │       └── vendor ==[symlink]==> ../../node_copies/css
         │           ├── ...                                    >>           │
         │           ├── ... (Generated/Source:                 >>           │
         │           ├── ...  CSS copied from node_modules)     >>           │
         │           └── ...                                    >>           │
         └── css                                                >>           ├── css
             ├── ...                                            >>           │   ├── ...
             ├── ... (Source:                                   >>           │   ├── ... (Source:
             ├── ...  Common styles)                            >>           │   ├── ...  Common styles)
             ├── ...                                            >>           │   ├── ...
             └── vendor                                         >>           │   └── vendor
                 ├── ...                                        >>           │       ├── ...
                 ├── ... (Source:                               >>           │       ├── ... (Source:
                 ├── ...  Common "vendored-in" CSS libs         >>           │       ├── ...  Common+LMS "vendored-in" CSS libs)
                 └── ...                                        >>           │       └── ...
                                                                >>           └── node_copies
                                                                >>               ├── css                  
                                                                >>               │   ├── ...       
                                                                >>               │   ├── ... (Generated/Source:
                                                                >>               │   ├── ...  CSS copied from node_modules)
                                                                >>               │   └── ...      
                                                                >>               └── js  
                                                                >>                   ├── ...
                                                                >>                   ├── ... (Generated/Source:
                                                                >>                   ├── ...  JS copied from node_modules)
                                                                >>                   └── ...

Change 1: build: lms/static/css/vendor/* -> common/static/css/vendor

Sass compilation includes the LMS's "vendored-in" CSS libraries at lms/static/css/vendor as a source, and output CSS is written to the parent directory, lms/static/css. This violates the constraint that source directories cannot be written to while they are bind-mounted.

To resolve the issue, we merge the contents of lms/static/css/vendor into common/static/css/vendor. (The directory common/static/css only contains sources: no CSS is outputted there.)

Now, common/static/css only contains sources for the Sass build, and lms/static/css is only used for output.

Change 2: common/static/common/[js|css]/vendor -> common/static/node_copies/[js|css]

To support our old RequireJS-based frontends, a post-install hook on npm clean-install copies certain JS & CSS modules into common/static/common/[js|css]/vendor. However, the copied modules' ancestor directory, common/static/common, is a source for the Webpack build that we need to bind-mount. This violates the constraint that we must not bind-mount items that were previously modified by the build.

To resolve the issue, we relocate common/static/common/[js|css]/vendor to new directories, common/static/node_copies/[js|css]. We provide a symlink from the original location to a new location.

Now, common/static/common and common/static/node_copies can both be used as inputs to the Webpack build, but the latter will not be clobbered when we bind-mount the former. Furthermore, the new directory name ("node_copies") is
more illustrative than the old one ("vendor").

Note: I originally attempted to make this change without the symlink--I updated all references to the old path to the new path. I struggled for several hours to get tests passing for JS using RequireJS, and concluded that I don't understand our RequireJS setup well enough to safely make that change.

Bonus change: log npm run postinstall steps to STDOUT so they are visible in CI

This is a small logging improvement that I ended up needing in order to debug this PR, which was helpful enough that I've decided to keep in the PR.

Testing

Here's how I tested:

Setup

I tested this edx-platform branch three different configurations, using latest Tutor Nightly and Devstack:

tutor dev (with bindmount)

# Nav into edx-platform, switch to this branch, and clean out generated files
cd ~/openedx/edx-platform
sudo chown "$USER" -R .  # fix perms in case you were just using devstack
git switch kdmccormick/asset-folders
git clean . -f
make clean

# Build image without bind-mount, and provision repo
tutor mounts remove .
tutor config save --unset EDX_PLATFORM_VERSION --unset EDX_PLATFORM_REPOSITORY
tutor images build openedx-dev
tutor dev launch --non-interactive

# Build image with this repo bind-mounted and re-provision/re-launch
tutor mounts add .
tutor images build openedx-dev
tutor dev launch --non-interactive

tutor local (no bindmount)

# Un-mount edx-platform
tutor mounts remove .

# Config Tutor to build prod image from this remote branch
tutor config save --set EDX_PLATFORM_REPOSITORY=https://github.com/kdmccormick/edx-platform
tutor config save --set EDX_PLATFORM_VERSION=kdmccormick/asset-folders
tutor images build openedx

# Provision & launch.
tutor local launch --non-interactive

devstack

# Nav into edx-platform, switch to this branch, and clean out generated files
cd ~/openedx/edx-platform
git switch kdmccormick/asset-folders
git clean . -f
make clean

# Nav to devstack, provision, and start.
cd ~/openedx/devstack
make dev.provision.lms+cms
make dev.start.lms+cms

Test playbook

I ran through this basic flow, keeping an eye out for any visual issues or misbehaving components:

• Go to Studio home (logged out)
• Log in as admin
• Click into a course
• Add a section, subsection, and unit
• Click into a unit
• Add a problem, video, html, and drag&drop
• Publish
• Edit each of those
• Publish
• Click "View Preview"
• Navigate backwards, into previous sequence
• Click "View in Studio"
• Click "View Live"
• Navigate forwards, into next sequence
• Go into the dev tools -> netowrk log, hard refresh, and ensure there is a file loaded named "jquery.treeview.css". This was one of the vendored-in CSS files that was moved.
• Go to instructor dashboard
• Click around a bit

[kdmccormick]

@feanil Looks like I'll need to merge this one last assets PR in order for the optimized Tutor image build to work.

The change is here kinda nuanced, and I'm not sure I did the best job describing it. Review if you can; if not, I can come back and rework the description once I'm back from PTO.

[kdmccormick]

Eh, nevermind, tests are failing. Putting this back in draft state for now.

[openedx-webhooks]

Thanks for the pull request, @kdmccormick! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

⚠️ We can't start reviewing your pull request until you've submitted a signed contributor agreement or indicated your institutional affiliation. Please see the CONTRIBUTING file for more information. If you've signed an agreement in the past, you may need to re-sign. See The New Home of the Open edX Codebase for details.

Once you've signed the CLA, please allow 1 business day for it to be processed. After this time, you can re-run the CLA check by editing the PR title. If the problem persists, you can tag the @openedx/cla-problems team in a comment on your PR for further assistance.

[kdmccormick]

I've realized that I can use COPY --from=(bind-mount) rather than RUN --mount=from=(bind-mount) in Dockerfiles, which gives me much more flexibility to work around the weirdness of these vendor directories as they are today. I'm going to close this in order to avoid sinking more time into it.

Two much smaller followup PRs:
#33142
#33143

[openedx-webhooks]

@kdmccormick Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

[openedx-webhooks]

@kdmccormick Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.