protobuf-d icon indicating copy to clipboard operation
protobuf-d copied to clipboard

Fix module import statements and nested type fields in protoc-gen-d.

Open ShigekiKarita opened this issue 4 years ago • 6 comments

Protoc on this file: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/protobuf/debug_event.proto generates broken code like this

// Generated by the protocol buffer compiler.  DO NOT EDIT!
// source: tensorflow/core/protobuf/debug_event.proto

module tensorflow.debug_event;

import google.protobuf;
import tensorflow.core.framework.tensor;
import tensorflow.core.protobuf.graph_debug_info;

...

struct StackFrameWithId
{
    @Proto(1) string id = protoDefaultValue!string;
    @Proto(2) FileLineCol fileLineCol = protoDefaultValue!FileLineCol;
}

There are two kinds of errors.

1 import module name is not equal to generated files

For example, this line is invalid:

import tensorflow.core.protobuf.graph_debug_info;

It should be import tensorflow.graph_debug_info because it is generated as {package}/{proto basename}.d.

2 nested type field declaration omits its outer class

This line contains an invalid type name:

    @Proto(2) FileLineCol fileLineCol = protoDefaultValue!FileLineCol;

The FileLineCol must be GraphDebugInfo.FileLineCol because the message is nested https://github.com/tensorflow/tensorflow/blob/161381e3379c9c5bb47acd994838850efde7a385/tensorflow/core/protobuf/graph_debug_info.proto#L11-L13

ShigekiKarita avatar Jun 15 '21 10:06 ShigekiKarita

Thank you for the contribution! Could you add some unit-tests to document and verify the use-case?

dcarp avatar Jun 15 '21 18:06 dcarp

OK. I added a proto intest, and testing script for travis. However, travis did not detect the change.

ShigekiKarita avatar Jun 16 '21 03:06 ShigekiKarita

image Oh travis-ci.org ended yesterday. Can you migrate this repo to travis-ci.com or something else?

ShigekiKarita avatar Jun 16 '21 10:06 ShigekiKarita

FYI I confirmed that travis-ci.com worked on my side https://travis-ci.com/github/ShigekiKarita/protobuf-d/jobs/514841261#L1096

ShigekiKarita avatar Jun 16 '21 14:06 ShigekiKarita

Thanks for the review comments. I believe everything is done.

ShigekiKarita avatar Jun 17 '21 02:06 ShigekiKarita

Hi @dcarp, I just noticed that you replaced Travis with GitHub action recently. And I merged the master branch. Can you approve running the workflows?

ShigekiKarita avatar Mar 17 '22 07:03 ShigekiKarita