protoc: support '=' in --proto_path arguments

[mathstuf]

---

Fixes #877.

[googlebot]

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).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• 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 your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[mathstuf]

I've asked about the CLA here. The company is Kitware if it has already been signed (which may be true; I think we've had Gerrit patches before).

[xfxyjwf]

Could you avoid using "=" in file paths instead (or avoid passing file paths with "=" to protoc)? It seems very weird to support such file paths. It would be very problematic if the "=" sign appears in proto file import path names as I am pretty we don't have special handling for such names. And I also believe this PR addresses one special case but not the general problem of using "=" in file paths. For example, how would you interpret such a proto_path:

$ protoc --proto_path=foo/bar=dir/with=sign xxx.proto

[xfxyjwf]

Regarding the CLA, I can find a corporate CLA signed for Kitware, but I can't find your email address in the authorized contributors from that CLA. Could you ask whoever signed the CLA representing Kitware to give us a updated list of authorized contributors?

[mathstuf]

On Tue, Oct 13, 2015 at 13:21:34 -0700, Feng Xiao wrote:

Could you avoid using "=" in file paths instead (or avoid passing file
paths with "=" to protoc)?

I wish :) . These are autogenerated paths using sigils to indicate
different parts of the buildname (e.g., = is for architecture).

It seems very weird to support such file paths. It would be very
problematic if the "=" sign appears in proto file import path names as
I am pretty we don't have special handling for such names.

The '=' is in the parentage of the source tree. It shouldn't be in the
import paths at all (AFAIK).

And I also believe this PR addresses one special case
but not the general problem of using "=" in file paths. For example,
how would you interpret such a proto_path:

$ protoc --proto_path=foo/bar=dir/with=sign xxx.proto

The code only splits on the first '=', so this would first try
'dir/with=sign' and then if that fails, 'foo/bar=dir/with=sign'.
Supporting aliases with '=' seems less interesting :) .

[xfxyjwf]

On Tue, Oct 13, 2015 at 1:43 PM, Ben Boeckel [email protected]
wrote:

On Tue, Oct 13, 2015 at 13:21:34 -0700, Feng Xiao wrote:

Could you avoid using "=" in file paths instead (or avoid passing file
paths with "=" to protoc)?

I wish :) . These are autogenerated paths using sigils to indicate
different parts of the buildname (e.g., = is for architecture).

What's the source tree layout of your project? If "=" does not appear in
proto file import paths, there gets to be a way to avoid it in protoc
invocations. For example:
/tmp$ cd dir/with=sign
/tmp/dir/with=sign$ protoc --proto_path=. path/to/protos ...

It seems very weird to support such file paths. It would be very
problematic if the "=" sign appears in proto file import path names as
I am pretty we don't have special handling for such names.

The '=' is in the parentage of the source tree. It shouldn't be in the
import paths at all (AFAIK).

And I also believe this PR addresses one special case
but not the general problem of using "=" in file paths. For example,
how would you interpret such a proto_path:

$ protoc --proto_path=foo/bar=dir/with=sign xxx.proto

The code only splits on the first '=', so this would first try
'dir/with=sign' and then if that fails, 'foo/bar=dir/with=sign'.
Supporting aliases with '=' seems less interesting :) .

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

[mathstuf]

On Tue, Oct 13, 2015 at 14:03:01 -0700, Feng Xiao wrote:

What's the source tree layout of your project? If "=" does not appear in
proto file import paths, there gets to be a way to avoid it in protoc
invocations. For example:
/tmp$ cd dir/with=sign
/tmp/dir/with=sign$ protoc --proto_path=. path/to/protos ...

Not really. With out-of-source builds, there may not be a relative
path to the source tree from the binary tree, so CMake always uses the
absolute path to the source directory. Changing directory into the
source tree is generally not done for out-of-source builds (I don't
think I've ever seen such a thing at least).

[xfxyjwf]

On Tue, Oct 13, 2015 at 2:11 PM, Ben Boeckel [email protected]
wrote:

On Tue, Oct 13, 2015 at 14:03:01 -0700, Feng Xiao wrote:

What's the source tree layout of your project? If "=" does not appear in
proto file import paths, there gets to be a way to avoid it in protoc
invocations. For example:
/tmp$ cd dir/with=sign
/tmp/dir/with=sign$ protoc --proto_path=. path/to/protos ...

Not really. With out-of-source builds, there may not be a relative
path to the source tree from the binary tree, so CMake always uses the
absolute path to the source directory. Changing directory into the
source tree is generally not done for out-of-source builds (I don't
think I've ever seen such a thing at least).

Thanks. Seems there isn't a way to workaround here.

I'll merge this PR after the CLA issue is resolved. The original signer of
the Kitware CLA is Berk Geveci. As far as I can tell, he hadn't provided us
a list of authorized contributors under this CLA, so I can't accept this PR
until we get a updated list of authorized contributors. Could you contact
him to update the CLA?

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

[mathstuf]

Thanks. Going through our process here (it seems to have changed since he signed it) :) .

[mathstuf]

So we've updated to the Google Groups CLA thing now.

[mathstuf]

I signed it!

[mathstuf]

Ping? Is there some detection that isn't working?

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok to test.

[xfxyjwf]

@mathstuf, I still cannot find the CLA for you (check https://signcla.corp.google.com/). Are you using a different email for your CLA? Presumably the CLA only covers your corp email like [email protected]?

[mathstuf]

On Wed, Apr 20, 2016 at 15:04:00 -0700, Feng Xiao wrote:

@mathstuf, I still cannot find the CLA for you (check https://signcla.corp.google.com/). Are you using a different email for your CLA? Presumably the CLA only covers your corp email like [email protected]?

The author and committer email are using my Kitware email address
([email protected]) which is an alternate email on this account.

[xfxyjwf]

Could you check again? I'm seeing Ben Boeckel [email protected] as the commiter:
https://github.com/mathstuf/protobuf/commit/44c36312fe7eb2d77f8b5ea8bc5d2adc50bc5a41.patch

[googlebot]

CLAs look good, thanks!

[mathstuf]

Ah ha. I had cherry-picked from our internal fork of protobuf into my local github clone without changing user.email.

[xfxyjwf]

ok to test

[mathstuf]

The failing test here doesn't seem related (NuGet package errors).

[xfxyjwf]

Thanks!