From 90efb0abe39c0ff9df18cc96c4a4ebfdc12b7f28 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Mon, 10 Jul 2023 09:31:59 -0400 Subject: [PATCH 1/3] Add `LD_LIBRARY_PATH` in `check_diff.sh` There were some upstream changes made a while back that requires this to be set when building rustfmt from source like we do in the `check_diff.sh` script. See issue 5675 for more details. --- ci/check_diff.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ci/check_diff.sh b/ci/check_diff.sh index 062c2dd8673..6f30e8ba9cb 100755 --- a/ci/check_diff.sh +++ b/ci/check_diff.sh @@ -1,5 +1,8 @@ #!/bin/bash +# https://github.com/rust-lang/rustfmt/issues/5675 +export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib:$LD_LIBRARY_PATH + function print_usage() { echo "usage check_diff REMOTE_REPO FEATURE_BRANCH [COMMIT_HASH] [OPTIONAL_RUSTFMT_CONFIGS]" } From d58b3e83a189f41a65a7d15488138fcb1e304b38 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Mon, 10 Jul 2023 10:03:51 -0400 Subject: [PATCH 2/3] Improve error discovery in `check_diff.sh` The `set -e` option is used to immediately exit if any command exits with a non zero exit status. This will help us catch errors in the script, for example, needing the `LD_LIBRARY_PATH` to be set. --- ci/check_diff.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ci/check_diff.sh b/ci/check_diff.sh index 6f30e8ba9cb..a685c2e108e 100755 --- a/ci/check_diff.sh +++ b/ci/check_diff.sh @@ -1,5 +1,7 @@ #!/bin/bash +set -e + # https://github.com/rust-lang/rustfmt/issues/5675 export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib:$LD_LIBRARY_PATH @@ -143,9 +145,15 @@ function check_repo() { init_submodules $SUBMODULES fi + + # rustfmt --check returns 1 if a diff was found + # Also check_diff returns 1 if there was a diff between master rustfmt and the feature branch + # so we want to ignore the exit status check + set +e check_diff $REPO_NAME # append the status of running `check_diff` to the STATUSES array STATUSES+=($?) + set -e echo "removing tmp_dir $tmp_dir" rm -rf $tmp_dir From a968ffbff7c003bc1a1c18955efacf40d1050ae1 Mon Sep 17 00:00:00 2001 From: Yacin Tmimi Date: Mon, 10 Jul 2023 11:07:49 -0400 Subject: [PATCH 3/3] use the `branch_name` as the default for the optional commit hash There was an issue with the script when passing optional rustfmt configs without specifying a commit hash. Because these optional values are passed via positional arguments the configs ($4) would be used in place of the commit hash ($3). Now that we set a default value for the optional commit hash we avoid this problem. --- .github/workflows/check_diff.yml | 2 +- ci/check_diff.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check_diff.yml b/.github/workflows/check_diff.yml index 8bfb5834519..2f2beb76915 100644 --- a/.github/workflows/check_diff.yml +++ b/.github/workflows/check_diff.yml @@ -30,4 +30,4 @@ jobs: rustup target add x86_64-unknown-linux-gnu - name: check diff - run: bash ${GITHUB_WORKSPACE}/ci/check_diff.sh ${{ github.event.inputs.clone_url }} ${{ github.event.inputs.branch_name }} ${{ github.event.inputs.commit_hash }} ${{ github.event.inputs.rustfmt_configs }} + run: bash ${GITHUB_WORKSPACE}/ci/check_diff.sh ${{ github.event.inputs.clone_url }} ${{ github.event.inputs.branch_name }} ${{ github.event.inputs.commit_hash || github.event.inputs.branch_name }} ${{ github.event.inputs.rustfmt_configs }} diff --git a/ci/check_diff.sh b/ci/check_diff.sh index a685c2e108e..50c58b1f492 100755 --- a/ci/check_diff.sh +++ b/ci/check_diff.sh @@ -115,7 +115,7 @@ function compile_rustfmt() { git fetch feature $FEATURE_BRANCH cargo build --release --bin rustfmt && cp target/release/rustfmt $1/rustfmt - if [ -z "$OPTIONAL_COMMIT_HASH" ]; then + if [ -z "$OPTIONAL_COMMIT_HASH" ] || [ "$FEATURE_BRANCH" = "$OPTIONAL_COMMIT_HASH" ]; then git switch $FEATURE_BRANCH else git switch $OPTIONAL_COMMIT_HASH --detach