From 5315d8f9124fcfafbc1f619db6ff779ac1395fc9 Mon Sep 17 00:00:00 2001
From: "Kyle D. McCormick" <kyle@axim.org>
Date: Tue, 22 Aug 2023 16:34:10 -0400
Subject: [PATCH 1/3] build: log `npm run postinstall` steps to STDOUT so they
 are visible in CI

---
 scripts/copy-node-modules.sh | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/scripts/copy-node-modules.sh b/scripts/copy-node-modules.sh
index db997da95785..f49514b6def1 100755
--- a/scripts/copy-node-modules.sh
+++ b/scripts/copy-node-modules.sh
@@ -26,28 +26,33 @@ log ( ) {
 	echo -e "${COL_LOG}$* $COL_OFF"
 }
 
+# Print a command (prefixed with '+') and then run it, to aid in debugging.
+# This is just like `set -x`, except that `set -x` prints to STDERR, which is hidden
+# by GitHub Actions logs. This functions prints to STDOUT, which is visible.
+log_and_run ( ) {
+    log "+$*"
+    "$@"  # Joins arguments to form a command (quoting as necessary) and runs the command.
+}
+
 log "====================================================================================="
 log "Copying required assets from node_modules..."
 log "-------------------------------------------------------------------------------"
 
-# Start echoing all commands back to user for ease of debugging.
-set -x
-
 log "Ensuring vendor directories exist..."
-mkdir -p "$vendor_js"
-mkdir -p "$vendor_css"
+log_and_run mkdir -p "$vendor_js"
+log_and_run mkdir -p "$vendor_css"
 
 log "Copying studio-frontend JS & CSS from node_modules into vendor directores..."
 while read -r -d $'\0' src_file ; do
     if [[ "$src_file" = *.css ]] || [[ "$src_file" = *.css.map ]] ; then
-        cp --force "$src_file" "$vendor_css"
+        log_and_run cp --force "$src_file" "$vendor_css"
     else
-        cp --force "$src_file" "$vendor_js"
+        log_and_run cp --force "$src_file" "$vendor_js"
     fi
 done < <(find "$node_modules/@edx/studio-frontend/dist" -type f -print0)
 
 log "Copying certain JS modules from node_modules into vendor directory..."
-cp --force \
+log_and_run cp --force \
     "$node_modules/backbone.paginator/lib/backbone.paginator.js" \
     "$node_modules/backbone/backbone.js" \
     "$node_modules/bootstrap/dist/js/bootstrap.bundle.js" \
@@ -66,8 +71,8 @@ cp --force \
 
 log "Copying certain JS developer modules into vendor directory..."
 if [[ "${NODE_ENV:-production}" = development ]] ; then
-    cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js"
-    cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js"
+    log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js"
+    log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js"
 else
     # TODO: https://github.com/openedx/edx-platform/issues/31768
     # In the old implementation of this scipt (pavelib/assets.py), these two
@@ -77,13 +82,10 @@ else
     # However, in the future, it would be good to only copy them for dev
     # builds. Furthermore, these libraries should not be `npm install`ed
     # into prod builds in the first place.
-    cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true      # "|| true" means "tolerate errors"; in this case,
-    cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true  # that's "tolerate if these files don't exist."
+    log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true      # "|| true" means "tolerate errors"; in this case,
+    log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true  # that's "tolerate if these files don't exist."
 fi
 
-# Done echoing.
-set +x
-
 log "-------------------------------------------------------------------------------"
 log " Done copying required assets from node_modules."
 log "====================================================================================="

From e6b40b12749d80e467859faa87ff9fb933347fa0 Mon Sep 17 00:00:00 2001
From: "Kyle D. McCormick" <kyle@axim.org>
Date: Wed, 19 Jul 2023 15:08:17 -0400
Subject: [PATCH 2/3] 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.

More details: https://github.com/openedx/edx-platform/pull/32835
Part of: https://github.com/openedx/wg-developer-experience/issues/166
---
 .../css/vendor/images/treeview-default-line.gif     | Bin
 .../static/css/vendor/images/treeview-default.gif   | Bin
 {lms => common}/static/css/vendor/indicator.gif     | Bin
 .../static/css/vendor/jquery.autocomplete.css       |   0
 .../static/css/vendor/jquery.treeview.css           |   0
 5 files changed, 0 insertions(+), 0 deletions(-)
 rename {lms => common}/static/css/vendor/images/treeview-default-line.gif (100%)
 rename {lms => common}/static/css/vendor/images/treeview-default.gif (100%)
 rename {lms => common}/static/css/vendor/indicator.gif (100%)
 rename {lms => common}/static/css/vendor/jquery.autocomplete.css (100%)
 rename {lms => common}/static/css/vendor/jquery.treeview.css (100%)

