Unverified Commit e1e9a29d authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Fix some bugs in the `sdk-sync` tool (#1382)

parent 83af6085
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -2,7 +2,7 @@
import os
import sys

expected = [os.path.realpath("/tmp"), ["merge", "--ff-only", "test-branch-name"]]
expected = [os.path.realpath("/tmp"), ["branch", "-D", "test-branch-name"]]
actual = [os.getcwd(), sys.argv[1:]]
if expected != actual:
    print(f"ERROR\nExpect: {expected}\nActual: {actual}")
+19 −0
Original line number Diff line number Diff line
#!/usr/bin/env python3
import os
import sys

if sys.argv[1] == "merge":
    expected = [os.path.realpath("/tmp"), ["merge", "--squash", "test-branch-name"]]
    actual = [os.getcwd(), sys.argv[1:]]
    if expected != actual:
        print(f"ERROR\nExpect: {expected}\nActual: {actual}")
        sys.exit(1)
else:
    expected = [
        os.path.realpath("/tmp"),
        ["-c", "user.name=test-author", "-c", "user.email=test-author-email", "commit", "-m", "test message"]
    ]
    actual = [os.getcwd(), sys.argv[1:]]
    if expected != actual:
        print(f"ERROR\nExpect: {expected}\nActual: {actual}")
        sys.exit(1)
+50 −9
Original line number Diff line number Diff line
@@ -101,8 +101,17 @@ pub trait Git: Send + Sync {
    /// Creates a branch at the given revision.
    fn create_branch(&self, branch_name: &str, revision: &str) -> Result<()>;

    /// Fast-forward merges a branch.
    fn fast_forward_merge(&self, branch_name: &str) -> Result<()>;
    /// Deletes a branch.
    fn delete_branch(&self, branch_name: &str) -> Result<()>;

    /// Squash merges a branch into the current branch.
    fn squash_merge(
        &self,
        author_name: &str,
        author_email: &str,
        branch_name: &str,
        commit_message: &str,
    ) -> Result<()>;

    /// Returns list of untracked files.
    fn untracked_files(&self) -> Result<Vec<PathBuf>>;
@@ -322,18 +331,38 @@ impl Git for GitCLI {
        Ok(())
    }

    fn fast_forward_merge(&self, branch_name: &str) -> Result<()> {
    fn delete_branch(&self, branch_name: &str) -> Result<()> {
        let mut command = Command::new(&self.binary_name);
        command.arg("merge");
        command.arg("--ff-only");
        command.arg("branch");
        command.arg("-D");
        command.arg(branch_name);
        command.current_dir(&self.repo_path);

        let output = log_command(command).output()?;
        handle_failure("fast_forward_merge", &output)?;
        handle_failure("delete_branch", &output)?;
        Ok(())
    }

    fn squash_merge(
        &self,
        author_name: &str,
        author_email: &str,
        branch_name: &str,
        commit_message: &str,
    ) -> Result<()> {
        let mut command = Command::new(&self.binary_name);
        command.arg("merge");
        command.arg("--squash");
        command.arg(branch_name);
        command.current_dir(&self.repo_path);

        let output = log_command(command).output()?;
        handle_failure("squash_merge", &output)?;

        // `git merge --squash` only stages changes, so a commit is necessary after
        self.commit(author_name, author_email, commit_message)
    }

    fn untracked_files(&self) -> Result<Vec<PathBuf>> {
        let mut command = Command::new(&self.binary_name);
        command.arg("ls-files");
@@ -564,9 +593,21 @@ mod tests {
    }

    #[test]
    fn fast_forward_merge() {
        cli("git-ff-merge")
            .fast_forward_merge("test-branch-name")
    fn delete_branch() {
        cli("git-delete-branch")
            .delete_branch("test-branch-name")
            .expect("successful invocation");
    }

    #[test]
    fn squash_merge() {
        cli("git-squash-merge")
            .squash_merge(
                "test-author",
                "test-author-email",
                "test-branch-name",
                "test message",
            )
            .expect("successful invocation");
    }
}
+38 −80
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@

use self::gen::{DefaultSdkGenerator, SdkGenerator};
use crate::fs::{DefaultFs, Fs};
use crate::git::{Commit, CommitHash, Git, GitCLI};
use crate::git::{Commit, Git, GitCLI};
use crate::versions::{DefaultVersions, Versions, VersionsManifest};
use anyhow::{bail, Context, Result};
use smithy_rs_tool_common::macros::here;
@@ -21,49 +21,12 @@ pub const BOT_NAME: &str = "AWS SDK Rust Bot";
pub const BOT_EMAIL: &str = "aws-sdk-rust-primary@amazon.com";
pub const MODEL_STASH_BRANCH_NAME: &str = "__sdk_sync__models_";

#[cfg_attr(test, mockall::automock)]
pub trait CreateSdkGenerator: Send + std::marker::Sync {
    fn create_sdk_generator(
        &self,
        aws_doc_sdk_examples_revision: &CommitHash,
        examples_path: &Path,
        fs: Arc<dyn Fs>,
        reset_to_commit: Option<CommitHash>,
        original_smithy_rs_path: &Path,
    ) -> Result<Box<dyn SdkGenerator>>;
}

pub struct DefaultCreateSdkGenerator;

impl CreateSdkGenerator for DefaultCreateSdkGenerator {
    fn create_sdk_generator(
        &self,
        aws_doc_sdk_examples_revision: &CommitHash,
        examples_path: &Path,
        fs: Arc<dyn Fs>,
        reset_to_commit: Option<CommitHash>,
        original_smithy_rs_path: &Path,
    ) -> Result<Box<dyn SdkGenerator>> {
        Ok(Box::new(
            DefaultSdkGenerator::new(
                aws_doc_sdk_examples_revision,
                examples_path,
                fs,
                reset_to_commit,
                original_smithy_rs_path,
            )
            .context(here!())?,
        ))
    }
}

pub struct Sync {
    aws_doc_sdk_examples: Arc<dyn Git>,
    aws_sdk_rust: Arc<dyn Git>,
    smithy_rs: Arc<dyn Git>,
    fs: Arc<dyn Fs>,
    versions: Arc<dyn Versions>,
    create_sdk_generator: Arc<dyn CreateSdkGenerator>,
}

impl Sync {
@@ -78,7 +41,6 @@ impl Sync {
            smithy_rs: Arc::new(GitCLI::new(smithy_rs_path)?),
            fs: Arc::new(DefaultFs::new()) as Arc<dyn Fs>,
            versions: Arc::new(DefaultVersions::new()),
            create_sdk_generator: Arc::new(DefaultCreateSdkGenerator),
        })
    }

@@ -88,7 +50,6 @@ impl Sync {
        smithy_rs: impl Git + 'static,
        fs: impl Fs + 'static,
        versions: impl Versions + 'static,
        create_sdk_generator: impl CreateSdkGenerator + 'static,
    ) -> Self {
        Self {
            aws_doc_sdk_examples: Arc::new(aws_doc_sdk_examples),
@@ -96,7 +57,6 @@ impl Sync {
            smithy_rs: Arc::new(smithy_rs),
            fs: Arc::new(fs),
            versions: Arc::new(versions),
            create_sdk_generator: Arc::new(create_sdk_generator),
        }
    }

@@ -155,23 +115,31 @@ impl Sync {
    fn sync_model_changes(&self, versions: &VersionsManifest) -> Result<()> {
        info!("Syncing model changes...");

        // Restore the model changes
        // Restore the model changes. Note: endpoints.json/default config/model changes
        // may each be in their own commits coming into this, but we want them squashed into
        // one commit for smithy-rs.
        self.smithy_rs
            .fast_forward_merge(MODEL_STASH_BRANCH_NAME)
            .squash_merge(
                BOT_NAME,
                BOT_EMAIL,
                MODEL_STASH_BRANCH_NAME,
                "Update SDK models",
            )
            .context(here!())?;
        self.smithy_rs
            .delete_branch(MODEL_STASH_BRANCH_NAME)
            .context(here!())?;
        let model_change_commit = self.smithy_rs.show("HEAD").context(here!())?;

        // Generate with the original examples
        let sdk_gen = self
            .create_sdk_generator
            .create_sdk_generator(
        let sdk_gen = DefaultSdkGenerator::new(
            &versions.aws_doc_sdk_examples_revision,
            &self.aws_sdk_rust.path().join("examples"),
            self.fs.clone(),
            None,
            self.smithy_rs.path(),
        )
            .context(here!("failed to generate the SDK"))?;
        .context(here!())?;
        let generated_sdk = sdk_gen.generate_sdk().context(here!())?;
        self.copy_sdk(generated_sdk.path())
            .context(here!("failed to copy the SDK"))?;
@@ -214,7 +182,6 @@ impl Sync {
        // Generate code in parallel for each individual commit
        let code_gen_paths = {
            let smithy_rs = self.smithy_rs.clone();
            let create_sdk_generator = self.create_sdk_generator.clone();
            let examples_revision = versions.aws_doc_sdk_examples_revision.clone();
            let examples_path = self.aws_sdk_rust.path().join("examples");
            let fs = self.fs.clone();
@@ -234,8 +201,7 @@ impl Sync {
                        format!("couldn't find commit {} in smithy-rs", commit_hash)
                    })?;

                    let sdk_gen = create_sdk_generator
                        .create_sdk_generator(
                    let sdk_gen = DefaultSdkGenerator::new(
                        &examples_revision,
                        &examples_path,
                        fs.clone(),
@@ -285,9 +251,7 @@ impl Sync {
        }
        let examples_head = example_revisions.iter().cloned().next().unwrap();

        let sdk_gen = self
            .create_sdk_generator
            .create_sdk_generator(
        let sdk_gen = DefaultSdkGenerator::new(
            &examples_head,
            &self.aws_doc_sdk_examples.path().join("rust_dev_preview"),
            self.fs.clone(),
@@ -445,7 +409,7 @@ impl Sync {
mod tests {
    use super::*;
    use crate::fs::MockFs;
    use crate::git::MockGit;
    use crate::git::{CommitHash, MockGit};
    use crate::versions::MockVersions;

    // Wish this was in std...
@@ -544,7 +508,6 @@ mod tests {
            MockGit::new(),
            MockFs::new(),
            MockVersions::new(),
            MockCreateSdkGenerator::new(),
        );
        assert!(sync
            .commit_sdk_changes(
@@ -596,7 +559,6 @@ mod tests {
            MockGit::new(),
            MockFs::new(),
            MockVersions::new(),
            MockCreateSdkGenerator::new(),
        );
        assert!(sync
            .commit_sdk_changes(
@@ -630,7 +592,6 @@ mod tests {
            MockGit::new(),
            MockFs::new(),
            MockVersions::new(),
            MockCreateSdkGenerator::new(),
        );

        assert!(
@@ -657,7 +618,6 @@ mod tests {
            MockGit::new(),
            MockFs::new(),
            MockVersions::new(),
            MockCreateSdkGenerator::new(),
        );

        assert!(
@@ -684,7 +644,6 @@ mod tests {
            MockGit::new(),
            MockFs::new(),
            MockVersions::new(),
            MockCreateSdkGenerator::new(),
        );

        assert!(sync.sdk_has_changes().unwrap(), "it should have changes");
@@ -713,7 +672,6 @@ mod tests {
            MockGit::new(),
            MockFs::new(),
            MockVersions::new(),
            MockCreateSdkGenerator::new(),
        );

        assert!(sync.sdk_has_changes().unwrap(), "it should have changes");
+6 −0
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ if [[ $# -eq 1 && "$1" == "--with-model-changes" ]]; then
    INCLUDE_MODEL_CHANGES=1
fi

ENDPOINTS_JSON_PATH="aws/sdk-codegen/src/main/resources/software/amazon/smithy/rustsdk/endpoints.json"

mkdir aws-doc-sdk-examples
mkdir aws-sdk-rust
mkdir smithy-rs
@@ -42,7 +44,9 @@ mkdir -p aws/sdk/aws-models
mkdir -p aws/sdk/examples
mkdir -p aws/sdk/build/aws-sdk/examples/s3
mkdir -p aws/sdk/build/aws-sdk/sdk/s3
mkdir -p $(dirname "${ENDPOINTS_JSON_PATH}")
echo "Ancient S3 model" > aws/sdk/aws-models/s3.json
echo "Old endpoints.json" > "${ENDPOINTS_JSON_PATH}"
echo "Some S3 client code" > aws/sdk/build/aws-sdk/sdk/s3/fake_content
cat "${SCRIPT_PATH}/fake-sdk-assemble" > gradlew
chmod +x gradlew
@@ -65,6 +69,8 @@ if [[ "${INCLUDE_MODEL_CHANGES}" == "1" ]]; then
    pushd smithy-rs
    echo "Updated S3 model" > aws/sdk/aws-models/s3.json
    git -c user.name="Automated Process" -c user.email="bot@example.com" commit -am "Update the S3 model"
    echo "Updated endpoints.json" > "${ENDPOINTS_JSON_PATH}"
    git -c user.name="Automated Process" -c user.email="bot@example.com" commit -am "Update endpoints.json"
    popd
fi

Loading