Unverified Commit 2f701abc authored by Zelda Hessler's avatar Zelda Hessler Committed by GitHub
Browse files

Fix: strange commit behavior in sync tool (#1042)

* update: create mirror commit now works by shelling out
update: rm -rf instead of gradle clean
update: find_last_commit to work by shelling out

* add: apologetic comment regarding shelling out
parent a77c5ca2
Loading
Loading
Loading
Loading
+51 −46
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@ mod fs;

use crate::fs::{delete_all_generated_files_and_folders, find_handwritten_files_and_folders};
use anyhow::{anyhow, bail, Context, Result};
use git2::{Commit, IndexAddOption, ObjectType, Oid, Repository, ResetType, Signature};
use git2::{Commit, Oid, Repository, ResetType};
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use std::process::Command;
@@ -34,8 +34,8 @@ struct Opt {

const BOT_NAME: &str = "AWS SDK Rust Bot";
const BOT_EMAIL: &str = "aws-sdk-rust-primary@amazon.com";
const COMMIT_HASH_FILENAME: &str = ".smithyrs-githash";
const BOT_COMMIT_PREFIX: &str = "[autosync]";
const COMMIT_HASH_FILENAME: &str = ".smithyrs-githash";

/// A macro for attaching info to error messages pointing to the line of code responsible for the error.
/// [Thanks to dtolnay for this macro](https://github.com/dtolnay/anyhow/issues/22#issuecomment-542309452)
@@ -158,7 +158,7 @@ fn sync_aws_sdk_with_smithy_rs(
        }

        copy_sdk(&build_artifacts, &aws_sdk)?;
        create_mirror_commit(&aws_sdk_repo, &commit)
        create_mirror_commit(&aws_sdk, &commit)
            .context("couldn't commit SDK changes to aws-sdk-rust")?;
    }

@@ -216,8 +216,7 @@ fn get_last_synced_commit(repo_path: &Path) -> Result<Oid> {
}

/// Write the last synced commit to the file in aws-sdk-rust that tracks the last smithy-rs commit it was synced with.
fn set_last_synced_commit(repo: &Repository, oid: &Oid) -> Result<()> {
    let repo_path = repo.workdir().expect("this will always exist");
fn set_last_synced_commit(repo_path: &Path, oid: &Oid) -> Result<()> {
    let oid_string = oid.to_string();
    let oid_bytes = oid_string.as_bytes();
    let path = repo_path.join(COMMIT_HASH_FILENAME);
@@ -237,11 +236,7 @@ fn build_sdk(smithy_rs_path: &Path) -> Result<PathBuf> {
        .expect("for our use case, this will always be UTF-8");

    // The output of running these commands isn't logged anywhere unless they fail
    let _ = run(
        &[gradlew, "-Paws.fullsdk=true", ":aws:sdk:clean"],
        smithy_rs_path,
    )
    .context(here!())?;
    let _ = run(&["rm", "-rf", "aws/sdk/build"], smithy_rs_path).context(here!())?;
    let _ = run(
        &[gradlew, "-Paws.fullsdk=true", ":aws:sdk:assemble"],
        smithy_rs_path,
@@ -302,54 +297,51 @@ fn copy_sdk(from_path: &Path, to_path: &Path) -> Result<()> {
    Ok(())
}

/// Find the last commit made to a repo.
fn find_last_commit(repo: &Repository) -> Result<Commit> {
    let obj = repo
        .head()
        .context(here!())?
        .resolve()
        .context(here!())?
        .peel(ObjectType::Commit)
        .context(here!())?;
    obj.into_commit()
        .map_err(|_| anyhow!("couldn't find last commit"))
}

/// Create a "mirror" commit. Works by reading a smithy-rs commit and then using the info
/// attached to it to create a commit in aws-sdk-rust. This also updates the `.smithyrs-githash`
/// file with the hash of `based_on_commit`.
fn create_mirror_commit(aws_sdk_repo: &Repository, based_on_commit: &Commit) -> Result<()> {
///
/// *NOTE: If you're wondering why `git2` is used elsewhere in this tool but not in this function,
/// it's due to strange, undesired behavior that occurs. For every commit, something
/// happened that created leftover staged and unstaged changes. When the unstaged changes
/// were staged, they cancelled out the first set of staged changes. I don't know why this
/// happened and if you think you can fix it, you can check out the previous version of the*
/// tool in [this PR](https://github.com/awslabs/smithy-rs/pull/1042)
fn create_mirror_commit(aws_sdk_path: &Path, based_on_commit: &Commit) -> Result<()> {
    eprintln!("\tcreating mirror commit...");

    // Update the file that tracks what smithy-rs commit the SDK was generated from
    set_last_synced_commit(aws_sdk_repo, &based_on_commit.id()).context(here!())?;

    let mut index = aws_sdk_repo.index().context(here!())?;
    // The equivalent of `git add .`
    index
        .add_all(["."].iter(), IndexAddOption::DEFAULT, None)
        .context(here!())?;
    let oid = index.write_tree().context(here!())?;
    let parent_commit = find_last_commit(aws_sdk_repo).context(here!())?;
    let tree = aws_sdk_repo.find_tree(oid).context(here!())?;
    let bot_signature = Signature::now(BOT_NAME, BOT_EMAIL).context(here!())?;

    let _ = aws_sdk_repo
        .commit(
            Some("HEAD"),
            &based_on_commit.author(),
            &bot_signature,
    set_last_synced_commit(aws_sdk_path, &based_on_commit.id()).context(here!())?;

    run(&["git", "add", "."], aws_sdk_path).context(here!())?;
    run(
        &[
            "git",
            // Set the committer of this commit
            "-c",
            &format!("user.name={}", BOT_NAME),
            "-c",
            &format!("user.email={}", BOT_EMAIL),
            "commit",
            // Set the commit message
            "-m",
            // Even thought we're inserting user input here, we're safe from shell injection because
            // the Rust Command API passes args directly to the application to be run
            &format!(
                "{} {}",
                BOT_COMMIT_PREFIX,
                based_on_commit.message().unwrap_or_default()
            ),
            &tree,
            &[&parent_commit],
            // Set the original author of this commit
            "--author",
            &based_on_commit.author().to_string(),
        ],
        aws_sdk_path,
    )
    .context(here!())?;
    let commit_hash = find_last_commit(aws_sdk_path).context(here!())?;

    eprintln!("\tsuccessfully created mirror commit");
    eprintln!("\tsuccessfully created mirror commit {}", commit_hash);

    Ok(())
}
@@ -422,6 +414,19 @@ where
    Ok(())
}

fn find_last_commit(repo_path: &Path) -> Result<String> {
    let output = Command::new("git")
        .arg("rev-parse")
        .arg("HEAD")
        .current_dir(&repo_path)
        .output()
        .map_err(|err| anyhow!("couldn't get commit hash: {}", err))?;

    let hash = String::from_utf8_lossy(&output.stdout);

    Ok(hash.to_string())
}

/// For a slice containing `S` where `S: AsRef<OsStr>`, join all `S` into a space-separated String.
fn stringify_args<S>(args: &[S]) -> String
where