dcarp / protobuf-d

Protocol Buffers Compiler Plugin and Support Library for D
Boost Software License 1.0
38 stars 9 forks source link

Wrong import paths in generated files #13

Closed deviator closed 5 years ago

deviator commented 5 years ago

I have 2 files: one.proto

syntax = "proto3";
package msg;

message OneData {
    bool input = 1;
}

ppp.proto

syntax = "proto3";
package msg;

import public "one.proto";

message Data {
    repeated OneData one = 1;
}

For generate I use this command

protoc -I./proto --plugin=protobuf-d/build/protoc-gen-d --d_out=source proto/ppp.proto

and get in source/msg 2 output files

// Generated by the protocol buffer compiler.  DO NOT EDIT!
// source: one.proto

module msg.one;

import google.protobuf;

enum protocVersion = 3005000;

class OneData
{
    @Proto(1) bool input = protoDefaultValue!bool;
}
// Generated by the protocol buffer compiler.  DO NOT EDIT!
// source: ppp.proto

module msg.ppp;

import google.protobuf;
import one; // <- at this point wrong import of msg.one module

enum protocVersion = 3005000;

class Data
{
    @Proto(1) OneData[] one = protoDefaultValue!(OneData[]);
}

Then I compile I get error message:

source/msg/ppp.d(7,8): Error: module `one` is in file 'one.d' which cannot be read
dcarp commented 5 years ago

From proto3 documentation

The protocol compiler searches for imported files in a set of directories specified on the protocol compiler command line using the -I/--proto_path flag. If no flag was given, it looks in the directory in which the compiler was invoked. In general you should set the --proto_path flag to the root of your project and use fully qualified names for all imports.

Will this work? ppp.proto

syntax = "proto3";
package msg;

import public "msg/one.proto"; // <-- msg/ prefix

message Data {
    repeated OneData one = 1;
}
deviator commented 5 years ago

Will this work?

No =(

msg/one.proto: File not found.
ppp.proto: Import "msg/one.proto" was not found or had errors.
ppp.proto:7:14: "OneData" is not defined.
deviator commented 5 years ago

Solution: use package name same as name of directory what contains .proto files. My first try:

├── proto
│   ├── one.proto
│   └── ppp.proto
└── source
    ├── app.d
    └── msg
        ├── one.d
        └── ppp.d

need to change to:

├── msg
│   ├── one.proto
│   └── ppp.proto
└── source
    ├── app.d
    └── msg
        ├── one.d
        └── ppp.d

and at ppp.proto change import string to import public "msg/one.proto"; and change build command to protoc --plugin=protobuf-d/build/protoc-gen-d --d_out=source msg/ppp.proto

ghost91- commented 5 years ago

@dcarp while it indeed seems to be discouraged to have a different directory structure than the package structure, it works perfectly fine for other languages. E.g. the java output of the above example is something like the following:

├── msg
│   ├── One.java
│   └── Ppp.java

msg/Ppp.java:


package msg;

public final class Ppp {
  private Ppp() {}
  public static void registerAllExtensions(
      com.google.protobuf.ExtensionRegistryLite registry) {
  }

  public static void registerAllExtensions(
      com.google.protobuf.ExtensionRegistry registry) {
    registerAllExtensions(
        (com.google.protobuf.ExtensionRegistryLite) registry);
  }
  public interface DataOrBuilder extends
      // @@protoc_insertion_point(interface_extends:msg.Data)
      com.google.protobuf.MessageOrBuilder {

    /**
     * <code>repeated .msg.OneData one = 1;</code>
     */
    java.util.List<msg.One.OneData> 
        getOneList();
/* ... */

I also think that it is (unfortunately) quite common practice to do it like this. At least at my work place it is done like that. So I think that we should support this.