Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[protobuf_cpp] Updated to Protobuf 3.5.1 + fixed protoc issue #1440

Merged
merged 1 commit into from
Nov 10, 2018

Conversation

goffi-contrib
Copy link
Contributor

Protobuf has been updated to 3.5.1, it's not the last version (3.6.1)
because last version cause trouble during compilation and needs more
work.

This patch also fixes the issue with hardcoded local path for protoc which
could not work. Protobuf needs a version of "protoc" (its compiler, used
to produce .py file from .proto files) compiled for the host platform.
Official binaries for protoc are available, the recipe will download the
one suitable for host and use it.
If the platform is not supported, building may still work if the right "protoc"
is available in search path.

@AndreMiras
Copy link
Member

Thanks for the PR! Could you fix the linter so the integration test makes it to the end.

�[1mpep8 runtests: commands[0] | flake8 pythonforandroid/ tests/ ci/�[0m
pythonforandroid/recipes/protobuf_cpp/__init__.py:22:5: E303 too many blank lines (2)
pythonforandroid/recipes/protobuf_cpp/__init__.py:41:25: E122 continuation line missing indentation or outdented

Keep in mind you can always run tox locally to check linting issues upfront.

@goffi-contrib
Copy link
Contributor Author

@AndreMiras done

@AndreMiras
Copy link
Member

Looking good thank you 👍 I'm sorry that the CI is still far from being perfect and it currently fails because of a missing system dependency. Until we tacle that, can you simply add protobuf to our ignore list with something like:

diff --git a/ci/constants.py b/ci/constants.py
index 804ceffa..e7f84aca 100644
--- a/ci/constants.py
+++ b/ci/constants.py
@@ -44,6 +44,9 @@ BROKEN_RECIPES_PYTHON2 = set([
     'm2crypto',
     'netifaces',
     'Pillow',
+    # requires autoconf system dependency on host
+    # https://api.travis-ci.org/v3/job/450538715/log.txt
+    'protobuf_cpp',
     # https://github.com/kivy/python-for-android/issues/1405
     'psycopg2',
     'pygame',

Otherwise no worries I can do it since I'll pick up #1454 pretty soon.

Protobuf has been updated to 3.5.1, it's not the last version (3.6.1)
because last version cause trouble during compilation and needs more
work.

This patch also fixes the issue with hardcoded local path for protoc which
could not work. Protobuf needs a version of "protoc" (its compiler, used
to produce .py file from .proto files) compiled for the host platform.
Official binaries for protoc are available, the recipe will download the
one suitable for host and use it.
If the platform is not supported, building may still work if the right "protoc"
is available in search path.
@goffi-contrib
Copy link
Contributor Author

@AndreMiras done :)

Copy link
Member

@AndreMiras AndreMiras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks for maintaining

@AndreMiras AndreMiras merged commit 6369041 into kivy:master Nov 10, 2018
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