Skip to content

Fix for issue with importing proto files from subpackages in another jar#16

Closed
pnyheim wants to merge 1 commit intodtrott:masterfrom
pnyheim:master
Closed

Fix for issue with importing proto files from subpackages in another jar#16
pnyheim wants to merge 1 commit intodtrott:masterfrom
pnyheim:master

Conversation

@pnyheim
Copy link

@pnyheim pnyheim commented Oct 24, 2011

Fixes the issue where proto files in dependencies are incorrectly specified on the path if they reside in subpackages inside the jar

This is exactly the issue described by sergei-ivanov in TEST-5 of his pull-request, https://github.com/sergei-ivanov/maven-protoc-plugin/blob/master/src/it/TEST-5/invoker.properties

I have also tested this fix against his pull-request by commenting out the invoker.failureBehavior = fail-never line in invoker.properties

…cified on the path if they reside in subpackages inside the jar
@sergei-ivanov
Copy link

Hello Paul,

Thanks for sumitting a patch. Curiously, I have already made exactly the same changes on my local branch, but wanted to clean up the integration tests before pushing it back to github. I have also fixed the import paths inside a reactor (TEST-9 and TEST-11), and our fix should also fix TEST-7 (same as TEST-5, but for test dependencies). I have also integrated some changes from pull request #15 into my local branch to fix inclusion of 'main' protos into 'test' protos within the same module. These are backed by further integration tests. Let me clean up some things and push the changes to my github branch. I think once all these changes are in place, I'll roll a release, and then I'll do a bigger refactoring to accommodate generation of non-java outputs.

Kind regards,
Sergei

@pnyheim
Copy link
Author

pnyheim commented Oct 24, 2011

Ah. Great work. I also noticed that this would affect more tests than I mentionned, and I noticed that it did not work for importing src/main/proto/someprotofile from files in src/test/proto but did not have time to look more into it today. Just had to roll a release for internal use until an 'official' release comes out of this repo.

Keep up the good work,
Paul

@pnyheim pnyheim closed this Oct 24, 2011
@sergei-ivanov
Copy link

Pushed all pending changes to sergei-ivanov/maven-protoc-plugin
That should fix all previously discovered namespace-related problems.

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.

2 participants

Comments