update

Author: tgross35

library/compiler-builtins/crates/josh-sync/src/main.rs

@@ -36,8 +36,11 @@ fn main() -> anyhow::Result<()> {
                 }
             }
         }
-        ["rustc-push", branch, github_username] => {
-            sync.rustc_push(github_username, branch)?;
+        ["rustc-push", github_user, branch] => {
+            sync.rustc_push(github_user, Some(branch))?;
+        }
+        ["rustc-push", github_user] => {
+            sync.rustc_push(github_user, None)?;
         }
         _ => {
             println!("{USAGE}");

library/compiler-builtins/crates/josh-sync/src/sync.rs

@@ -1,7 +1,7 @@
 use std::io::{Write, stdout};
 use std::ops::Not;
 use std::path::PathBuf;
-use std::process::{Command, Stdio};
+use std::process::{Command, Stdio, exit};
 use std::time::Duration;
 use std::{env, fs, net, process};
 
@@ -10,6 +10,7 @@ use xshell::{Shell, cmd};
 
 const PREPARING_COMMIT_MESSAGE: &str = "Preparing for merge from rustc";
 const MERGE_COMMIT_MESSAGE: &str = "Merge from rustc";
+const DEFAULT_PR_BRANCH: &str = "update-builtins";
 
 pub struct GitSync {
     dir: PathBuf,
@@ -53,9 +54,7 @@ impl GitSync {
         sh.change_dir(&self.dir);
 
         let upstream_head = commit.unwrap_or_else(|| {
-            let out = cmd!(sh, "git ls-remote {upstream_url} {upstream_ref}")
-                .read()
-                .unwrap();
+            let out = self.read(&["git", "ls-remote", upstream_url, upstream_ref]);
             out.split_whitespace()
                 .next()
                 .unwrap_or_else(|| panic!("could not split output: '{out}'"))
@@ -72,24 +71,25 @@ impl GitSync {
         let previous_base_commit = sh.read_file("rust-version").unwrap().trim().to_string();
         assert_ne!(previous_base_commit, upstream_head, "nothing to pull");
 
-        let orig_head = cmd!(sh, "git rev-parse HEAD").read().unwrap();
+        let orig_head = self.read(["git", "rev-parse", "HEAD"]);
 
         // Update rust-version file. As a separate commit, since making it part of
         // the merge has confused the heck out of josh in the past.
         // We pass `--no-verify` to avoid running git hooks.
         // We do this before the merge so that if there are merge conflicts, we have
         // the right rust-version file while resolving them.
         sh.write_file("rust-version", format!("{upstream_head}\n"))?;
-        cmd!(
-            sh,
-            "git commit rust-version --no-verify -m {PREPARING_COMMIT_MESSAGE}"
-        )
-        .run()?;
+        self.run([
+            "git",
+            "commit",
+            "rust-version",
+            "--no-verify",
+            "-m",
+            PREPARING_COMMIT_MESSAGE,
+        ]);
 
         // Fetch given rustc commit.
-        cmd!(sh, "git fetch {josh_url}")
-            .run()
-            .expect("FAILED to fetch new commits, something went wrong");
+        self.run(["git", "fetch", &josh_url]);
 
         // // Fetch given rustc commit.
         // if let Err(e) = cmd!(sh, "git fetch {josh_url}").run() {
@@ -106,139 +106,134 @@ impl GitSync {
 
         // This should not add any new root commits. So count those before and after merging.
         let num_roots = || -> u32 {
-            let out = cmd!(sh, "git rev-list HEAD --max-parents=0 --count")
-                .read()
-                .expect("failed to determine the number of root commits");
+            let out = self.read(["git", "rev-list", "HEAD", "--max-parents=0", "--count"]);
             out.parse::<u32>()
-                .unwrap_or_else(|e| panic!("failed to parse `out`: {e}"))
+                .unwrap_or_else(|e| panic!("failed to parse `{out}`: {e}"))
         };
         let num_roots_before = num_roots();
 
-        let sha = cmd!(sh, "git rev-parse HEAD")
-            .output()
-            .expect("FAILED to get current commit")
-            .stdout;
+        let pre_merge_sha = self.read(["git", "rev-parse", "HEAD"]);
 
         // Merge the fetched commit.
-        cmd!(
-            sh,
-            "git merge FETCH_HEAD --no-verify --no-ff -m {MERGE_COMMIT_MESSAGE}"
-        )
-        .run()
-        .expect("FAILED to merge new commits, something went wrong");
-
-        let current_sha = cmd!(sh, "git rev-parse HEAD")
-            .output()
-            .context("FAILED to get current commit")?
-            .stdout;
-        if current_sha == sha {
-            cmd!(sh, "git reset --hard {orig_head}")
-                .run()
-                .expect("FAILED to clean up after creating the preparation commit");
+        self.run([
+            "git",
+            "merge",
+            "FETCH_HEAD",
+            "--no-verify",
+            "--no-ff",
+            "-m",
+            MERGE_COMMIT_MESSAGE,
+        ]);
+
+        let current_sha = self.read(["git", "rev-parse", "HEAD"]);
+        if current_sha == pre_merge_sha {
+            self.run(["git", "reset", "--hard", &orig_head]);
             eprintln!(
                 "No merge was performed, no changes to pull were found. \
                 Rolled back the preparation commit."
             );
-            return Err(RustcPullError::NothingToPull);
+            exit(1);
         }
 
         // Check that the number of roots did not increase.
-        if num_roots() != num_roots_before {
-            return Err(anyhow::anyhow!(
-                "Josh created a new root commit. This is probably not the history you want."
-            )
-            .into());
-        }
+        assert_eq!(
+            num_roots(),
+            num_roots_before,
+            "Josh created a new root commit. This is probably not the history you want."
+        );
 
         Ok(())
     }
 
-    pub fn rustc_push(&self, github_user: &str, branch: &str) -> anyhow::Result<()> {
-        let sh = Shell::new()?;
+    pub fn rustc_push(&self, github_user: &str, branch: Option<&str>) -> anyhow::Result<()> {
         let Self {
             upstream_repo,
             josh_filter,
             josh_url_base,
+            upstream_url,
             ..
         } = self;
+
+        let sh = Shell::new()?;
         sh.change_dir(&self.dir);
-        let base = sh.read_file("rust-version")?.trim().to_owned();
+
+        let branch = branch.unwrap_or(DEFAULT_PR_BRANCH);
+        let josh_url = format!("{josh_url_base}/{github_user}/rust.git{josh_filter}.git");
+        let user_rust_url = format!("git@github.com:{github_user}/rust.git");
+
+        let Ok(rustc_git) = env::var("RUSTC_GIT") else {
+            panic!("the RUSTC_GIT environment variable must be set to a rust-lang/rust checkout")
+        };
+
         ensure_clean(&sh)?;
+        let base = sh.read_file("rust-version")?.trim().to_owned();
 
         // Make sure josh is running.
         let josh = self.start_josh()?;
-        let josh_url = format!("{josh_url_base}/{github_user}/rust.git{josh_filter}.git");
 
-        // Find a repo we can do our preparation in.
-        if let Ok(rustc_git) = env::var("RUSTC_GIT") {
-            // If rustc_git is `Some`, we'll use an existing fork for the branch updates.
-            sh.change_dir(rustc_git);
-        } else {
-            // Otherwise, do this in the local repo.
-            println!(
-                "This will pull a copy of the rust-lang/rust history into this checkout, growing it by about 1GB."
-            );
-            print!(
-                "To avoid that, abort now and set the `RUSTC_GIT` environment variable to an existing rustc checkout. Proceed? [y/N] "
-            );
-            std::io::stdout().flush()?;
-            let mut answer = String::new();
-            std::io::stdin().read_line(&mut answer)?;
-            if answer.trim().to_lowercase() != "y" {
-                std::process::exit(1);
-            }
-        };
         // Prepare the branch. Pushing works much better if we use as base exactly
         // the commit that we pulled from last time, so we use the `rust-version`
         // file to find out which commit that would be.
         println!("Preparing {github_user}/rust (base: {base})...");
-        if cmd!(
-            sh,
-            "git fetch https://github.com/{github_user}/rust {branch}"
-        )
-        .ignore_stderr()
-        .read()
-        .is_ok()
+
+        if Command::new("git")
+            .args(["-C", &rustc_git, "fetch", &user_rust_url])
+            .output()
+            .expect("could not run fetch")
+            .status
+            .success()
         {
-            println!(
-                "The branch '{branch}' seems to already exist in 'https://github.com/{github_user}/rust'. Please delete it and try again."
+            panic!(
+                "The branch '{branch}' seems to already exist in '{user_rust_url}'. \
+                 Please delete it and try again."
             );
-            std::process::exit(1);
         }
-        cmd!(sh, "git fetch https://github.com/{upstream_repo} {base}").run()?;
-        cmd!(
-            sh,
-            "git push https://github.com/{github_user}/rust {base}:refs/heads/{branch}"
-        )
-        .ignore_stdout()
-        .ignore_stderr() // silence the "create GitHub PR" message
-        .run()?;
-        println!();
+
+        self.run([
+            "git",
+            "-C",
+            &rustc_git,
+            "fetch",
+            &format!("https://github.com/{upstream_repo}"),
+            &base,
+        ]);
+
+        self.run_args("git", |c| {
+            c.args([
+                "-C",
+                &rustc_git,
+                "push",
+                &format!("https://github.com/{github_user}/rust"),
+                &format!("{base}:refs/heads/{branch}"),
+            ])
+            .stdout(Stdio::null())
+            .stderr(Stdio::null()) // silence the "create GitHub PR" message
+        });
+        println!("pushed PR branch");
 
         // Do the actual push.
         sh.change_dir(&self.dir);
         println!("Pushing changes...");
-        cmd!(sh, "git push {josh_url} HEAD:{branch}").run()?;
+        self.run(["git", "push", &josh_url, &format!("HEAD:{branch}")]);
         println!();
 
         // Do a round-trip check to make sure the push worked as expected.
-        cmd!(sh, "git fetch {josh_url} {branch}")
-            .ignore_stderr()
-            .read()?;
-        let head = cmd!(sh, "git rev-parse HEAD").read()?;
-        let fetch_head = cmd!(sh, "git rev-parse FETCH_HEAD").read()?;
-        if head != fetch_head {
-            bail!(
-                "Josh created a non-roundtrip push! Do NOT merge this into rustc!\n\
-                Expected {head}, got {fetch_head}."
-            );
-        }
+        self.run(["git", "fetch", &josh_url, &branch]); // TODO depth=1?
+
+        let head = self.read(["git", "rev-parse", "HEAD"]);
+        let fetch_head = self.read(["git", "rev-parse", "FETCH_HEAD"]);
+        assert_eq!(
+            head, fetch_head,
+            "Josh created a non-roundtrip push! Do NOT merge this into rustc!\n\
+             Expected {head}, got {fetch_head}."
+        );
         println!(
-            "Confirmed that the push round-trips back to rustc-dev-guide properly. Please create a rustc PR:"
+            "Confirmed that the push round-trips back to rustc-dev-guide properly. Please \
+            create a rustc PR:"
         );
+        // Open PR with `subtree update` title to silence the `no-merges` triagebot check
         println!(
-            // Open PR with `subtree update` title to silence the `no-merges` triagebot check
-            "    https://github.com/{upstream_repo}/compare/{github_user}:{branch}?quick_pull=1&title=rustc-dev-guide+subtree+update&body=r?+@ghost"
+            "    {upstream_url}/compare/{github_user}:{branch}?quick_pull=1&title=rustc-dev-guide+subtree+update&body=r?+@ghost"
         );
 
         drop(josh);
@@ -263,16 +258,15 @@ impl GitSync {
         cmd.stderr(process::Stdio::null());
         let josh = cmd
             .spawn()
-            .context("failed to start josh-proxy, make sure it is installed")?;
+            .expect("failed to start josh-proxy, make sure it is installed");
 
         // Create a wrapper that stops it on drop.
         struct Josh(process::Child);
         impl Drop for Josh {
             fn drop(&mut self) {
-                #[cfg(unix)]
-                {
+                if cfg!(unix) {
                     // Try to gracefully shut it down.
-                    process::Command::new("kill")
+                    Command::new("kill")
                         .args(["-s", "INT", &self.0.id().to_string()])
                         .output()
                         .expect("failed to SIGINT josh-proxy");
@@ -290,7 +284,8 @@ impl GitSync {
                 }
                 // If that didn't work (or we're not on Unix), kill it hard.
                 eprintln!(
-                    "I have to kill josh-proxy the hard way, let's hope this does not break anything."
+                    "I have to kill josh-proxy the hard way, let's hope this does not \
+                    break anything."
                 );
                 self.0.kill().expect("failed to SIGKILL josh-proxy");
             }
@@ -312,7 +307,22 @@ impl GitSync {
         bail!("Even after waiting for 1s, josh-proxy is still not available.")
     }
 
-    fn run(&self, prog: &str, f: impl FnOnce(&mut Command) -> &mut Command) -> String {
+    fn run<'a, Args: AsRef<[&'a str]>>(&self, l: Args) {
+        let l = l.as_ref();
+        self.run_args(l[0], |c| c.args(&l[1..]))
+    }
+
+    fn run_args(&self, prog: &str, f: impl FnOnce(&mut Command) -> &mut Command) {
+        // self.read(l.as_ref());
+        self.read_args(prog, |c| f(c.stdout(Stdio::inherit())));
+    }
+
+    fn read<'a, Args: AsRef<[&'a str]>>(&self, l: Args) -> String {
+        let l = l.as_ref();
+        self.read_args(l[0], |c| c.args(&l[1..]))
+    }
+
+    fn read_args(&self, prog: &str, f: impl FnOnce(&mut Command) -> &mut Command) -> String {
         let mut cmd = Command::new(prog);
         cmd.current_dir(&self.dir).stderr(Stdio::inherit());
         f(&mut cmd);