Ensure that the protoc root import path is examined first. #4129

Merged
merged 2 commits into from Dec 8, 2016

Conversation

Projects
None yet
2 participants
@benjyw
Contributor

benjyw commented Dec 8, 2016

Problem

While using this feature I noticed that protoc can sometimes get confused by the other proto_path entries when doing imports from the repo root of .proto files in the same package (I'm not 100% sure why).

The easy fix is to make sure the root is the first thing to be searched for the imports. The other option was to assume that if you're importing from the root anywhere then you have to everywhere, and not even use the source roots as proto_paths. But that seemed more extreme than was called for.

Solution

Simply move the root to the front of the list of proto paths to add to the cmd line.

Result

Protoc doesn't get confused any more. I modified the test case to exercise the issue.

Ensure that the protoc root import path is examined first.
Otherwise protoc can get confused, it turns out.

@benjyw benjyw requested review from baroquebobcat and stuhood Dec 8, 2016

@stuhood

stuhood approved these changes Dec 8, 2016

Gotta say, pretty confused by the changes to protoc, as they seem to make it harder to do the right thing. Oh well!

+// Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+package org.pantsbuild.testproject.import_from_buildroot;
+

This comment has been minimized.

@stuhood

stuhood Dec 8, 2016

Member

Should add a comment indicating that this file exists to test a particular behavior probably.

@stuhood

stuhood Dec 8, 2016

Member

Should add a comment indicating that this file exists to test a particular behavior probably.

This comment has been minimized.

@benjyw

benjyw Dec 8, 2016

Contributor

Good idea, added a README.md, since this is actually true of the entire package.

@benjyw

benjyw Dec 8, 2016

Contributor

Good idea, added a README.md, since this is actually true of the entire package.

@benjyw benjyw merged commit 930ec02 into pantsbuild:master Dec 8, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@benjyw benjyw deleted the benjyw:protoc_import_order branch Dec 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment