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

Add .buffrs/config.toml and allow dots in package names #251

Closed
wants to merge 17 commits into from

Conversation

dgehriger
Copy link

@dgehriger dgehriger commented Jun 26, 2024

Based on upstream version 0.8.1 but with following additions:

  • Introduction of a configuration file in .buffrs/config.toml (in the current working directory, or any parent)
  • Addition of flag for buffrs install --only-dependencies to only install dependent packages into the vendor folder. This allows the buf linter to be run on the main proto files of a package without triggering errors due to duplicate definitions.
  • Config file allows specifying:
    • List of registry URLs and their names
    • A default registry
    • Changed paths for protos and vendor folders
    • Use of . in package names for hierarchical organization of protos in the vendor folder

Furthermore, this release allows use of . in the package name of the Proto.toml

Example:

[store]
proto_path = "proto"
proto_vendor_path = "proto/vendor"
hierarchical_packages = true

[registries]
some_org = "https://artifactory.example.com/artifactory/some-org"

[registry]
default = "some_org"

@dgehriger dgehriger changed the title Add .buffrs/config.toml registries Add .buffrs/config.toml and allow dots in package names Jun 26, 2024
@mara-schulke
Copy link
Contributor

Can we split the features here into multiple MRs?

Eg. I'm happy to accept the deps only flag fairly instantly whereas I have concerns about the implications of the config file -> This needs to be thoroughly designed for not hurting dependency resolution (ie. what happens when you declare different registries than your consumers? How do you deal with diverging default registries?)

@mara-schulke
Copy link
Contributor

I disagree with the fact that this should be configurable by the user! I'm happy to change the default behavior of buffrs to hierarchical packages if there is a really well reasoned and thoughtful solution proposed!

Ie. I see three different features / MRs here:

  • Deps only install
  • Config file for registry
  • Hierarchical packages / import system

@dgehriger
Copy link
Author

@mara-schulke : yes, you are right. And on top of that, I think that the correct solution isn't about interpreting package names as hierarchical, but instead to allow the published packages to have their protos in subfolders. That way, one can define e.g. a package named api_example_hub (or using dashes), whose contents are:

- Proto.toml
  +- proto/
       +- api/
            +- example/
                 +- hub/
                      +- pkg.proto

In fact, such a package could contain proto files anywhere below proto. However, when doing a buffrs install, this should end up in the vendor folder as:

+- vendor/
     +- api
          +- example
               +- hub
                     +- pkg.proto

Note that I intentionally:

  1. Do not include neither the package name nor a proto folder in the vendor folder. This allows buf to see the correct hierarchy.
  2. Do not include the _Proto.toml_s of the dependent packages either. They aren't needed and would end up in the same location (just below the archive folder)
  3. Do not include the archive folder below proto because doing so again would prevent me from telling buf to lint the original protos (under proto) and the dependent packages (under vendor).

@dgehriger dgehriger marked this pull request as draft June 26, 2024 12:54
@dgehriger
Copy link
Author

Can we split the features here into multiple MRs?

Eg. I'm happy to accept the deps only flag fairly instantly whereas I have concerns about the implications of the config file -> This needs to be thoroughly designed for not hurting dependency resolution (ie. what happens when you declare different registries than your consumers? How do you deal with diverging default registries?)

The config file is hierarchical -- so one can simply override the config file by defining another one a level below.

@dgehriger
Copy link
Author

I disagree with the fact that this should be configurable by the user! I'm happy to change the default behavior of buffrs to hierarchical packages if there is a really well reasoned and thoughtful solution proposed!

Ie. I see three different features / MRs here:

  • Deps only install
  • Config file for registry
  • Hierarchical packages / import system

Fair enough, and I'll try to submit these in 3 separate PRs.

The deps-only + import system actually go together. The final purpose being to make the result compatible with buf's linter & breaking change detection.

@dgehriger
Copy link
Author

Closing in favor of PR #252 and PR #253

@dgehriger dgehriger closed this Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants