about summary refs log tree commit diff
diff options
context:
space:
mode:
authorLeón Orell Valerian Liehr <me@fmease.dev>2025-07-17 03:58:32 +0200
committerGitHub <noreply@github.com>2025-07-17 03:58:32 +0200
commita07ae6287acb7ab95b7c70372da49a6822ffcefb (patch)
treefc50ca3b5d02412dbaf0de59751f8328b933ec56
parentb9fd2bccfad4fd56538ccc4ae20b62c72748c311 (diff)
parentbbb114ef6198b1d2de4898f838b4ae3183c2db90 (diff)
downloadrust-a07ae6287acb7ab95b7c70372da49a6822ffcefb.tar.gz
rust-a07ae6287acb7ab95b7c70372da49a6822ffcefb.zip
Rollup merge of #143851 - lolbinarycat:bootstrap-node_modules, r=Kobzol
ci cleanup: rustdoc-gui-test now installs browser-ui-test

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

cc ```@GuillaumeGomez``` ```@Kobzol```
-rw-r--r--src/build_helper/src/lib.rs1
-rw-r--r--src/build_helper/src/npm.rs30
-rw-r--r--src/ci/docker/host-x86_64/x86_64-gnu-miri/Dockerfile8
-rw-r--r--src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile11
-rw-r--r--src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version1
-rw-r--r--src/tools/rustdoc-gui-test/src/main.rs93
6 files changed, 44 insertions, 100 deletions
diff --git a/src/build_helper/src/lib.rs b/src/build_helper/src/lib.rs
index 05de8fd2d42..266eedc6245 100644
--- a/src/build_helper/src/lib.rs
+++ b/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;
diff --git a/src/build_helper/src/npm.rs b/src/build_helper/src/npm.rs
new file mode 100644
index 00000000000..dedef40978d
--- /dev/null
+++ b/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)
+}
diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-miri/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-miri/Dockerfile
index b937bc3e678..ad2ee85c7bb 100644
--- a/src/ci/docker/host-x86_64/x86_64-gnu-miri/Dockerfile
+++ b/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
diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile
index e770c58bd9c..95357d22937 100644
--- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile
+++ b/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'"
diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version
deleted file mode 100644
index b9f8e558df4..00000000000
--- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version
+++ /dev/null
@@ -1 +0,0 @@
-0.21.1
\ No newline at end of file
diff --git a/src/tools/rustdoc-gui-test/src/main.rs b/src/tools/rustdoc-gui-test/src/main.rs
index 0e35861fbf7..5b86bea8932 100644
--- a/src/tools/rustdoc-gui-test/src/main.rs
+++ b/src/tools/rustdoc-gui-test/src/main.rs
@@ -1,63 +1,15 @@
+use std::env;
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::sync::Arc;
-use std::{env, fs};
 
+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 {
@@ -134,16 +65,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"))
@@ -154,6 +81,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);
     }