JuliaIO / ProtoBuf.jl

Julia protobuf implementation
Other
205 stars 55 forks source link

Import results in new type definitions #251

Open tuckermcclure opened 4 months ago

tuckermcclure commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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.