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.

Could you use redirects instead if you want to read/write to files?

in the context where this was running, redirects weren't possible which is why we added explicit flags. we could spawn a shell simply to do the redirects, but that adds a pretty big hassle of dealing with shell expansion/etc...

@pprabhu It looks like Gentoo is still carrying this patch out of tree.

The patch still applies to the latest protobuf sources, but now EncodeDecodeTest.RedirectInputOutput fails when running make check. Would you mind taking a quick look?

My bad. Updated test to dtrt.
I've created a new pull-request (#513) so we don't have to hop through @vapier for updates.

Re: ABI, readme seems to wash it's hands off of ABI compatibility. Let me know if you'd rather I added a new function with the extra arguments.

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.

for protobuf maintainers, the CLA checks are a false positive and can be ignored in this PR ... see http://b/20917233 for details

@vapier I am going to close this because there hasn't been any activity in a couple years, but let me know if you are still interested in this change.

yes, we're still interested as it makes integration with build systems much easier. i can rebase if need be.

@vapier Could you elaborate on why shell redirects are not sufficient? My initial reaction is to agree with Jisi that it would be best to avoid adding new flags if we can get by with redirects.

not all build tools allow to redirect stdin/stdout easily. sure, instead of running protoc directly, i could do something awful like sh -c 'protoc < $foo > $bar', but that gets even messier to properly quote arguments.

conversely, it seems fairly common for low level tools to have CLI options to control inputs/outputs.

I'm still a bit skeptical about this. The thing is that once we add flags to protoc, we will have to maintain them pretty much forever, so it seems like a good idea to err on the side of not adding any new flags unless we're really sure they're worth it.

This encoding and decoding feature seems like something that could go in a simple standalone utility without needing any protoc changes. You could run protoc with --descriptor_set_out to generate serialized descriptors. Then the utility could read in the serialized descriptors and use DynamicMessage to encode/decode between text format and the binary format.

i grok that accepting anything is a maintenance burden. but i'm not sure parameterizing inputs/outputs is really that big of a deal, especially considering it's bog standard across such a wide variety of tools, and considering protoc already has a wealth of --xxx_out flags. it's not like we're creating a new convention.

i'm not sure i'm following you wrt --descriptor_set_out. if you want us to write a wrapper, why wouldn't we just write a shell wrapper to do it ? not that i want to as it means replicating that little wrapper to all our build projects rather than just using the flags with protoc directly.

in="$1" out="$2"
shift 2
exec protoc "$@" <"${in}" >"${out}"

I was trying to think of ways to avoid using the shell, but if that shell script wrapper works then that's even better.

imo this belongs in the tool itself. compilers/translators that only operate on stdin/stdout are needlessly hostile to build integration (which is the primary use case for these tools).

@xfxyjwf What do you think about this?

I'm against this proposal.

The primary functionality of the protocol compiler is to translate .proto files into generated code and we already have a lot of flags just for that. Adding more flags for text format decoding/encoding is a distraction from protoc's core functionality. It not only adds complexity to the implementation but also adds complexity to the protoc command line interface which a lot of users find confusing enough already. The proposed names for these two new flags are also pretty bad. "--protobuf_in": absolute path to the protobuf file to read to encode/decode. It sounds so much like it's reading a .proto file instead of an input data file. "--protobuf_out", if --cpp_out produces C++ code, shouldn't --protobuf_out produces .proto files? They are even more confusing if you put them together with something like --descriptor_set_in/--descriptor_set_out. And all of this for some strange build tools that can't do stdin/stdout redirect properly? I don't think so.

I agree with Adam that this encoding and decoding feature should really be put into a standalone utility. It shouldn't be too hard to create and we can even add more features there like JSON support. We actually already have such tools in Google internally and users are using that instead of protoc. After we move protobuf to its own organization, I think we can port some of those tools to opensource. That would give us a better tool for format conversion and also relieves protoc from this complexity.

Can we close it as two owners are against this proposal?