Implement a feature to generate a dependency file #193

TeBoring · 2015-02-04T05:56:58Z

By giving protoc the flag
"--dependency_out=FILE", protoc will write dependencies of input proto files into FILE. In FILE, the format will be: ^output_file_path: input1_path\n input2_path$
Whether path is relative or absolute will be based on the input on command line.
This cl is based on google#178

…nd make Use --manifest-file=somefile.d to output the dependency manifest. This file will contain a list of files which were read by protoc as part of creating the output files. It doesn't include the plugin inputs if plugins are used, that could be a later extension. The manifest file is in the format <output file>: <input files>. The manifest file format only allows you to specify one output file, which isn't a problem as it's used to detect input changes in order to detect when to rerun the protoc command. The output file used in the manifest is the manifest filename itself; to use this in ninja you should declare the manifest file as the first output as well as the depfile input.

…he flag "--dependency_manifest_out=FILE", protoc will write dependencies of input proto files into FILE. In FILE, the format will be <full path to FILE>: <full path to 1st proto>\\\n <full path to 2nd proto> ... This cl is based on protocolbuffers#178

googlebot · 2015-02-04T05:56:59Z

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

TeBoring · 2015-02-04T06:18:10Z

I have signed CLA

googlebot · 2015-02-04T06:18:12Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

TeBoring · 2015-02-04T06:21:34Z

@rgeary1 to review

test-driver

dmaclach · 2015-02-04T20:37:32Z

src/google/protobuf/compiler/command_line_interface.cc

@@ -1012,6 +1019,22 @@ CommandLineInterface::InterpretArgument(const string& name,
    }
    descriptor_set_name_ = value;

