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

buf breaking errors with workspaces containing different directories #567

Closed
amckinney opened this issue Sep 18, 2021 · 7 comments
Closed
Labels
Bug Something isn't working

Comments

@amckinney
Copy link
Contributor

Several users have been bitten by the "Failure: input contained 2 images, whereas against contained 1 images" error when running buf breaking with workspaces. Let's revisit this and see if we can do anything about it.

@amckinney
Copy link
Contributor Author

I did a bit more investigation into this one. At first glance, it seems like we can just merge the images and compare the current snapshot with the --against snapshot, but this breaks down with respect to the breaking configuration configured for each image.

If we were to do this, we need a way to assign each configuration to different portions of the image, and each file contained in the image would only have a single configuration. This gets complicated in a variety of cases, such as files that are present in the current snapshot, but not in the --against snapshot. In general, these files should only introduce new types, but it's possible that they redeclare and/or break types that are already found in other files in the same package. We'd need to compare these files against the merged --against image (at least for files that belong to the same package).

I'll continue to look into this and see what we can do.

@amckinney amckinney removed their assignment Dec 7, 2021
@andrewparmet
Copy link

andrewparmet commented Dec 18, 2021

I was just bitten by this in my Buf Gradle plugin integration.

The way the plugin works is that it uses Buf to create a JSON image and then breakage checks run against images from previous versions. I recently modified the plugin to use workspaces with symlinks to protobuf source directories instead of copying protobuf source into a single directory and now breakage checks fail with this error, even when run against images generated by the same version (using workspaces with symlinks).

Given that I want to check compatibility against a published artifact, is there a better way to achieve this workflow than publishing a JSON image with Buf itself? Should I just export the protobuf directories as an archive? I feel like this is a use case that images are intended for but I notice the documentation on using images for breakage checks seems to have vanished.

Here's a branch with a failing test. The test can be executed by running ./gradlew test --tests 'BreakingTest.schema with multi-directory workspace'.

The test runs against the sample project defined here and creates a temporary buf.work.yaml that looks like this:

version: v1
directories:
  - src-main-proto
  - build-extracted--include--protos-main

where src-main-proto is a symlink to src/main/proto and build-extracted--include--protos-main is a symlink to the directory where the protobuf-gradle-plugin extracts its protobuf, build/extracted-include-protos/main.

@amckinney
Copy link
Contributor Author

amckinney commented Dec 20, 2021

I've explored a couple options for this. From what I can tell, you really just want to verify compatibility with respect to the compiled artifact (the image) created from the workspace as a whole - is that right?

If you don't care about the potentially different breaking configurations specified in each of the module's buf.yaml, you can simply compile each workspace into a separate Image and compare them with either the default breaking configuration, or one tailored to match all of the files contained in the workspace.

For example, suppose that against.bin is the Image you're comparing against and image.bin is the target Image. They can be created like so:

$ buf build -o image.bin
$ buf build .git#branch=main -o against.bin

If you are OK with the default FILE configuration, you can just use the following command:

$ buf breaking image.bin --against against.bin

If you care about the individual breaking configurations specified in each of the workspace modules' buf.yaml, you'd need to adapt this to something like the following:

$ buf breaking image.bin --against against.bin --config path/to/buf.yaml
$ buf breaking image.bin --against against.bin --config path/to/other/buf.yaml

Those buf.yaml configurations will only apply to specific files in the compiled image (the files that belong to the corresponding workspace directory), so you'd probably see false negatives/positives. One buf.yaml might complain about the files that belong to and are ignored by the other buf.yaml.

Depending on your configuration, you could merge the breaking configurations in each buf.yaml and apply it uniformly.

$ buf breaking image.bin --against against.bin --config path/to/merged/buf.yaml

Ideally all of this is automated behind the buf breaking command. For now, we require that the workspaces directories match up so that each buf.yaml configuration can be applied as needed.

With all this said, I'll continue to investigate how we can implement this behavior to support all of these requirements, but hopefully this unblocks you in the meantime!

@andrewparmet
Copy link

I think I'm following what you're saying, but there's still a piece I don't understand. The --against image I'm using was built from with the same buf.yaml as the local workspace in which the protobuf code has not changed - so why should the generated image be incompatible (unusable) against the workspace from which it was generated?

@jan-xyz
Copy link

jan-xyz commented Mar 17, 2022

What would be a graceful path to adding new directories to the buf.work.yaml? We have automated PR checks for breaking change detection and I am a bit hesitant now to overrule a change that adds a new directory to the workspace and is failing with this error. I would rather like to have a graceful path.

@amckinney
Copy link
Contributor Author

It's not ideal, but you could adapt your workflow to always compare against Images (as described above). You could do it in a one-liner like so:

$ buf build -o current.bin; buf build .git#branch=main -o against.bin; buf breaking current.bin --against against.bin

This is only a problem if each of the directories in your buf.work.yaml specify a different breaking configuration in their buf.yaml (again, described above).

Alternatively, you could compare compatibility between each of the directories listed in buf.work.yaml independently. So if you had two directories in your buf.work.yaml, you would run separate buf breaking commands for each one. You could do this with separate steps in your GitHub Actions workflow with two calls to buf-breaking-action, or even in a custom script.

version: v1
directories:
  - proto
  - vendor

GitHub Actions

    steps:
      - uses: actions/checkout@v3
      - uses: bufbuild/[email protected]
        if: success()
        with:
          version: 1.1.0
      - uses: bufbuild/[email protected]
        if: success()
        with:
          input: proto
          against: 'https://github.com/<your_org>/<your_repo>.git#branch=main,ref=HEAD~1,subdir=proto'
          buf_token: ${{ secrets.BUF_TOKEN }}
      - uses: bufbuild/[email protected]
        if: success()
        with:
          input: vendor
          against: 'https://github.com/<your_org>/<your_repo>.git#branch=main,ref=HEAD~1,subdir=vendor'
          buf_token: ${{ secrets.BUF_TOKEN }}

Custom Script

for directory in proto vendor; do
  buf breaking ${directory} --against .git#branch=main,subdir=${directory}
done

Then if you ever need to add another directory to your buf.work.yaml, you would add it to the script you run in CI after the commit that adds a new directory has been merged.


I admit that neither of these options are ideal, they are just workarounds for the time being. We'll try to have a better answer here soon, but other things have taken precedence. Please let us know if this helps you in the meantime!

@doriable
Copy link
Member

buf breaking will fundamentally require some congruency between the input and --against. If the input is a workspace, and there is a difference in the number of modules between the workspace input and --against, e.g. a module is being added, then the workaround for the initial breaking check is to target specific modules, e.g. buf breaking proto/foo --against .git#branch=main,subdir=proto/foo, as noted by the workaround.

andrewparmet added a commit to bufbuild/buf-gradle-plugin that referenced this issue Sep 20, 2024
…no stdout (#241)

This would allow the `buf breaking` [test that requires Buf
1.31.0](https://github.com/bufbuild/buf-gradle-plugin/pull/213/files#diff-dbe62edf00cb02d5ffc116d39393f9a9dd0a2f2cccb2d92b5f6d8ba529d16b03R19)
to fail with an actual error message rather than silence.

When this line is removed, the test now fails with the correct error
message, which is this one: bufbuild/buf#567
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants