Skip to content
This repository has been archived by the owner on Mar 4, 2022. It is now read-only.

Include proto files from Go modules #498

Closed
fgrosse opened this issue Sep 13, 2019 · 8 comments
Closed

Include proto files from Go modules #498

fgrosse opened this issue Sep 13, 2019 · 8 comments

Comments

@fgrosse
Copy link

fgrosse commented Sep 13, 2019

I want to include Protobuf definitions from another repository. So far this works nicely when I have a vendor directory with the following prototool.yml:

protoc:
  version: 3.3.0

  includes:
    - vendor/example.com/foo/bar
    - vendor/example.com/foo/baz

Now I want to switch to Go modules (normal mode without vendor directory) and I am not sure how I can reference modules in the prototool includes directive. Is this a known use case somebody else already solved or is it simply not possible to include proto files from other Modules?

@smaye81
Copy link
Contributor

smaye81 commented Oct 15, 2019

If you specify an absolute path to your $GOPATH/pkg/mod directory it should work. Obviously not ideal, but right now that is the only way to specify an include path using non-vendored Go modules.

@smaye81
Copy link
Contributor

smaye81 commented Oct 15, 2019

We could maybe add a new config flag vendored: true/false which will look in your vendor directory if true and in pkg/mod if false.

So, the following would look in <projdir>/vendor for your includes:

protoc:
  version: 3.3.0
  
  includes:
    vendored: true
    - example.com/foo/bar
    - example.com/foo/baz

…

This would look in $GOPATH/pkg/mod for the includes

protoc:
  version: 3.3.0
  
  includes:
    vendored: false
    - example.com/foo/bar
    - example.com/foo/baz

…

@fgrosse
Copy link
Author

fgrosse commented Oct 15, 2019

Thanks for the tip with $GOPATH/pkg/mod directory. To use this however I would have to hard-code both the value of my concrete $GOPATH and also the version of the library in the proto tool now.

E.g. this works:

protoc:
  version: 3.3.0

  includes:
    - /home/fgrosse/pkg/mod/example.com/foo/bar@v0.6.1

Since this is not very practical a vendor mode in prototool would be very good. It should be possible given all information required can already be created using go list.

E.g. from the example, the following command would generate the path I used in the prototool.yml if I am inside a Go module that uses example.com/foo/bar

echo $GOPATH/pkg/mod/example.com/foo/bar@$(go list -m -f '{{.Version}}' example.com/foo/bar)
/home/fgrosse/pkg/mod/example.com/foo/bar@v0.6.1

@fgrosse
Copy link
Author

fgrosse commented Oct 15, 2019

Well actually looking into it, currently I don't see a way to access module information without having to execute go list (e.g. using os/exec) because the code is still an internal Go package. This would not be very elegant and I can understand that's not an approach prototool wants to follow.

However maybe we could be allowed to use environment variables in the imports?

diff --git a/internal/settings/config_provider.go b/internal/settings/config_provider.go
index e8fa72f..e4d6353 100644
--- a/internal/settings/config_provider.go
+++ b/internal/settings/config_provider.go
@@ -204,6 +204,8 @@ func externalConfigToConfig(develMode bool, e ExternalConfig, dirPath string) (C
        }
        includePaths := make([]string, 0, len(e.Protoc.Includes))
        for _, includePath := range strs.SortUniq(e.Protoc.Includes) {
+               includePath = os.ExpandEnv(includePath)
+
                if !filepath.IsAbs(includePath) {
                        includePath = filepath.Join(dirPath, includePath)
                }

Then we could reference the modules directory if we know the version:

protoc:
  version: 3.3.0

  includes:
    - $GOPATH/pkg/mod/example.com/foo/bar@v0.6.1

I would then put a CI validation script in place that will execute go list to check if I did not forget to update the version in the prototool.yaml in case dependencies get updated.

If that would be ok I can open the PR.

@smaye81
Copy link
Contributor

smaye81 commented Oct 15, 2019

This seems fine. Please make sure to update any docs to indicate that env variables are possible in imports.

@hit9
Copy link

hit9 commented Oct 30, 2019

Hello, how is this going?

@fgrosse
Copy link
Author

fgrosse commented Nov 1, 2019

Sorry, I got distracted and did not find time yet. Maybe I will have time on the weekend to return to this but if you want to submit a PR yourself go ahead :)

@smaye81
Copy link
Contributor

smaye81 commented Mar 10, 2020

Closing for now due to inactivity. Please reopen (or reference this issue) if a PR is created

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants