ninenines / erlang.mk

A build tool for Erlang that just works.
https://erlang.mk
ISC License
580 stars 238 forks source link

Building .proto files with imports fails #860

Closed Bathtor closed 1 year ago

Bathtor commented 5 years ago

It seems that building protobuf files that have imports on other files in the same folder fails to find the imported file. Something is wrong in the path where it looks for the imports.

I added tests for both gpb and protobuffs, and both show the same behaviour.

In fact I managed to get it to work for gpb by putting the imported file also on the projects top level, because gpb looks for ./<importfile>.proto and it seems the ./ is top level not src/ when it is invoked.

It's probably an easy fix, I'm just terrible with Makefiles.

essen commented 5 years ago

Based on those two paragraphs we are doing it wrong: https://github.com/tomas-abrahamsson/gpb/blob/master/src/gpb_compile.erl#L268

Bathtor commented 5 years ago

Based on those two paragraphs we are doing it wrong: https://github.com/tomas-abrahamsson/gpb/blob/master/src/gpb_compile.erl#L268

Hmm, yeah, so the following change at least gets rid of the not found error for gpb:

diff --git a/plugins/protobuffs.mk b/plugins/protobuffs.mk
index b4f9ce6..d56c2e6 100644
--- a/plugins/protobuffs.mk
+++ b/plugins/protobuffs.mk
@@ -43,7 +43,8 @@ define compile_proto.erl
                        {include_as_lib, true},
                        {module_name_suffix, "_pb"},
                        {o_hrl, "./include"},
-                       {o_erl, "./src"}])
+                       {o_erl, "./src"},
+                       {i, "./src/proto"}])
        end || F <- string:tokens("$1", " ")],
        halt().
 endef

But a) this is just for the single concrete folder I'm using the test and not all folders that contain .proto files and b) the import still doesn't seem to be working quite right for me afterwards, though that may be a gpb issue instead?

essen commented 5 years ago

The file given as first argument also needs to be without the path. As for other folders, maybe just specify the option more than once for all the folders where proto files were found.

Bathtor commented 5 years ago

The file given as first argument also needs to be without the path. As for other folders, maybe just specify the option more than once for all the folders where proto files were found.

Right. I have no idea how to do either of that^^ Maybe you could give it a try when you have some time? Would be much appreciated :)

essen commented 5 years ago

I'll see if I have time before vacation starts, otherwise probably after.

benjamin-bergia commented 2 years ago

Sorry for digging this up but I encountered the exact same issue yesterday. I modified the import from my proto files from: import "test.proto" to import "src/test.proto" for it to work. I also added {use_packages, true} to the list of gpb options since some of the files use packages.

essen commented 1 year ago

Thanks for the hints. Based on both of you's comments, I added the use_packages option to gpb and also add {i, ...} for each folder containing a .proto file. This makes the tests pass. I'll push it later today. Cheers!