bufbuild / buf-action

Build, format, lint, and check for breaking changes in your Protobuf schemas, and automatically publish to the Buf Schema Registry.
https://buf.build
Apache License 2.0
19 stars 1 forks source link

Breaking check fails on new proto files #81

Open iCynara opened 4 days ago

iCynara commented 4 days ago

We have a GH action scan changed proto file and run bufbuild/buf-action on it. But if the PR only contain a new added proto file, breaking fails with error

Failure: no .proto files were targeted. This can occur if no .proto files are found in your input, --path points to files that do not exist, or --exclude-path excludes all files.

I think this is because breaking try to pull the same file on the older version which does exist.

Here is our original GH action

name: Protobuf Lint
on:
  pull_request:
    types: [opened, synchronize, reopened, labeled, unlabeled]
permissions:
  contents: read
  pull-requests: write
jobs:
  buf:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
        with:
          fetch-depth: 0
      - name: Get changed .proto files
        id: changed-protos
        run: |
          git fetch origin ${{ github.base_ref }}
          echo 'changed_files<<EOF' >> $GITHUB_OUTPUT
          git diff --name-only HEAD $(git merge-base HEAD origin/main) -- '*.proto' >> $GITHUB_OUTPUT
          echo 'EOF' >> $GITHUB_OUTPUT
      - uses: bufbuild/buf-action@3fb70352251376e958c4c2c92c3818de82a71c2b
        if: steps.changed-protos.outputs.changed_files != ''
        with:
          push: false
          paths: ${{ steps.changed-protos.outputs.changed_files }}

To fix that i did a work around (because i don't want people to use buf skip breaking if they have new files and modified files)

Separated changed files and new files in GH action. And add `breaking: false for new files.

  - name: Get new .proto files
        id: added-protos
        run: |
          git fetch origin ${{ github.base_ref }}
          echo 'added_files<<EOF' >> $GITHUB_OUTPUT
          git diff --name-only --diff-filter=A $(git merge-base HEAD origin/main)...HEAD -- '*.proto' >> $GITHUB_OUTPUT
          echo 'EOF' >> $GITHUB_OUTPUT
          echo $GITHUB_OUTPUT
      - uses: bufbuild/buf-action@3fb70352251376e958c4c2c92c3818de82a71c2b
        if: steps.added-protos.outputs.added_files != ''
        with:
          push: false
          breaking: false
          paths: ${{ steps.added-protos.outputs.added_files }}

My question is if this is what we suppose to do? Adding new file is very common and i feel like it should be natively supported and we just didn't use it right?

nicksnyder commented 1 day ago

You shouldn't need to set the paths: field at all, unless you have some special needs. Can you see if removing the path: field solves your problem?

emcfarlane commented 9 hours ago

@iCynara I agree with Nick and think the paths restriction here is what is causing issues. There shouldn't be a need to run checks on only the changeset. Using the paths will restrict the module build input and I believe cause the breaking changes as files not modified will appear to be deleted. Adding new files is not a breaking change. Could you provide a bit more details on why you needed to set paths?