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

Go package imported twice if proto package split into multiple files #330

Open
lrstanley opened this issue Aug 15, 2021 · 3 comments
Open
Labels
feature request pending Keeps issues from going stale

Comments

@lrstanley
Copy link

lrstanley commented Aug 15, 2021

If this is intended behavior, my bad. I tried searching through issues and I couldn't find much.

I have the following proto file (where I have just services defined):

// file services.proto
syntax = "proto3";

package rpc;
option go_package = "github.com/lrstanley/spectrograph/internal/rpc";

import "google/protobuf/empty.proto";
import "internal/models/protos/servers.proto";
import "internal/models/protos/worker.proto";

service Worker {
    rpc Health(google.protobuf.Empty) returns (models.WorkerHealth); // models.WorkerHealth being from worker.proto
    rpc UpdateServer(models.ServerDiscordData) returns (google.protobuf.Empty); // models.ServerDiscordData being from servers.proto
    rpc UpdateServerStatus(models.ServerStatus) returns (google.protobuf.Empty); // models.ServerStatus being from servers.proto
}

It has two imports:

import "internal/models/protos/servers.proto";
import "internal/models/protos/worker.proto";

Both of these imports are within the same directory of eachother, but not in the directory of the services/twirp files, and both actually specify the same go_package and package line, as they're only split into multiple files given the size and specific-scope of the messages:

// file: servers.proto
syntax = "proto3";

package models;
option go_package = "github.com/lrstanley/spectrograph/internal/models";

[... server-specific messages defined below...]
// file: worker.proto
syntax = "proto3";

package models;
option go_package = "github.com/lrstanley/spectrograph/internal/models";

[... worker-specific messages defined below...]

I am able to import both and use them successfully. However, I noticed the generated *.twirp.go files have:

import models "github.com/lrstanley/spectrograph/internal/models"
import models1 "github.com/lrstanley/spectrograph/internal/models"

And reference the same go_package as multiple imports throughout the file, which can be rather confusing.

This works successfully (it's supported behavior by protobuf, and from my understanding, not discouraged), however I'm wondering if we can add some logic that ensures if there are multiple imports with the same exact go package name and path, that only one line is imported?

The more serious issue I suspect is: if the current behavior could cause issues for those that use init() functions in their Go packages, where the *.pb.go files are generated, as I suspect these would get executed twice. I.e. if variables or similar are initialized, this could cause weird state issues for users, where two sets of initialized state are used.

@wmatveyenko
Copy link
Contributor

Hi. Thanks for the report. We have added an item to our backlog to look into this.
--Wade

@hemanth132
Copy link

A golang package is initialized only once even if it's imported multiple times by different packages. So the init() function being executed twice shouldn't happen.

@wmatveyenko wmatveyenko added feature request pending Keeps issues from going stale labels Nov 1, 2021
@na4ma4
Copy link

na4ma4 commented Jun 5, 2024

If you use /foo/bar/directory/*.proto (bash glob, so if using something like mage, you'll need to expand it first) when you compile the source it should only import it once.

Not a fix, but a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request pending Keeps issues from going stale
Projects
None yet
Development

No branches or pull requests

4 participants