ci cleanup: rustdoc-gui-test now installs browser-ui-test

this removes the need for --unsafe-perm in the Dockerfile.

Author: lolbinarycat

src/build_helper/src/lib.rs

@@ -5,6 +5,7 @@ pub mod drop_bomb;
 pub mod fs;
 pub mod git;
 pub mod metrics;
+pub mod npm;
 pub mod stage0_parser;
 pub mod targets;
 pub mod util;

src/build_helper/src/npm.rs

@@ -0,0 +1,30 @@
+use std::error::Error;
+use std::path::{Path, PathBuf};
+use std::process::Command;
+use std::{fs, io};
+
+/// Install an exact package version, and return the path of `node_modules`.
+pub fn install_one(
+    out_dir: &Path,
+    npm_bin: &Path,
+    pkg_name: &str,
+    pkg_version: &str,
+) -> Result<PathBuf, io::Error> {
+    let nm_path = out_dir.join("node_modules");
+    let _ = fs::create_dir(&nm_path);
+    let mut child = Command::new(npm_bin)
+        .arg("install")
+        .arg("--audit=false")
+        .arg("--fund=false")
+        .arg(format!("{pkg_name}@{pkg_version}"))
+        .current_dir(out_dir)
+        .spawn()?;
+    let exit_status = child.wait()?;
+    if !exit_status.success() {
+        eprintln!("npm install did not exit successfully");
+        return Err(io::Error::other(Box::<dyn Error + Send + Sync>::from(format!(
+            "npm install returned exit code {exit_status}"
+        ))));
+    }
+    Ok(nm_path)
+}

src/ci/docker/host-x86_64/x86_64-gnu-miri/Dockerfile

@@ -46,12 +46,4 @@ ENV HOST_TARGET x86_64-unknown-linux-gnu
 
 COPY scripts/shared.sh /scripts/
 
-# For now, we need to use `--unsafe-perm=true` to go around an issue when npm tries
-# to create a new folder. For reference:
-# https://github.com/puppeteer/puppeteer/issues/375
-#
-# We also specify the version in case we need to update it to go around cache limitations.
-#
-# The `browser-ui-test.version` file is also used by bootstrap to emit warnings in case
-# the local version of the package is different than the one used by the CI.
 ENV SCRIPT /tmp/check-miri.sh ../x.py

src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile

@@ -76,8 +76,6 @@ COPY scripts/nodejs.sh /scripts/
 RUN sh /scripts/nodejs.sh /node
 ENV PATH="/node/bin:${PATH}"
 
-COPY host-x86_64/x86_64-gnu-tools/browser-ui-test.version /tmp/
-
 ENV RUST_CONFIGURE_ARGS \
   --build=x86_64-unknown-linux-gnu \
   --save-toolstates=/tmp/toolstate/toolstates.json \
@@ -91,15 +89,6 @@ ENV HOST_TARGET x86_64-unknown-linux-gnu
 
 COPY scripts/shared.sh /scripts/
 
-# For now, we need to use `--unsafe-perm=true` to go around an issue when npm tries
-# to create a new folder. For reference:
-# https://github.com/puppeteer/puppeteer/issues/375
-#
-# We also specify the version in case we need to update it to go around cache limitations.
-#
-# The `browser-ui-test.version` file is also used by bootstrap to emit warnings in case
-# the local version of the package is different than the one used by the CI.
 ENV SCRIPT /tmp/checktools.sh ../x.py && \
-  npm install browser-ui-test@$(head -n 1 /tmp/browser-ui-test.version) --unsafe-perm=true && \
   python3 ../x.py check compiletest --set build.compiletest-use-stage0-libtest=true && \
   python3 ../x.py test tests/rustdoc-gui --stage 2 --test-args "'--jobs 1'"

src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version

@@ -1,63 +1,15 @@
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::sync::Arc;
-use std::{env, fs};
+use std::env;
 
+use build_helper::npm;
 use build_helper::util::try_run;
 use compiletest::directives::TestProps;
 use config::Config;
 
 mod config;
 
