Skip to content

Export protobuf sources as filegroup#77

Merged
htuch merged 2 commits intoenvoyproxy:masterfrom
lizan:export_proto_src
Jun 13, 2017
Merged

Export protobuf sources as filegroup#77
htuch merged 2 commits intoenvoyproxy:masterfrom
lizan:export_proto_src

Conversation

@lizan
Copy link
Member

@lizan lizan commented Jun 10, 2017

@htuch
Copy link
Member

htuch commented Jun 11, 2017

@lizan Out of curiosity, who is doing the descriptor generation? I.e. what are you working on with v2 API dependency?
@mattklein123 Can you review/merge staging to master before we do this merge?

@mattklein123
Copy link
Member

#78

@lizan
Copy link
Member Author

lizan commented Jun 12, 2017

@htuch the transcoding filter need protobuf descriptors as config, sample:
https://github.com/istio/proxy/blob/master/src/envoy/transcoding/envoy.conf#L37
and it is depending google/api/annotations.proto and google/api/http.proto for the HTTP mapping.

The descriptor is generated from .proto files, so we have a genrule to generate descriptors and load it into tests. Since envoy's googleapis repo are loaded from here, that what this PR is.

Alternatively we can move this dependency loading to envoy repo itself, I'm fine with either way.

@htuch htuch changed the base branch from staging to master June 12, 2017 18:40
@htuch
Copy link
Member

htuch commented Jun 12, 2017

@lizan We've switched back to working on master. I changed the base of this PR, but there are now merge conflicts. Can you resolve these and update the PR? Thanks!

@lizan lizan force-pushed the export_proto_src branch from ab981b5 to 96c783b Compare June 13, 2017 00:34
@lizan
Copy link
Member Author

lizan commented Jun 13, 2017

@htuch rebased. PTAL.

@htuch
Copy link
Member

htuch commented Jun 13, 2017

@lizan Looks like Travis is unhappy, can you take a look?

@lizan
Copy link
Member Author

lizan commented Jun 13, 2017

Seems {cc,py}_proto_library doesn't like filegroup as srcs, revert that part.

@htuch htuch merged commit eb2e9a4 into envoyproxy:master Jun 13, 2017
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

Successfully merging this pull request may close these issues.

3 participants