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

Fix proto imports cannot see other proto folders #111

Closed
AdrianRaFo opened this issue Jul 11, 2019 · 7 comments
Closed

Fix proto imports cannot see other proto folders #111

AdrianRaFo opened this issue Jul 11, 2019 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@AdrianRaFo
Copy link

@SemanticBeeng discovered an issue on the Protobuf imports where if we have 2 folders with proto files even on the same module we are not able to import from one folder to another (you can see the diff here ).

The real problem is we are facing here is that we are missing the -I argument on the Protobuf compilation which should allow us to add the other folder on our compilation AFAIK.

@SemanticBeeng
Copy link

SemanticBeeng commented Jul 11, 2019

There might be 2-3 related problems.

Have a multi-module sbt project with few services sharing a domain model,
Using protobuf for each, including the domain model which is in a separate module; service modules depend on it.

The first issue is making the dependency work for both protobuf includes / compile and generated Scala code. (above from Adrian)

Had to use symbolic links to the shared domain model proto file.
This makes protoc work based on the import : see "incl" folder.

Using these settings

idlType := "proto",
srcGenJarNames ++= Seq("authentication_module_shared")

image

Second issue: this is how generated code looks like:

image

The third issue : Scala classes generated from shared module are not visible to sbt build (getting compile errors; Intellij sees them).
UPDATE: Adding this to settings (as per Adrian) solved this issue

sourceGenerators in Compile += (srcGen in Compile).taskValue

UPDATE : this is how the correct generated service should look like

image

shared_domain is the package name - first line I fixed manually.
incl/shared_domain.proto is the file name : seems the plugin / skeumorph mixes the two. 🤔

@AdrianRaFo this is the only blocking issue; Is this a plugin issue or a skeumorph one?

Used muRPC versions 0.18.0 and 0.18.4 / skewmorph 0.0.11

@AdrianRaFo
Copy link
Author

I guess is a skeuomorph one but people in charge of this could verify it

@bilki
Copy link
Contributor

bilki commented Jul 12, 2019

I'm taking a look at this. My first impression for imports is that we need to pipe srcGenSourceDirs (or even better, srcGenIDLTargetDir) from Mu gen plugin to skeuomorph, maybe adjusting paths, as @AdrianRaFo suggested.

@bilki
Copy link
Contributor

bilki commented Jul 12, 2019

Ok, I have located the place where slash should be replaced with dots (or whatever other adjustments that we may need to do): https://github.com/higherkindness/skeuomorph/blob/master/src/main/scala/higherkindness/skeuomorph/protobuf/ParseProto.scala#L117

Let me test a bit more and I will open a PR fixing these bugs.

@SemanticBeeng
Copy link

SemanticBeeng commented Jul 12, 2019

@bilki thanks - please also check if these soft links can be avoided.

Cannot see how unless mu passes down to skeumorph knowledge about inter-module dependencies.
Event then work in Intellij would not be nice (without the proto file softlinks and imports)

@bilki
Copy link
Contributor

bilki commented Jul 15, 2019

@SemanticBeeng I don't think we could get rid of those soft links :( Anyway, check the new PRs that solve the problem of the nested folders containing imported proto files.

@juanpedromoreno
Copy link
Member

Closed by #115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants