Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace PR bot's diff tool with one that supports pagination #2245

Merged
merged 15 commits into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/manual-pull-request-bot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,18 @@ jobs:
# it uploads the image as a build artifact for other jobs to download and use.
acquire-base-image:
name: Acquire Base Image
needs:
- get-pr-info
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
path: smithy-rs
# The ref used needs to match the HEAD revision of the PR being diffed, or else
# the `docker-build` action won't find the built Docker image. This has the unfortunate
# side effect that the codegen diff tool used is the one in the PR rather than in
# the branch this workflow was launched from.
ref: ${{ fromJSON(needs.get-pr-info.outputs.pull_data).head_revision }}
fetch-depth: 0
- name: Acquire base image
id: acquire
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/pull-request-bot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ jobs:
- uses: actions/checkout@v3
with:
path: smithy-rs
ref: ${{ inputs.head_revision }}
- name: Generate diff
uses: ./smithy-rs/.github/actions/docker-build
with:
Expand Down
36 changes: 2 additions & 34 deletions tools/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,6 @@ ARG rust_nightly_version=nightly-2022-11-16

FROM ${base_image} AS bare_base_image
RUN yum -y updateinfo
#
# Node Installation Stage
#
FROM bare_base_image AS install_node
ARG node_version=v16.14.0
ENV DEST_PATH=/opt/nodejs \
PATH=/opt/nodejs/bin:${PATH}
RUN yum -y install \
ca-certificates \
curl \
tar \
xz && \
yum clean all
WORKDIR /root
RUN set -eux; \
ARCHITECTURE=""; \
if [[ "$(uname -m)" == "aarch64" || "$(uname -m)" == "arm64" ]]; then \
curl "https://nodejs.org/dist/${node_version}/node-${node_version}-linux-arm64.tar.xz" --output node.tar.xz; \
echo "5a6e818c302527a4b1cdf61d3188408c8a3e4a1bbca1e3f836c93ea8469826ce node.tar.xz" | sha256sum --check; \
ARCHITECTURE="arm64"; \
else \
curl "https://nodejs.org/dist/${node_version}/node-${node_version}-linux-x64.tar.xz" --output node.tar.xz; \
echo "0570b9354959f651b814e56a4ce98d4a067bf2385b9a0e6be075739bc65b0fae node.tar.xz" | sha256sum --check; \
ARCHITECTURE="x64"; \
fi; \
mkdir -p "${DEST_PATH}"; \
tar -xJvf node.tar.xz -C "${DEST_PATH}"; \
mv "${DEST_PATH}/node-${node_version}-linux-${ARCHITECTURE}/"* "${DEST_PATH}"; \
rmdir "${DEST_PATH}"/node-${node_version}-linux-${ARCHITECTURE}; \
rm node.tar.xz; \
node --version

