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

Improvements to running buf breaking for workspaces #3654

Open
2 tasks
doriable opened this issue Feb 28, 2025 · 3 comments
Open
2 tasks

Improvements to running buf breaking for workspaces #3654

doriable opened this issue Feb 28, 2025 · 3 comments
Assignees
Labels
Feature New feature or request

Comments

@doriable
Copy link
Member

Feature

Right now there are some pain points to running buf breaking for workspaces:

  • Support adding new modules with buf breaking #3641 -- when a new module is added to a workspace, buf breaking will error because there is an incongruence in the number of images between the input and the --against.
  • There is currently no way to run buf breaking for a workspace against the registry. The user must run buf breaking for each module.
    • Proposing a buf breaking --against-registry flag to address this, that would have the following behaviour:
      • When set, this would compare each module in the input to its registry counterpart. This will be based on the name key in the module config.
      • If there is no way to determine a module's registry (e.g. no name key is set or it is a pre-built image), then the module will be ignored.
      • If there are no commits for the module on the registry (e.g. a new module), then the module will be ignored.
@doriable doriable added the Feature New feature or request label Feb 28, 2025
@doriable doriable self-assigned this Feb 28, 2025
@mfridman
Copy link
Member

Really like the idea of being able to do a buf breaking against a registry. More than once I've wanted to compare the state of my current changes against whatever is on the default (main) label.

Mainly dropping a +1 for this.

@emcfarlane
Copy link
Contributor

emcfarlane commented Mar 4, 2025

As a more general solution we might be able to add a new input that resolves modules to the BSR. This would allow any command that takes an input to use it, and avoid the need for the new flag.

A workspace input would be required to resolve all named modules at a specific ref/label/commit to the BSR. It would also need the buf.lock file to resolve dependencies. So giving a file path like buf.yaml may not specify enough information to resolve the lock file. I suggest adding a new option ref to the dir input. This would cause all named modules to resolve against the BSR. For example valid inputs would be .#ref=main or path/to/workspace#ref=main. Local modules would ignore the ref option and resolve to the local directory. A breaking change would now look like:

buf breaking . --against=.#ref=main

An issue using the workspace input for breaking changes is that it won't capture module deletions. The workspace would resolve using the current configuration. For example if module B is removed between the main branch and the PR branch the previous .git input would see the deletion, but the new workspace resolution wouldn't see the change in the workspace config.

@doriable
Copy link
Member Author

doriable commented Mar 5, 2025

Re: adding a new input type

I like the overall idea of this, especially since workspaces are already a valid input type, it's more around better exposure and resolution of workspaces in different ways. I think we should invest some time this week to discuss what that means.

In terms of the implementation suggestions above, a couple of things:

I suggest adding a new option ref to the dir input.

So a workspace is already a dir input, and that is for local workspaces. I know this is a suggestion for basically using a local file to inform resolving a remote workspace, but the way our inputs are resolved, they're either Source or Module, and then we go from there. I think implementation-wise, I would treat this as a Module input. But I think this is a very minor point and we can discuss how to go about this if we are pursuing these changes. I do get the use of dir because of the string representation of the input being a path.

An issue using the workspace input for breaking changes is that it won't capture module deletions. The workspace would resolve using the current configuration. For example if module B is removed between the main branch and the PR branch the previous .git input would see the deletion, but the new workspace resolution wouldn't see the change in the workspace config.

Yeah, this is both a good thing and bad thing. If something was intentionally removed from a workspace's buf.yaml configuration, and now buf breaking against the previous source doesn't work, it's not a great experience. On the other hand, if configuration was unintentionally changed, then this is helpful. I think that if we choose to make our configuration the source of truth for our builds, then it's fine to stick by that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants