From 4fed612d5d4b30238b3dd0ca57633ccda26b3547 Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Mon, 3 Jun 2024 19:15:01 +0000 Subject: [PATCH 1/8] Add workflow for detecting breaking proto changes. --- .github/workflows/proto_breaking.yml | 87 ++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 .github/workflows/proto_breaking.yml diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml new file mode 100644 index 0000000..eb50af6 --- /dev/null +++ b/.github/workflows/proto_breaking.yml @@ -0,0 +1,87 @@ +name: Identify Breaking Proto Changes + +on: + workflow_call: + +jobs: + build: + name: Identify Breaking Proto Changes + runs-on: ubuntu-latest + env: + BAZEL: bazelisk-linux-amd64 + BAZELISK_VERSION: v1.19.0 + + steps: + - name: Checkout Code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Install bazelisk + run: | + curl -LO "https://github.com/bazelbuild/bazelisk/releases/download/${BAZELISK_VERSION}/${BAZEL}" + chmod +x "${BAZEL}" + sudo mv "${BAZEL}" /usr/local/bin/bazel + - name: Mount bazel cache + uses: actions/cache@v4 + with: + # See https://docs.bazel.build/versions/master/output_directories.html + path: "~/.cache/bazel" + # Create a new cache entry whenever Bazel files change. + # See https://docs.github.com/en/actions/guides/caching-dependencies-to-speed-up-workflows + key: bazel-${{ runner.os }}-build-${{ hashFiles('**/*.bzl', '**/*.bazel') }} + restore-keys: bazel-${{ runner.os }}-build- + - name: Compare HEAD and BASE for breaking protobuf changes + run: | + virtualenv .venv + source .venv/bin/activate + pip install git+https://github.com/googleapis/proto-breaking-change-detector.git@v2.4.0 + HEAD="${{ github.event.pull_request.head.sha }}" + BASE="$(git merge-base origin/main "${HEAD}")" + TMPDIR="$(mktemp -d -t compare-XXXX)" + TMPDIRBASE="${TMPDIR}/base" + TMPDIRHEAD="${TMPDIR}/head" + + extract_protodesc() { + local TMPTARGETDIR="${1}" + local TARGETS="$(bazel query 'attr("generator_function", "proto_library", "//...")' 2>/dev/null)" + local GENF="$(bazel info bazel-genfiles)" + for TARGET in ${TARGETS}; do + local TARGETDIR="$(echo "${TARGET}" | cut -d ":" -f1 | sed 's#^//##')" + local PROTONAME="$(echo "${TARGET}" | cut -d ":" -f2-)" + mkdir -p "${TMPTARGETDIR}"/"${TARGETDIR}" + bazel build "${TARGET}" + cp "${GENF}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin "${TMPTARGETDIR}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" + done + } + + bazel clean + extract_protodesc "${TMPDIRHEAD}" + git checkout "${BASE}" + bazel clean + BASETARGETS="$(bazel query 'attr("generator_function", "proto_library", "//...")' 2>/dev/null)" + extract_protodesc "${TMPDIRBASE}" + + ERROR=false + for TARGET in ${BASETARGETS}; do + TARGETDIR="$(echo "${TARGET}" | cut -d ":" -f1 | sed 's#^//##')" + PROTONAME="$(echo "${TARGET}" | cut -d ":" -f2-)" + BASEPROTODESC="${TMPDIRBASE}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" + HEADPROTODESC="${TMPDIRHEAD}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" + if [ -f ${BASEPROTODESC} ] && [ -f ${HEADPROTODESC} ]; then + echo "Comparing build target ${TARGET} at HEAD and BASE..." + OUTPUT=$(proto-breaking-change-detector --original_descriptor_set_file_path="${BASEPROTODESC}" --update_descriptor_set_file_path="${HEADPROTODESC}" --human_readable_message 2>&1) + if [ "${OUTPUT}" == "" ]; then + echo "No breaking changes." + else + echo $OUTPUT + ERROR=true + fi + fi + if [ -f "${BASEPROTODESC}" ] && ! [ -f "${HEADPROTODESC}" ]; then + ERROR=true + echo "${BASEPROTODESC} exists and ${HEADPROTODESC} does not: removing a proto library is breaking." + fi + done + if [ "${ERROR}" == "true" ]; then + exit 1 + fi From ea04f6c241d58f1bb5cc15de4dbcd1045847ef3c Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Mon, 3 Jun 2024 19:20:15 +0000 Subject: [PATCH 2/8] Rename job in workflow --- .github/workflows/proto_breaking.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml index eb50af6..9068571 100644 --- a/.github/workflows/proto_breaking.yml +++ b/.github/workflows/proto_breaking.yml @@ -4,7 +4,7 @@ on: workflow_call: jobs: - build: + proto_breaking: name: Identify Breaking Proto Changes runs-on: ubuntu-latest env: From 07596457fbd765139789b7fc0ac928e160545594 Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Mon, 3 Jun 2024 19:38:34 +0000 Subject: [PATCH 3/8] Fix python venv --- .github/workflows/proto_breaking.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml index 9068571..849649f 100644 --- a/.github/workflows/proto_breaking.yml +++ b/.github/workflows/proto_breaking.yml @@ -30,9 +30,13 @@ jobs: # See https://docs.github.com/en/actions/guides/caching-dependencies-to-speed-up-workflows key: bazel-${{ runner.os }}-build-${{ hashFiles('**/*.bzl', '**/*.bazel') }} restore-keys: bazel-${{ runner.os }}-build- + - name: Set up Python 3.11 + uses: actions/setup-python@v5 + with: + python-version: 3.11 - name: Compare HEAD and BASE for breaking protobuf changes run: | - virtualenv .venv + python3 -m venv .venv source .venv/bin/activate pip install git+https://github.com/googleapis/proto-breaking-change-detector.git@v2.4.0 HEAD="${{ github.event.pull_request.head.sha }}" From 4aa1c49ef7eb5af9b340c5cd83b6901f79df6c8a Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Mon, 3 Jun 2024 19:48:24 +0000 Subject: [PATCH 4/8] Fix quotes --- .github/workflows/proto_breaking.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml index 849649f..1e5c707 100644 --- a/.github/workflows/proto_breaking.yml +++ b/.github/workflows/proto_breaking.yml @@ -52,9 +52,9 @@ jobs: for TARGET in ${TARGETS}; do local TARGETDIR="$(echo "${TARGET}" | cut -d ":" -f1 | sed 's#^//##')" local PROTONAME="$(echo "${TARGET}" | cut -d ":" -f2-)" - mkdir -p "${TMPTARGETDIR}"/"${TARGETDIR}" + mkdir -p "${TMPTARGETDIR}/${TARGETDIR}" bazel build "${TARGET}" - cp "${GENF}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin "${TMPTARGETDIR}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" + cp "${GENF}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" "${TMPTARGETDIR}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" done } From 7942e2beec8614c671cd23179dd15c93252d7eab Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Mon, 3 Jun 2024 20:00:57 +0000 Subject: [PATCH 5/8] Fix newline character output --- .github/workflows/proto_breaking.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml index 1e5c707..479a42b 100644 --- a/.github/workflows/proto_breaking.yml +++ b/.github/workflows/proto_breaking.yml @@ -77,7 +77,7 @@ jobs: if [ "${OUTPUT}" == "" ]; then echo "No breaking changes." else - echo $OUTPUT + echo -e "${OUTPUT}" ERROR=true fi fi From a8f8b9a36e9ffc19e79ae41bbbd16015ab38693a Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Mon, 3 Jun 2024 21:00:19 +0000 Subject: [PATCH 6/8] Fix bazel query to use exact match --- .github/workflows/proto_breaking.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml index 479a42b..8f9b3be 100644 --- a/.github/workflows/proto_breaking.yml +++ b/.github/workflows/proto_breaking.yml @@ -47,7 +47,7 @@ jobs: extract_protodesc() { local TMPTARGETDIR="${1}" - local TARGETS="$(bazel query 'attr("generator_function", "proto_library", "//...")' 2>/dev/null)" + local TARGETS="$(bazel query 'attr("generator_function", "^proto_library$", "//...")' 2>/dev/null)" local GENF="$(bazel info bazel-genfiles)" for TARGET in ${TARGETS}; do local TARGETDIR="$(echo "${TARGET}" | cut -d ":" -f1 | sed 's#^//##')" @@ -62,7 +62,7 @@ jobs: extract_protodesc "${TMPDIRHEAD}" git checkout "${BASE}" bazel clean - BASETARGETS="$(bazel query 'attr("generator_function", "proto_library", "//...")' 2>/dev/null)" + BASETARGETS="$(bazel query 'attr("generator_function", "^proto_library$", "//...")' 2>/dev/null)" extract_protodesc "${TMPDIRBASE}" ERROR=false From f2be33a67f833a2125e04bfb9baff8fd346b84e7 Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Wed, 12 Jun 2024 16:40:23 +0000 Subject: [PATCH 7/8] Fix linter warnings --- .github/workflows/proto_breaking.yml | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml index 8f9b3be..af03f99 100644 --- a/.github/workflows/proto_breaking.yml +++ b/.github/workflows/proto_breaking.yml @@ -46,15 +46,21 @@ jobs: TMPDIRHEAD="${TMPDIR}/head" extract_protodesc() { - local TMPTARGETDIR="${1}" - local TARGETS="$(bazel query 'attr("generator_function", "^proto_library$", "//...")' 2>/dev/null)" - local GENF="$(bazel info bazel-genfiles)" + local TMPTARGETDIR + local TARGETS + local GENF + local TARGETDIR + local PROTONAME + TMPTARGETDIR="${1}" + TARGETS="$(bazel query 'attr("generator_function", "^proto_library$", "//...")' 2>/dev/null)" + GENF="$(bazel info bazel-genfiles)" for TARGET in ${TARGETS}; do - local TARGETDIR="$(echo "${TARGET}" | cut -d ":" -f1 | sed 's#^//##')" - local PROTONAME="$(echo "${TARGET}" | cut -d ":" -f2-)" + TARGETDIR="$(echo "${TARGET}" | cut -d ":" -f1 | sed 's#^//##')" + PROTONAME="$(echo "${TARGET}" | cut -d ":" -f2-)" mkdir -p "${TMPTARGETDIR}/${TARGETDIR}" bazel build "${TARGET}" - cp "${GENF}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" "${TMPTARGETDIR}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" + cp "${GENF}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" \ + "${TMPTARGETDIR}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" done } @@ -71,9 +77,10 @@ jobs: PROTONAME="$(echo "${TARGET}" | cut -d ":" -f2-)" BASEPROTODESC="${TMPDIRBASE}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" HEADPROTODESC="${TMPDIRHEAD}/${TARGETDIR}/${PROTONAME}-descriptor-set.proto.bin" - if [ -f ${BASEPROTODESC} ] && [ -f ${HEADPROTODESC} ]; then + if [ -f "${BASEPROTODESC}" ] && [ -f "${HEADPROTODESC}" ]; then echo "Comparing build target ${TARGET} at HEAD and BASE..." - OUTPUT=$(proto-breaking-change-detector --original_descriptor_set_file_path="${BASEPROTODESC}" --update_descriptor_set_file_path="${HEADPROTODESC}" --human_readable_message 2>&1) + OUTPUT=$(proto-breaking-change-detector --original_descriptor_set_file_path="${BASEPROTODESC}" \ + --update_descriptor_set_file_path="${HEADPROTODESC}" --human_readable_message 2>&1) if [ "${OUTPUT}" == "" ]; then echo "No breaking changes." else From acf71048555db4175ae169c5894eb0c71f73f3c9 Mon Sep 17 00:00:00 2001 From: Brandon Stoll Date: Thu, 13 Jun 2024 15:56:29 +0000 Subject: [PATCH 8/8] Split setup, build, and compare steps. --- .github/workflows/proto_breaking.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/proto_breaking.yml b/.github/workflows/proto_breaking.yml index af03f99..5d2dd0b 100644 --- a/.github/workflows/proto_breaking.yml +++ b/.github/workflows/proto_breaking.yml @@ -10,6 +10,8 @@ jobs: env: BAZEL: bazelisk-linux-amd64 BAZELISK_VERSION: v1.19.0 + TMPDIRBASE: /tmp/compare/base + TMPDIRHEAD: /tmp/compare/head steps: - name: Checkout Code @@ -34,16 +36,15 @@ jobs: uses: actions/setup-python@v5 with: python-version: 3.11 - - name: Compare HEAD and BASE for breaking protobuf changes + - name: Set up proto-breaking-change-detector run: | python3 -m venv .venv source .venv/bin/activate pip install git+https://github.com/googleapis/proto-breaking-change-detector.git@v2.4.0 + - name: Build protobuf descriptors at HEAD and BASE + run: | HEAD="${{ github.event.pull_request.head.sha }}" BASE="$(git merge-base origin/main "${HEAD}")" - TMPDIR="$(mktemp -d -t compare-XXXX)" - TMPDIRBASE="${TMPDIR}/base" - TMPDIRHEAD="${TMPDIR}/head" extract_protodesc() { local TMPTARGETDIR @@ -68,9 +69,11 @@ jobs: extract_protodesc "${TMPDIRHEAD}" git checkout "${BASE}" bazel clean - BASETARGETS="$(bazel query 'attr("generator_function", "^proto_library$", "//...")' 2>/dev/null)" extract_protodesc "${TMPDIRBASE}" - + - name: Compare HEAD and BASE for breaking protobuf changes + run: | + source .venv/bin/activate + BASETARGETS="$(bazel query 'attr("generator_function", "^proto_library$", "//...")' 2>/dev/null)" ERROR=false for TARGET in ${BASETARGETS}; do TARGETDIR="$(echo "${TARGET}" | cut -d ":" -f1 | sed 's#^//##')"