-fn get_browser_ui_test_version_inner(npm: &Path, global: bool) -> Option<String> {
-    let mut command = Command::new(&npm);
-    command.arg("list").arg("--parseable").arg("--long").arg("--depth=0");
-    if global {
-        command.arg("--global");
-    }
-    let lines = match command.output() {
-        Ok(output) => String::from_utf8_lossy(&output.stdout).into_owned(),
-        Err(e) => {
-            eprintln!(
-                "path to npm can be wrong, provided path: {npm:?}. Try to set npm path \
-            in bootstrap.toml in [build.npm]",
-            );
-            panic!("{:?}", e)
-        }
-    };
-    lines
-        .lines()
-        .find_map(|l| l.rsplit(':').next()?.strip_prefix("browser-ui-test@"))
-        .map(|v| v.to_owned())
-}
-
-fn get_browser_ui_test_version(npm: &Path) -> Option<String> {
-    get_browser_ui_test_version_inner(npm, false)
-        .or_else(|| get_browser_ui_test_version_inner(npm, true))
-}
-
-fn compare_browser_ui_test_version(installed_version: &str, src: &Path) {
-    match fs::read_to_string(
-        src.join("src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version"),
-    ) {
-        Ok(v) => {
-            if v.trim() != installed_version {
-                eprintln!(
-                    "⚠️ Installed version of browser-ui-test (`{}`) is different than the \
-                     one used in the CI (`{}`)",
-                    installed_version, v
-                );
-                eprintln!(
-                    "You can install this version using `npm update browser-ui-test` or by using \
-                     `npm install browser-ui-test@{}`",
-                    v,
-                );
-            }
-        }
-        Err(e) => eprintln!("Couldn't find the CI browser-ui-test version: {:?}", e),
-    }
-}
-
 fn find_librs<P: AsRef<Path>>(path: P) -> Option<PathBuf> {
     for entry in walkdir::WalkDir::new(path) {
         let entry = entry.ok()?;
@@ -71,27 +23,6 @@ fn find_librs<P: AsRef<Path>>(path: P) -> Option<PathBuf> {
 fn main() -> Result<(), ()> {
     let config = Arc::new(Config::from_args(env::args().collect()));
 
-    // The goal here is to check if the necessary packages are installed, and if not, we
-    // panic.
-    match get_browser_ui_test_version(&config.npm) {
-        Some(version) => {
-            // We also check the version currently used in CI and emit a warning if it's not the
-            // same one.
-            compare_browser_ui_test_version(&version, &config.rust_src);
-        }
-        None => {
-            eprintln!(
-                r#"
-error: rustdoc-gui test suite cannot be run because npm `browser-ui-test` dependency is missing.
-
-If you want to install the `browser-ui-test` dependency, run `npm install browser-ui-test`
-"#,
-            );
-
-            panic!("Cannot run rustdoc-gui tests");
-        }
-    }
-
     let src_path = config.rust_src.join("tests/rustdoc-gui/src");
     for entry in src_path.read_dir().expect("read_dir call failed") {
         if let Ok(entry) = entry {
@@ -138,16 +69,12 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse
         }
     }
 
-    let mut command = Command::new(&config.nodejs);
+    // FIXME(binarycat): once we get package.json in version control, this should be updated to install via that instead
+    let local_node_modules =
+        npm::install_one(&config.out_dir, &config.npm, "browser-ui-test", "0.21.1")
+            .expect("unable to install browser-ui-test");
 
-    if let Ok(current_dir) = env::current_dir() {
-        let local_node_modules = current_dir.join("node_modules");
-        if local_node_modules.exists() {
-            // Link the local node_modules if exists.
-            // This is useful when we run rustdoc-gui-test from outside of the source root.
-            env::set_var("NODE_PATH", local_node_modules);
-        }
-    }
+    let mut command = Command::new(&config.nodejs);
 
     command
         .arg(config.rust_src.join("src/tools/rustdoc-gui/tester.js"))
@@ -158,6 +85,12 @@ If you want to install the `browser-ui-test` dependency, run `npm install browse
         .arg("--tests-folder")
         .arg(config.rust_src.join("tests/rustdoc-gui"));
 
+    if local_node_modules.exists() {
+        // Link the local node_modules if exists.
+        // This is useful when we run rustdoc-gui-test from outside of the source root.
+        command.env("NODE_PATH", local_node_modules);
+    }
+
     for file in &config.goml_files {
         command.arg("--file").arg(file);
     }