Open
Description
Buffrs does not allow underscores in the package.name
field and recommends hyphens as a word separator. On the other hand, the Protobuf package ...;
directive does not accept hyphens and underscores can be used as a word separator.
This leads to inconsistencies between the package
and import
directive.
# Proto.toml
[package]
name = "foo-bar"
type = "api"
version = "0.1.0"
// proto/service.proto
package foo_bar;
import "foo-bar/messages.proto";
This is because Buffrs will construct the proto/vendor
folder as
proto/vendor/
foo-bar/
service.proto
messages.proto
I would argue that this is inconsistent and confusing for no benefit and that Buffrs should instead align with the Protobuf specification; if not for the Buffrs package.name
, at least for the generated directory structure.
Metadata
Assignees
Labels
Issues or ideas which require some discussion, research and more implementation workEverything related to the buffrs cliChanges related to the DatamodelPlease dont work on this if you can contribute something with a higher priorityChanging the inner-workings of buffrsRelated to personal or community opinions
Activity
mara-schulke commentedon Mar 22, 2024
FWIW this would be a huge breaking change to everyone using buffrs at the moment (e.g. require manual intervention in every single project)
Tpt commentedon Mar 22, 2024
Can't this be gated on the edition? If the package
foo_bar
of any package depending on it is usingedition = 0.8
, thenproto/vendor/foo-bar/
is generated and the packagefoo_bar
of any package depending on it is usingedition = 0.9
thenproto/vendor/foo_bar/
is generated? This way alledition = 0.8
packages can use theimport foo-bar
syntax and alledition = 0.9
packages theimport foo_bar
syntax. This might lead to some duplication in thevendor
directory during the migration but it shouldn't be a big dealNiklasRosenstein commentedon Mar 22, 2024
A migration path could also be to generate both folders (i.e. the old one symlinking to the new one).
mara-schulke commentedon Mar 25, 2024
@Tpt yes, but I dont see huge value in this change other than it being a style issue at the moment?
It works reliably and doesn't cause any issues hence im hesitant on spending time on this as it also would break existing projects (one would need to migrate all proto file imports manually, even with editions).
@NiklasRosenstein what is the motivation behind this? Is it related to python support?
heatonmatthew commentedon Mar 25, 2024
For an additional perspective, I've introduced Buffrs at my company and this inconsistency was something I needed to document well for my teams in our usage guidelines (as it wasn't intuitive). We basically just accepted it as "how things were done" for this tool.
If it were to change, my perspective would be to suggest:
-
) and underscore (_
) in the Proto.toml package name-
to_
)That seems like it would provide an opt-in migration path. But there might be other restrictions in how the Proto.toml file is consumed which means it can't accept underscores in the package name.
kixa commentedon Mar 25, 2024
FYI no underscore support breaks our current legacy generate-and-package JS (via: https://github.com/protobufjs/protobuf.js) setup too since the generated code uses relative imports and expects
package ...
to match the folder name. I.e. Given (for example)package my_proto
(in the proto),package.name: my-proto
(inproto.toml
) and generating stubs fromvendor/
after install, the output will contain (for example)import * from ../my_proto
, but since the folder name ismy-proto
, every import fails.poliorcetics commentedon Apr 19, 2024
We could introduce a change in the form of:
That way project that don't have the issue would just continue working and those that do have the escape hatch
Somewhat prior art:
directory
to support installing in a different directory than 'package-name' #236dgehriger commentedon Jun 25, 2024
Hi @mara-schulke. I have been looking into
buffrs
and it really looks very promising. However, I'm also suffering from the fact that thepackage
statements don't allow dashes, whilebuffrs
Proto.toml files require them.As pointed out many times by others, this forces one to:
name = api-examples-hub
package api_examples_hub
import "api-examples-hub/foo.proto"
api_examples_hub.Type
From your comments, it seems that this is actually a feature in your setup. Maybe I misunderstand how you are using
buffrs
in your organization. My suspicion is that you aren't using hierarchical names. Is that the case? If not, why this restriction? Simply allowing.
in the Proto.toml name would solve the problem, right?