Rollup merge of #127108 - onur-ozkan:bin-helper, r=Kobzol

unify `dylib` and `bin_helpers` and create `shared_helpers::parse_value_from_args`

`dylib` and `bin_helpers` were already used in similar logic. This PR unifies them under a `shared_helpers` module that is utilized by both the bootstrap library and shims. Additionally, created `parse_value_from_args` with a unit test.  This helps avoid code duplication in shims and can also be used in the bootstrap library if needed (which is the case in one of `@Kobzol's` tasks).

r? `@Kobzol`

src/bootstrap/src/bin/rustc.rs

@@ -20,26 +20,24 @@
 use std::process::{Child, Command};
 use std::time::Instant;
 
-use dylib_util::{dylib_path, dylib_path_var, exe};
+use shared_helpers::{
+    dylib_path, dylib_path_var, exe, maybe_dump, parse_rustc_stage, parse_rustc_verbose,
+    parse_value_from_args,
+};
 
-#[path = "../utils/bin_helpers.rs"]
-mod bin_helpers;
-
-#[path = "../utils/dylib.rs"]
-mod dylib_util;
+#[path = "../utils/shared_helpers.rs"]
+mod shared_helpers;
 
 fn main() {
     let orig_args = env::args_os().skip(1).collect::<Vec<_>>();
     let mut args = orig_args.clone();
-    let arg =
-        |name| orig_args.windows(2).find(|args| args[0] == name).and_then(|args| args[1].to_str());
 
-    let stage = bin_helpers::parse_rustc_stage();
-    let verbose = bin_helpers::parse_rustc_verbose();
+    let stage = parse_rustc_stage();
+    let verbose = parse_rustc_verbose();
 
     // Detect whether or not we're a build script depending on whether --target
     // is passed (a bit janky...)
-    let target = arg("--target");
+    let target = parse_value_from_args(&orig_args, "--target");
     let version = args.iter().find(|w| &**w == "-vV");
 
     // Use a different compiler for build scripts, since there may not yet be a
@@ -102,7 +100,7 @@ fn main() {
     cmd.args(&args).env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
 
     // Get the name of the crate we're compiling, if any.
-    let crate_name = arg("--crate-name");
+    let crate_name = parse_value_from_args(&orig_args, "--crate-name");
 
     if let Some(crate_name) = crate_name {
         if let Some(target) = env::var_os("RUSTC_TIME") {
@@ -143,10 +141,11 @@ fn main() {
             cmd.arg("-C").arg("panic=abort");
         }
 
+        let crate_type = parse_value_from_args(&orig_args, "--crate-type");
         // `-Ztls-model=initial-exec` must not be applied to proc-macros, see
         // issue https://github.com/rust-lang/rust/issues/100530
         if env::var("RUSTC_TLS_MODEL_INITIAL_EXEC").is_ok()
-            && arg("--crate-type") != Some("proc-macro")
+            && crate_type != Some("proc-macro")
             && !matches!(crate_name, Some("proc_macro2" | "quote" | "syn" | "synstructure"))
         {
             cmd.arg("-Ztls-model=initial-exec");
@@ -251,7 +250,7 @@ fn main() {
         eprintln!("{prefix} libdir: {libdir:?}");
     }
 
-    bin_helpers::maybe_dump(format!("stage{stage}-rustc"), &cmd);
+    maybe_dump(format!("stage{stage}-rustc"), &cmd);
 
     let start = Instant::now();
     let (child, status) = {

src/bootstrap/src/bin/rustdoc.rs

@@ -6,19 +6,19 @@
 use std::path::PathBuf;
 use std::process::Command;
 
-use dylib_util::{dylib_path, dylib_path_var};
+use shared_helpers::{
+    dylib_path, dylib_path_var, maybe_dump, parse_rustc_stage, parse_rustc_verbose,
+    parse_value_from_args,
+};
 
-#[path = "../utils/bin_helpers.rs"]
-mod bin_helpers;
-
-#[path = "../utils/dylib.rs"]
-mod dylib_util;
+#[path = "../utils/shared_helpers.rs"]
+mod shared_helpers;
 
 fn main() {
     let args = env::args_os().skip(1).collect::<Vec<_>>();
 
-    let stage = bin_helpers::parse_rustc_stage();
-    let verbose = bin_helpers::parse_rustc_verbose();
+    let stage = parse_rustc_stage();
+    let verbose = parse_rustc_verbose();
 
     let rustdoc = env::var_os("RUSTDOC_REAL").expect("RUSTDOC_REAL was not set");
     let libdir = env::var_os("RUSTDOC_LIBDIR").expect("RUSTDOC_LIBDIR was not set");
@@ -26,7 +26,7 @@ fn main() {
 
     // Detect whether or not we're a build script depending on whether --target
     // is passed (a bit janky...)
-    let target = args.windows(2).find(|w| &*w[0] == "--target").and_then(|w| w[1].to_str());
+    let target = parse_value_from_args(&args, "--target");
 
     let mut dylib_path = dylib_path();
     dylib_path.insert(0, PathBuf::from(libdir.clone()));
@@ -62,7 +62,7 @@ fn main() {
     cmd.arg("-Zunstable-options");
     cmd.arg("--check-cfg=cfg(bootstrap)");
 
-    bin_helpers::maybe_dump(format!("stage{stage}-rustdoc"), &cmd);
+    maybe_dump(format!("stage{stage}-rustdoc"), &cmd);
 
     if verbose > 1 {
         eprintln!(

src/bootstrap/src/utils/bin_helpers.rs

@@ -1,50 +0,0 @@
-//! This file is meant to be included directly from bootstrap shims to avoid a
-//! dependency on the bootstrap library. This reduces the binary size and
-//! improves compilation time by reducing the linking time.
-
-use std::env;
-use std::fs::OpenOptions;
-use std::io::Write;
-use std::process::Command;
-use std::str::FromStr;
-
-/// Parses the value of the "RUSTC_VERBOSE" environment variable and returns it as a `usize`.
-/// If it was not defined, returns 0 by default.
-///
-/// Panics if "RUSTC_VERBOSE" is defined with the value that is not an unsigned integer.
-pub(crate) fn parse_rustc_verbose() -> usize {
-    match env::var("RUSTC_VERBOSE") {
-        Ok(s) => usize::from_str(&s).expect("RUSTC_VERBOSE should be an integer"),
-        Err(_) => 0,
-    }
-}
-
-/// Parses the value of the "RUSTC_STAGE" environment variable and returns it as a `String`.
-///
-/// If "RUSTC_STAGE" was not set, the program will be terminated with 101.
-pub(crate) fn parse_rustc_stage() -> String {
-    env::var("RUSTC_STAGE").unwrap_or_else(|_| {
-        // Don't panic here; it's reasonable to try and run these shims directly. Give a helpful error instead.
-        eprintln!("rustc shim: FATAL: RUSTC_STAGE was not set");
-        eprintln!("rustc shim: NOTE: use `x.py build -vvv` to see all environment variables set by bootstrap");
-        std::process::exit(101);
-    })
-}
-
-/// Writes the command invocation to a file if `DUMP_BOOTSTRAP_SHIMS` is set during bootstrap.
-///
-/// Before writing it, replaces user-specific values to create generic dumps for cross-environment
-/// comparisons.
-pub(crate) fn maybe_dump(dump_name: String, cmd: &Command) {
-    if let Ok(dump_dir) = env::var("DUMP_BOOTSTRAP_SHIMS") {
-        let dump_file = format!("{dump_dir}/{dump_name}");
-
-        let mut file = OpenOptions::new().create(true).append(true).open(dump_file).unwrap();
-
-        let cmd_dump = format!("{:?}\n", cmd);
-        let cmd_dump = cmd_dump.replace(&env::var("BUILD_OUT").unwrap(), "${BUILD_OUT}");
-        let cmd_dump = cmd_dump.replace(&env::var("CARGO_HOME").unwrap(), "${CARGO_HOME}");
-
-        file.write_all(cmd_dump.as_bytes()).expect("Unable to write file");
-    }
-}

src/bootstrap/src/utils/dylib.rs

@@ -1,40 +0,0 @@
-//! Various utilities for working with dylib paths.
-
-/// Returns the environment variable which the dynamic library lookup path
-/// resides in for this platform.
-pub fn dylib_path_var() -> &'static str {
-    if cfg!(target_os = "windows") {
-        "PATH"
-    } else if cfg!(target_vendor = "apple") {
-        "DYLD_LIBRARY_PATH"
-    } else if cfg!(target_os = "haiku") {
-        "LIBRARY_PATH"
-    } else if cfg!(target_os = "aix") {
-        "LIBPATH"
-    } else {
-        "LD_LIBRARY_PATH"
-    }
-}
-
-/// Parses the `dylib_path_var()` environment variable, returning a list of
-/// paths that are members of this lookup path.
-pub fn dylib_path() -> Vec<std::path::PathBuf> {
-    let var = match std::env::var_os(dylib_path_var()) {
-        Some(v) => v,
-        None => return vec![],
-    };
-    std::env::split_paths(&var).collect()
-}
-
-/// Given an executable called `name`, return the filename for the
-/// executable for a particular target.
-#[allow(dead_code)]
-pub fn exe(name: &str, target: &str) -> String {
-    if target.contains("windows") {
-        format!("{name}.exe")
-    } else if target.contains("uefi") {
-        format!("{name}.efi")
-    } else {
-        name.to_string()
-    }
-}

src/bootstrap/src/utils/helpers.rs

@@ -18,7 +18,7 @@
 use crate::core::config::{Config, TargetSelection};
 use crate::LldMode;
 
-pub use crate::utils::dylib::{dylib_path, dylib_path_var};
+pub use crate::utils::shared_helpers::{dylib_path, dylib_path_var};
 
 #[cfg(test)]
 mod tests;
@@ -51,7 +51,7 @@ macro_rules! t {
 pub use t;
 
 pub fn exe(name: &str, target: TargetSelection) -> String {
-    crate::utils::dylib::exe(name, &target.triple)
+    crate::utils::shared_helpers::exe(name, &target.triple)
 }
 
 /// Returns `true` if the file name given looks like a dynamic library.

src/bootstrap/src/utils/mod.rs

@@ -6,11 +6,11 @@
 pub(crate) mod cc_detect;
 pub(crate) mod change_tracker;
 pub(crate) mod channel;
-pub(crate) mod dylib;
 pub(crate) mod exec;
 pub(crate) mod helpers;
 pub(crate) mod job;
 #[cfg(feature = "build-metrics")]
 pub(crate) mod metrics;
 pub(crate) mod render_tests;
+pub(crate) mod shared_helpers;
 pub(crate) mod tarball;

src/bootstrap/src/utils/shared_helpers.rs

@@ -0,0 +1,112 @@
+//! This module serves two purposes:
+//!     1. It is part of the `utils` module and used in other parts of bootstrap.
+//!     2. It is embedded inside bootstrap shims to avoid a dependency on the bootstrap library.
+//!        Therefore, this module should never use any other bootstrap module. This reduces binary
+//!        size and improves compilation time by minimizing linking time.
+
+#![allow(dead_code)]
+
+use std::env;
+use std::ffi::OsString;
+use std::fs::OpenOptions;
+use std::io::Write;
+use std::process::Command;
+use std::str::FromStr;
+
+#[cfg(test)]
+mod tests;
+
+/// Returns the environment variable which the dynamic library lookup path
+/// resides in for this platform.
+pub fn dylib_path_var() -> &'static str {
+    if cfg!(target_os = "windows") {
+        "PATH"
+    } else if cfg!(target_vendor = "apple") {
+        "DYLD_LIBRARY_PATH"
+    } else if cfg!(target_os = "haiku") {
+        "LIBRARY_PATH"
+    } else if cfg!(target_os = "aix") {
+        "LIBPATH"
+    } else {
+        "LD_LIBRARY_PATH"
+    }
+}
+
+/// Parses the `dylib_path_var()` environment variable, returning a list of
+/// paths that are members of this lookup path.
+pub fn dylib_path() -> Vec<std::path::PathBuf> {
+    let var = match std::env::var_os(dylib_path_var()) {
+        Some(v) => v,
+        None => return vec![],
+    };
+    std::env::split_paths(&var).collect()
+}
+
+/// Given an executable called `name`, return the filename for the
+/// executable for a particular target.
+pub fn exe(name: &str, target: &str) -> String {
+    if target.contains("windows") {
+        format!("{name}.exe")
+    } else if target.contains("uefi") {
+        format!("{name}.efi")
+    } else {
+        name.to_string()
+    }
+}
+
+/// Parses the value of the "RUSTC_VERBOSE" environment variable and returns it as a `usize`.
+/// If it was not defined, returns 0 by default.
+///
+/// Panics if "RUSTC_VERBOSE" is defined with the value that is not an unsigned integer.
+pub fn parse_rustc_verbose() -> usize {
+    match env::var("RUSTC_VERBOSE") {
+        Ok(s) => usize::from_str(&s).expect("RUSTC_VERBOSE should be an integer"),
+        Err(_) => 0,
+    }
+}
+
+/// Parses the value of the "RUSTC_STAGE" environment variable and returns it as a `String`.
+///
+/// If "RUSTC_STAGE" was not set, the program will be terminated with 101.
+pub fn parse_rustc_stage() -> String {
+    env::var("RUSTC_STAGE").unwrap_or_else(|_| {
+        // Don't panic here; it's reasonable to try and run these shims directly. Give a helpful error instead.
+        eprintln!("rustc shim: FATAL: RUSTC_STAGE was not set");
+        eprintln!("rustc shim: NOTE: use `x.py build -vvv` to see all environment variables set by bootstrap");
+        std::process::exit(101);
+    })
+}
+
+/// Writes the command invocation to a file if `DUMP_BOOTSTRAP_SHIMS` is set during bootstrap.
+///
+/// Before writing it, replaces user-specific values to create generic dumps for cross-environment
+/// comparisons.
+pub fn maybe_dump(dump_name: String, cmd: &Command) {
+    if let Ok(dump_dir) = env::var("DUMP_BOOTSTRAP_SHIMS") {
+        let dump_file = format!("{dump_dir}/{dump_name}");
+
+        let mut file = OpenOptions::new().create(true).append(true).open(dump_file).unwrap();
+
+        let cmd_dump = format!("{:?}\n", cmd);
+        let cmd_dump = cmd_dump.replace(&env::var("BUILD_OUT").unwrap(), "${BUILD_OUT}");
+        let cmd_dump = cmd_dump.replace(&env::var("CARGO_HOME").unwrap(), "${CARGO_HOME}");
+
+        file.write_all(cmd_dump.as_bytes()).expect("Unable to write file");
+    }
+}
+
+/// Finds `key` and returns its value from the given list of arguments `args`.
+pub fn parse_value_from_args<'a>(args: &'a [OsString], key: &str) -> Option<&'a str> {
+    let mut args = args.iter();
+    while let Some(arg) = args.next() {
+        let arg = arg.to_str().unwrap();
+
+        if let Some(value) = arg.strip_prefix(&format!("{key}=")) {
+            return Some(value);
+        } else if arg == key {
+            return args.next().map(|v| v.to_str().unwrap());
+        }
+    }
+
+    None
+}

src/bootstrap/src/utils/shared_helpers/tests.rs

@@ -0,0 +1,28 @@
+use super::parse_value_from_args;
+
+#[test]
+fn test_parse_value_from_args() {
+    let args = vec![
+        "--stage".into(),
+        "1".into(),
+        "--version".into(),
+        "2".into(),
+        "--target".into(),
+        "x86_64-unknown-linux".into(),
+    ];
+
+    assert_eq!(parse_value_from_args(args.as_slice(), "--stage").unwrap(), "1");
+    assert_eq!(parse_value_from_args(args.as_slice(), "--version").unwrap(), "2");
+    assert_eq!(parse_value_from_args(args.as_slice(), "--target").unwrap(), "x86_64-unknown-linux");
+    assert!(parse_value_from_args(args.as_slice(), "random-key").is_none());
+
+    let args = vec![
+        "app-name".into(),
+        "--key".into(),
+        "value".into(),
+        "random-value".into(),
+        "--sysroot=/x/y/z".into(),
+    ];
+    assert_eq!(parse_value_from_args(args.as_slice(), "--key").unwrap(), "value");
+    assert_eq!(parse_value_from_args(args.as_slice(), "--sysroot").unwrap(), "/x/y/z");
+}