#
# Rust & Tools Installation Stage
Expand Down Expand Up @@ -102,6 +71,7 @@ RUN set -eux; \
cargo +${rust_nightly_version} -Z sparse-registry install --locked --path tools/publisher; \
cargo +${rust_nightly_version} -Z sparse-registry install --locked --path tools/changelogger; \
cargo +${rust_nightly_version} -Z sparse-registry install --locked --path tools/crate-hasher; \
cargo +${rust_nightly_version} -Z sparse-registry install --locked --path tools/difftags; \
cargo +${rust_nightly_version} -Z sparse-registry install --locked --path tools/sdk-lints; \
cargo +${rust_nightly_version} -Z sparse-registry install --locked --path tools/sdk-versioner; \
chmod g+rw -R /opt/cargo/registry
Expand Down Expand Up @@ -160,7 +130,6 @@ RUN set -eux; \
groupadd build; \
useradd -m -g build build; \
chmod 775 /home/build;
COPY --chown=build:build --from=install_node /opt/nodejs /opt/nodejs
COPY --chown=build:build --from=local_tools /opt/cargo /opt/cargo
COPY --chown=build:build --from=cargo_deny /opt/cargo/bin/cargo-deny /opt/cargo/bin/cargo-deny
COPY --chown=build:build --from=cargo_udeps /opt/cargo/bin/cargo-udeps /opt/cargo/bin/cargo-udeps
Expand All @@ -169,7 +138,7 @@ COPY --chown=build:build --from=cargo_minimal_versions /opt/cargo/bin/cargo-mini
COPY --chown=build:build --from=cargo_check_external_types /opt/cargo/bin/cargo-check-external-types /opt/cargo/bin/cargo-check-external-types
COPY --chown=build:build --from=maturin /opt/cargo/bin/maturin /opt/cargo/bin/maturin
COPY --chown=build:build --from=install_rust /opt/rustup /opt/rustup
ENV PATH=/opt/cargo/bin:/opt/nodejs/bin:$PATH \
ENV PATH=/opt/cargo/bin:$PATH \
CARGO_HOME=/opt/cargo \
RUSTUP_HOME=/opt/rustup \
JAVA_HOME=/usr/lib/jvm/java-11-amazon-corretto.x86_64 \
Expand All @@ -185,7 +154,6 @@ ENV PATH=/opt/cargo/bin:/opt/nodejs/bin:$PATH \
# This is used primarily by the `build.gradle.kts` files in choosing how to execute build tools. If inside the image,
# they will assume the tools are on the PATH, but if outside of the image, they will `cargo run` the tools.
ENV SMITHY_RS_DOCKER_BUILD_IMAGE=1
RUN npm install -g diff2html-cli@5.1.11 && pip3 install --no-cache-dir uvloop==0.16.0 aiohttp==3.8.1
WORKDIR /home/build
COPY ci-build/scripts/sanity-test /home/build/sanity-test
RUN /home/build/sanity-test
1 change: 1 addition & 0 deletions tools/ci-build/scripts/check-tools
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ function test_tool {
test_tool "tools/changelogger" "${RUST_STABLE_VERSION}"
test_tool "tools/ci-cdk/canary-runner" "${RUST_STABLE_VERSION}"
test_tool "tools/crate-hasher" "${RUST_STABLE_VERSION}"
test_tool "tools/difftags" "${RUST_STABLE_VERSION}"
test_tool "tools/publisher" "${RUST_STABLE_VERSION}"
test_tool "tools/sdk-lints" "${RUST_STABLE_VERSION}"
test_tool "tools/sdk-versioner" "${RUST_STABLE_VERSION}"
Expand Down
3 changes: 1 addition & 2 deletions tools/ci-build/scripts/sanity-test
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ set -eux
cargo --version
changelogger --version
crate-hasher --version
diff2html --version
difftags --version
git --version
java --version
node --version
publisher --version
python3 --version
rustc +"${RUST_NIGHTLY_VERSION}" --version
Expand Down
68 changes: 17 additions & 51 deletions tools/codegen-diff-revisions.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

HEAD_BRANCH_NAME = "__tmp-localonly-head"
BASE_BRANCH_NAME = "__tmp-localonly-base"
OUTPUT_PATH = "tmp-codegen-diff/"
OUTPUT_PATH = "tmp-codegen-diff"

COMMIT_AUTHOR_NAME = "GitHub Action (generated code preview)"
COMMIT_AUTHOR_EMAIL = "generated-code-action@github.com"
Expand Down Expand Up @@ -99,9 +99,9 @@ def generate_and_commit_generated_code(revision_sha):
# Move generated code into codegen-diff/ directory
run(f"rm -rf {OUTPUT_PATH}")
run(f"mkdir {OUTPUT_PATH}")
run(f"mv aws/sdk/build/aws-sdk {OUTPUT_PATH}")
run(f"mv codegen-server-test/build/smithyprojections/codegen-server-test {OUTPUT_PATH}")
run(f"mv codegen-server-test/python/build/smithyprojections/codegen-server-test-python {OUTPUT_PATH}")
run(f"mv aws/sdk/build/aws-sdk {OUTPUT_PATH}/")
run(f"mv codegen-server-test/build/smithyprojections/codegen-server-test {OUTPUT_PATH}/")
run(f"mv codegen-server-test/python/build/smithyprojections/codegen-server-test-python {OUTPUT_PATH}/")

# Clean up the server-test folder
run(f"rm -rf {OUTPUT_PATH}/codegen-server-test/source")
Expand All @@ -120,61 +120,27 @@ def generate_and_commit_generated_code(revision_sha):
f"commit --no-verify -m 'Generated code for {revision_sha}' --allow-empty")


# Writes an HTML template for diff2html so that we can add contextual information
def write_html_template(title, subtitle, tmp_file):
tmp_file.writelines(map(lambda line: line.encode(), [
"<!doctype html>",
"<html>",
"<head>",
' <metadata charset="utf-8">',
f' <title>Codegen diff for the {title}: {subtitle}</title>',
' <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/9.9.0/styles/github.min.css" / >',
' <!--diff2html-css-->',
' <!--diff2html-js-ui-->',
' <script>',
' document.addEventListener("DOMContentLoaded", () => {',
' const targetElement = document.getElementById("diff");',
' const diff2htmlUi = new Diff2HtmlUI(targetElement);',
' //diff2html-fileListToggle',
' //diff2html-synchronisedScroll',
' //diff2html-highlightCode',
' });',
' </script>',
"</head>",
"<body>",
f" <h1>Codegen diff for the {title}</h1>",
f" <p>{subtitle}</p>",
' <div id="diff">',
' <!--diff2html-diff-->',
' </div>',
"</body>",
"</html>",
]))
tmp_file.flush()


def make_diff(title, path_to_diff, base_commit_sha, head_commit_sha, suffix, whitespace):
whitespace_flag = "" if whitespace else "-b"
diff_exists = get_cmd_status(f"git diff --quiet {whitespace_flag} "
f"{BASE_BRANCH_NAME} {HEAD_BRANCH_NAME} -- {path_to_diff}")
if diff_exists == 0:
eprint(f"No diff output for {base_commit_sha}..{head_commit_sha}")
eprint(f"No diff output for {base_commit_sha}..{head_commit_sha} ({suffix})")
return None
else:
run(f"mkdir -p {OUTPUT_PATH}/{base_commit_sha}/{head_commit_sha}")
dest_path = f"{base_commit_sha}/{head_commit_sha}/diff-{suffix}.html"
partial_output_path = f"{base_commit_sha}/{head_commit_sha}/{suffix}"
full_output_path = f"{OUTPUT_PATH}/{partial_output_path}"
run(f"mkdir -p {full_output_path}")
run(f"git diff --output=codegen-diff.txt -U30 {whitespace_flag} {BASE_BRANCH_NAME} {HEAD_BRANCH_NAME} -- {path_to_diff}")

# Generate HTML diff. This uses the `difftags` tool from the `tools/` directory.
# All arguments after the first `--` go to the `git diff` command.
whitespace_context = "" if whitespace else "(ignoring whitespace)"
with tempfile.NamedTemporaryFile() as tmp_file:
write_html_template(title, f"rev. {head_commit_sha} {whitespace_context}", tmp_file)

# Generate HTML diff. This uses the diff2html-cli, which defers to `git diff` under the hood.
# All arguments after the first `--` go to the `git diff` command.
diff_cmd = f"diff2html -s line -f html -d word -i command --hwt "\
f"{tmp_file.name} -F {OUTPUT_PATH}/{dest_path} -- "\
f"-U20 {whitespace_flag} {BASE_BRANCH_NAME} {HEAD_BRANCH_NAME} -- {path_to_diff}"
eprint(f"Running diff cmd: {diff_cmd}")
run(diff_cmd)
return dest_path
subtitle = f"rev. {head_commit_sha} {whitespace_context}"
diff_cmd = f"difftags --output-dir {full_output_path} --title \"{title}\" --subtitle \"{subtitle}\" codegen-diff.txt"
eprint(f"Running diff cmd: {diff_cmd}")
run(diff_cmd)
return f"{partial_output_path}/index.html"


def diff_link(diff_text, empty_diff_text, diff_location, alternate_text, alternate_location):
Expand Down