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

Import results in new type definitions #251

Open
tuckermcclure opened this issue May 8, 2024 · 4 comments
Open

Import results in new type definitions #251

tuckermcclure opened this issue May 8, 2024 · 4 comments

Comments

@tuckermcclure
Copy link

When generating Julia definitions for multiple .proto files that each import a common .proto file, each of the generated modules gets its own definition of the "common" type. This makes it so that ProtoBuf definitions are very clumsy in Julia. Perhaps I'm misunderstanding how this is supposed to go.

Here's an example.

proto/common.proto

syntax = "proto3";

package Common;

message CommonThing {
    sint32 a = 1;
    string b = 2;
}

proto/a.proto

syntax = "proto3";

import "common.proto";

package A;

message AThing {
    Common.CommonThing f = 1;
}

proto/b.proto

syntax = "proto3";

import "common.proto";

package B;

message BThing {
    repeated Common.CommonThing g = 1;
}

script.jl

## Start fresh

run(`rm -rf "build"`)
mkdir("build")

## Generate the Julia files

using ProtoBuf
protojl(["common.proto", "a.proto", "b.proto"], "proto", "build")

## Include them

include("build/Common/Common.jl")
include("build/A/A.jl")
include("build/B/B.jl")

import .Common
import .A
import .B

## Make one of the common types

c = Common.CommonThing(1, "hi")

## Use it for A and B (these don't work)

a = A.AThing(c)
b = B.BThing([c,])

## What's it take to construct an AThing or BThing?

a2 = A.AThing(A.Common.CommonThing(1, "hi"))
b2 = B.BThing([B.Common.CommonThing(1, "hi"),])
# This is not helpful if I need to take something from A and give it to B.

Is there anything you can do to truly have common types? Have I misunderstood something fundamental either about ProtoBuf or about ProtoBuf.jl?

@Drvi
Copy link
Member

Drvi commented May 13, 2024

Hey @tuckermcclure. You are right -- the current way of including each generated "package" results in shared dependencies being re-defined in each of them, making them incompatible. This was supposed to be fixed by generating proper Julia packages (with the expected file structure and Project.toml) and loading them from the LOAD_PATH but we never got to it.

One workaround would be to put all of these into the same package:

package Common; -> package X.Common;
package A; -> package X.A;
package B; -> package X.A;

This way only a single package would be generated, X.jl, and the internal definitions would be set up correctly.

Sorry for this not being finished. Maybe the pre-1.0 version of this package worked better in this regard?

@tuckermcclure
Copy link
Author

Thank you, @Drvi ! Let me give this workaround a try.

Sorry for this not being finished.

I'm very thankful for this project as it stands today!

@tuckermcclure
Copy link
Author

Just to follow up, if I go modify common.proto, a.proto, and b.proto so that everything's in some top-level package (like X), then I generate code for x.proto, this does work as intended. That is, this will work:

c = Common.CommonThing(1, "hi")
a = A.AThing(c)
b = B.BThing([c,])

However, the actual proto files I'm working with are part of a large, shared code base. The Julia part is a very small piece. I can't force everyone to change all of the .proto files to pull things from some new top-level package.

I don't see another work-around right now.

@tuckermcclure
Copy link
Author

It seems to me that generating "real Julia packages" is not totally required to solve this problem (thought perhaps there are other things you were hoping to solve that way?). Potentially the generated code for a package could look in the parent module for the definitions it needs. E.g., instead of:

B.jl

module B
    include("../Common/Common.jl")
    include("b_pb.jl")
end

you could have:

module B
    import ..Common
    include("b_pb.jl")
end

For this simple example, if I make this change manually on both A.jl and B.jl, it works.

This implies that there is a new generated file that includes all of these things like so:

build/_pb.jl

include("build/Common/Common.jl")
include("build/A/A.jl")
include("build/B/B.jl")

such that the application code is just:

include("build/_pb.jl")

# Now this works:
c = Common.CommonThing(1, "hi")
a = A.AThing(c)
b = B.BThing([c,])

This clearly requires the code generator to figure out the hierarchy of imports so that the files can be included in the generated top-level file (_pb.jl here) in the right order. I'm not sure if that idea is invalid for some potential ProtoBuf structures. If it is a valid idea, it doesn't seem too hard to implement, and I might try it.

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

No branches or pull requests

2 participants