diff --git a/lms/static/css/vendor/images/treeview-default-line.gif b/common/static/css/vendor/images/treeview-default-line.gif
similarity index 100%
rename from lms/static/css/vendor/images/treeview-default-line.gif
rename to common/static/css/vendor/images/treeview-default-line.gif
diff --git a/lms/static/css/vendor/images/treeview-default.gif b/common/static/css/vendor/images/treeview-default.gif
similarity index 100%
rename from lms/static/css/vendor/images/treeview-default.gif
rename to common/static/css/vendor/images/treeview-default.gif
diff --git a/lms/static/css/vendor/indicator.gif b/common/static/css/vendor/indicator.gif
similarity index 100%
rename from lms/static/css/vendor/indicator.gif
rename to common/static/css/vendor/indicator.gif
diff --git a/lms/static/css/vendor/jquery.autocomplete.css b/common/static/css/vendor/jquery.autocomplete.css
similarity index 100%
rename from lms/static/css/vendor/jquery.autocomplete.css
rename to common/static/css/vendor/jquery.autocomplete.css
diff --git a/lms/static/css/vendor/jquery.treeview.css b/common/static/css/vendor/jquery.treeview.css
similarity index 100%
rename from lms/static/css/vendor/jquery.treeview.css
rename to common/static/css/vendor/jquery.treeview.css

From 8ec506c66e6c27ce9967fc2054d31f53cfa60d50 Mon Sep 17 00:00:00 2001
From: "Kyle D. McCormick" <kyle@axim.org>
Date: Thu, 24 Aug 2023 16:19:21 -0400
Subject: [PATCH 3/3] build: 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.)

More details: https://github.com/openedx/edx-platform/pull/32835
Part of: https://github.com/openedx/wg-developer-experience/issues/166
---
 .eslintignore                             |  3 +-
 common/static/.gitignore                  |  1 +
 common/static/common/css/vendor           |  1 +
 common/static/common/js/vendor            |  1 +
 pavelib/prereqs.py                        |  8 +----
 scripts/copy-node-modules.sh              | 40 ++++++++++++-----------
 scripts/xsslint/xsslint/default_config.py |  2 +-
 scripts/xsslint_config.py                 |  2 +-
 8 files changed, 29 insertions(+), 29 deletions(-)
 create mode 120000 common/static/common/css/vendor
 create mode 120000 common/static/common/js/vendor

diff --git a/.eslintignore b/.eslintignore
index 9044a0cc711f..989d401eedc9 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -1,5 +1,6 @@
-# Vendor files and generated test artifacts
+# Vendor files, copies of node_modules, and generated test artifacts
 **/vendor
+**/node_copies
 test_root/staticfiles
 
 
diff --git a/common/static/.gitignore b/common/static/.gitignore
index f2c422d5b03f..94771a9b5668 100644
--- a/common/static/.gitignore
+++ b/common/static/.gitignore
@@ -1 +1,2 @@
 xmodule
+/node_copies
diff --git a/common/static/common/css/vendor b/common/static/common/css/vendor
new file mode 120000
index 000000000000..a21c7e8e19f8
--- /dev/null
+++ b/common/static/common/css/vendor
@@ -0,0 +1 @@
+../../node_copies/css
\ No newline at end of file
diff --git a/common/static/common/js/vendor b/common/static/common/js/vendor
new file mode 120000
index 000000000000..b035c2f0004f
--- /dev/null
+++ b/common/static/common/js/vendor
@@ -0,0 +1 @@
+../../node_copies/js
\ No newline at end of file
diff --git a/pavelib/prereqs.py b/pavelib/prereqs.py
index a1b14c54455d..914e0311095a 100644
--- a/pavelib/prereqs.py
+++ b/pavelib/prereqs.py
@@ -138,13 +138,7 @@ def node_prereqs_installation():
     #
     # This hack should probably be left in place for at least a year.
     # See ADR 17 for more background on the transition.
-    sh("rm -rf common/static/common/js/vendor/ common/static/common/css/vendor/")
-    # At the time of this writing, the js dir has git-versioned files
-    # but the css dir does not, so the latter would have been created
-    # as root-owned (in the process of creating the vendor
-    # subdirectory). Delete it only if empty, just in case
-    # git-versioned files are added later.
-    sh("rmdir common/static/common/css || true")
+    sh("rm -rf common/static/node_copies")
 
     # NPM installs hang sporadically. Log the installation process so that we
     # determine if any packages are chronic offenders.
diff --git a/scripts/copy-node-modules.sh b/scripts/copy-node-modules.sh
index f49514b6def1..4d615045a36b 100755
--- a/scripts/copy-node-modules.sh
+++ b/scripts/copy-node-modules.sh
@@ -2,10 +2,12 @@
 # Copy certain npm-installed assets from node_modules to other folders in
 # edx-platform. These assets are used by certain especially-old legacy LMS & CMS
 # frontends that are not set up to import from node_modules directly.
-# Many of the destination folders are named "vendor", because they originally
+
+# App code refers to these copied modules via symlinks named "vendor" because they originally
 # held vendored-in (directly-committed) libraries; once we moved most frontends
-# to use NPM, we decided to keep library versions in-sync by copying to the
-# former "vendor" directories.
+# to use NPM, we decided to keep library versions in-sync by copying into the former "vendor" directories.
+# Eventually, we  but established "vendor" symlinks because
+# updaing all referecnes would have been too difficult.
 
 # Enable stricter error handling.
 set -euo pipefail
@@ -18,8 +20,8 @@ COL_OFF="\e[0m"   # Normal color
 # https://github.com/openedx/wg-developer-experience/issues/150
 # https://github.com/openedx/wg-developer-experience/issues/151
 node_modules="node_modules"
-vendor_js="common/static/common/js/vendor"
-vendor_css="common/static/common/css/vendor"
+target_js="common/static/node_copies/js"
+target_css="common/static/node_copies/css"
 
 # Stylized logging.
 log ( ) {
@@ -38,20 +40,20 @@ log "===========================================================================
 log "Copying required assets from node_modules..."
 log "-------------------------------------------------------------------------------"
 
-log "Ensuring vendor directories exist..."
-log_and_run mkdir -p "$vendor_js"
-log_and_run mkdir -p "$vendor_css"
+log "Ensuring target directories exist..."
+log_and_run mkdir -p "$target_js"
+log_and_run mkdir -p "$target_css"
 
-log "Copying studio-frontend JS & CSS from node_modules into vendor directores..."
+log "Copying studio-frontend JS & CSS from node_modules into target directores..."
 while read -r -d $'\0' src_file ; do
     if [[ "$src_file" = *.css ]] || [[ "$src_file" = *.css.map ]] ; then
-        log_and_run cp --force "$src_file" "$vendor_css"
+        log_and_run cp --force "$src_file" "$target_css"
     else
-        log_and_run cp --force "$src_file" "$vendor_js"
+        log_and_run cp --force "$src_file" "$target_js"
     fi
 done < <(find "$node_modules/@edx/studio-frontend/dist" -type f -print0)
 
-log "Copying certain JS modules from node_modules into vendor directory..."
+log "Copying certain JS modules from node_modules into target directory..."
 log_and_run cp --force \
     "$node_modules/backbone.paginator/lib/backbone.paginator.js" \
     "$node_modules/backbone/backbone.js" \
@@ -67,23 +69,23 @@ log_and_run cp --force \
     "$node_modules/underscore.string/dist/underscore.string.js" \
     "$node_modules/underscore/underscore.js" \
     "$node_modules/which-country/index.js" \
-    "$vendor_js"
+    "$target_js"
 
-log "Copying certain JS developer modules into vendor directory..."
+log "Copying certain JS developer modules into target directory..."
 if [[ "${NODE_ENV:-production}" = development ]] ; then
-    log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js"
-    log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js"
+    log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$target_js"
+    log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$target_js"
 else
     # TODO: https://github.com/openedx/edx-platform/issues/31768
     # In the old implementation of this scipt (pavelib/assets.py), these two
-    # developer libraries were copied into the JS vendor directory whether not
+    # developer libraries were copied into the JS target directory whether not
     # the build was for prod or dev. In order to exactly match the output of
     # the old script, this script will also copy them in for prod builds.
     # However, in the future, it would be good to only copy them for dev
     # builds. Furthermore, these libraries should not be `npm install`ed
     # into prod builds in the first place.
-    log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$vendor_js" || true      # "|| true" means "tolerate errors"; in this case,
-    log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$vendor_js" || true  # that's "tolerate if these files don't exist."
+    log_and_run cp --force "$node_modules/sinon/pkg/sinon.js" "$target_js" || true      # "|| true" means "tolerate errors"; in this case,
+    log_and_run cp --force "$node_modules/squirejs/src/Squire.js" "$target_js" || true  # that's "tolerate if these files don't exist."
 fi
 
 log "-------------------------------------------------------------------------------"
diff --git a/scripts/xsslint/xsslint/default_config.py b/scripts/xsslint/xsslint/default_config.py
index e5075785e157..47667ad08589 100644
--- a/scripts/xsslint/xsslint/default_config.py
+++ b/scripts/xsslint/xsslint/default_config.py
@@ -11,10 +11,10 @@
 SKIP_DIRS = (
     '.git',
     '.pycharm_helpers',
-    'common/static/xmodule/modules',
     'common/static/bundles',
     'perf_tests',
     'node_modules',
+    'node_copies',
     'reports/diff_quality',
     'scripts/xsslint',
     'spec',
diff --git a/scripts/xsslint_config.py b/scripts/xsslint_config.py
index 2f463a49dffb..e0f80836a116 100644
--- a/scripts/xsslint_config.py
+++ b/scripts/xsslint_config.py
@@ -20,11 +20,11 @@
 SKIP_DIRS = (
     '.git',
     '.pycharm_helpers',
-    'common/static/xmodule/modules',
     'common/static/bundles',
     'docs',
     'perf_tests',
     'node_modules',
+    'node_copies',
     'reports/diff_quality',
     'scripts/xsslint',
     'spec',