+  } else if (name == "--dependency_out") {
+    if (!dependency_manifest_name_.empty()) {


Make var name (dependency_manifest_name_) match argument name somewhat (dependency_out_name_?)

dmaclach · 2015-02-04T22:33:13Z

src/google/protobuf/compiler/command_line_interface_unittest.cc

+  ExpectNoErrors();
+
+  ExpectFileContent("manifest",
+                    "$tmpdir/manifest: $tmpdir/foo.proto\\\n"


normally the file would be:

<outputfile1>; <outputfile2>; ... : <dependency1>; \\ <dependency2>; \\ <dependency3>; \\ ...

where for a main.c of:

#include "main.h" int main() { return 0; }

cc -MMD main.c

you are going to get:

main.o: main.c main.h

our code is not going to set <outputfile> correctly. I would expect for the cpp compiler that we would actually generate

foo.pb.cc foo.pb.h: foo.proto

Thoughts? I think the current code will actually work for my needs, but it is certainly not a correct solution as far as I can tell.

I'm working from "http://make.mad-scientist.net/papers/advanced-auto-dependency-generation/".

foo.pb.cc foo.pb.h: foo.proto

It looks good. However, protoc doesn't know generated file is named foo.pb.cc and foo.pb.h

If code generator/plugin can tell protoc the path of its generated files, protoc can make it.

On Wed, Feb 4, 2015 at 2:33 PM, dmaclach notifications@github.com wrote:

In src/google/protobuf/compiler/command_line_interface_unittest.cc
#193 (comment):

"syntax = "proto2";\n"

"message Foo {}\n");

CreateTempFile("bar.proto",

"syntax = "proto2";\n"

"import "foo.proto";\n"

"message Bar {\n"

" optional Foo foo = 1;\n"

"}\n");

Run("protocol_compiler --dependency_out=$tmpdir/manifest "

"--test_out=$tmpdir --proto_path=$tmpdir bar.proto");

ExpectNoErrors();

ExpectFileContent("manifest",

"$tmpdir/manifest: $tmpdir/foo.proto\\n"

normally the file would be:
... :

...

where for a main.c of:
#include "main.h"

int main() {
return 0;
}

cc -MMD main.c

you are going to get:

main.o: main.c main.h

our code is not going to set correctly. I would expect for the cpp
compiler that we would actually generate

foo.pb.cc foo.pb.h : foo.proto

Thoughts? I think the current code will actually work for my needs, but it
is certainly not a correct solution as far as I can tell.

Protoc has one difference from gcc: protoc can generate different set of
files for different languages, where as gcc is tied to C/C++. This means
the output of protoc varies from language. And sometimes it's may not be
tied with any language at all. For example, users may just want to find all
imports of a certain .proto file and build a protodb for them. For this
reason I think it's better to avoid any language specific files in the
generated dependency file and make it a general purpose dependency file
that could be used for any language.

I'm working from "

http://make.mad-scientist.net/papers/advanced-auto-dependency-generation/
".

—
Reply to this email directly or view it on GitHub
https://github.com/google/protobuf/pull/193/files#r24126637.

rgeary1 · 2015-02-04T23:38:09Z

If the spec includes support for multiple output files in the depfile and it's supported by make, then I suspect this is a ninja implementation issue. I'll email the ninja mailing list to find out their reasons.

TeBoring · 2015-02-19T09:10:49Z

@rgeary1, how's the CLA going?

rgeary1 · 2015-02-19T16:35:25Z

Still working on it. I've asked our legal team for an ETA.

On 19 February 2015 at 04:11, Paul Yang notifications@github.com wrote:

@rgeary1 https://github.com/rgeary1, how's the CLA going?

—
Reply to this email directly or view it on GitHub
#193 (comment).

rgeary1 · 2015-03-03T01:53:28Z

Signed the CLA, Tower Research Capital

TeBoring · 2015-03-04T03:43:35Z

@rgeary1 could you try "Signed the CLA, Tower Research Capital" in #178

googlebot · 2015-03-04T03:43:37Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

TeBoring · 2015-03-06T09:54:57Z

All CLAs should have been signed.

Implement a feature to generate a dependency file

Richard Geary and others added 3 commits January 23, 2015 12:52

Fix comments.

5fa3956

googlebot added the cla: no label Feb 4, 2015

TeBoring mentioned this pull request Feb 4, 2015

Add support for outputting dependency manifest files, used by ninja and make #178

Merged

TeBoring added 2 commits February 4, 2015 10:20

Change flag for generating dependency file to "dependency_out". Delete

a1b351c

test-driver

Remove "include sstream" from command_line_interface.cc

2e32b8b

dmaclach reviewed Feb 4, 2015
View reviewed changes

Clean code

3edcbaf

dmaclach reviewed Feb 4, 2015
View reviewed changes

TeBoring added 3 commits February 4, 2015 15:48

Fix comment

1d627f8

Change target to output filenames

e2555e2

Fix absolute/relative path in output

eb2ce02

TeBoring added a commit that referenced this pull request Mar 8, 2015

Merge pull request #193 from TeBoring/sb

a5f7bb8

Implement a feature to generate a dependency file

TeBoring merged commit a5f7bb8 into protocolbuffers:master Mar 8, 2015

bithium pushed a commit to bithium/protobuf that referenced this pull request Sep 4, 2023

Merge pull request protocolbuffers#193 from TeBoring/sb

b4e9250

Implement a feature to generate a dependency file

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a feature to generate a dependency file #193

Implement a feature to generate a dependency file #193

TeBoring commented Feb 4, 2015

googlebot commented Feb 4, 2015

TeBoring commented Feb 4, 2015

googlebot commented Feb 4, 2015

TeBoring commented Feb 4, 2015

dmaclach Feb 4, 2015

dmaclach Feb 4, 2015

TeBoring Feb 4, 2015

TeBoring Feb 5, 2015

xiaofeng-gg Feb 6, 2015

rgeary1 commented Feb 4, 2015

TeBoring commented Feb 19, 2015

rgeary1 commented Feb 19, 2015

rgeary1 commented Mar 3, 2015

TeBoring commented Mar 4, 2015

googlebot commented Mar 4, 2015

TeBoring commented Mar 6, 2015

Implement a feature to generate a dependency file #193

Implement a feature to generate a dependency file #193

Conversation

TeBoring commented Feb 4, 2015

googlebot commented Feb 4, 2015

TeBoring commented Feb 4, 2015

googlebot commented Feb 4, 2015

TeBoring commented Feb 4, 2015

dmaclach Feb 4, 2015

Choose a reason for hiding this comment

dmaclach Feb 4, 2015

Choose a reason for hiding this comment

TeBoring Feb 4, 2015

Choose a reason for hiding this comment

TeBoring Feb 5, 2015

Choose a reason for hiding this comment

xiaofeng-gg Feb 6, 2015

Choose a reason for hiding this comment

rgeary1 commented Feb 4, 2015

TeBoring commented Feb 19, 2015

rgeary1 commented Feb 19, 2015

rgeary1 commented Mar 3, 2015

TeBoring commented Mar 4, 2015

googlebot commented Mar 4, 2015

TeBoring commented Mar 6, 2015