dev-python/brotlipy: Introducing the package #8261

bucefal91 · 2018-05-04T18:29:18Z

Introduce the dev/python/brotlipy package into Gentoo.

Closes: https://bugs.gentoo.org/654854
Package-Manager: Portage-2.3.33, Repoman-2.3.9

gentoo-repo-qa-bot · 2018-05-04T18:31:04Z

Pull Request assignment

Areas affected: ebuilds
Packages affected: dev-python/brotlipy

dev-python/brotlipy: @gentoo/proxy-maint (new package)

Bugs linked: 654854

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Gentoo Mirror & CI services are provided by Michał Górny. The hardware was kindly provided by Todd Goodman. This unofficial service is not associated with Gentoo Infrastructure or Gentoo Foundation.

This service is provided by the service provider "as is" and any express or implied warranties, including, but not limited to, the implied warranties of merchantability and fitness for a particular purpose are disclaimed. In no event shall the service provider be liable for any direct, indirect, incidental, special, exemplary, or consequential damages (including, but not limited to, procurement of substitute goods or services; loss of use, data, or profits; or business interruption) however caused and on any theory of liability, whether in contract, strict liability, or tort (including negligence or otherwise) arising in any way out of the use of this service, even if advised of the possibility of such damage.

ConiKost · 2018-05-04T19:39:12Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+LICENSE="MIT"
+SLOT="0"
+KEYWORDS="~amd64 ~x86"
+IUSE=""


Drop it, if empty.

ConiKost · 2018-05-04T19:41:46Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+IUSE=""
+
+DEPEND="${PYTHON_DEPS}
+		dev-python/setuptools[${PYTHON_USEDEP}]


Only one tab please.

ConiKost · 2018-05-04T19:41:47Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+		dev-python/setuptools[${PYTHON_USEDEP}]
+"
+RDEPEND="${DEPEND}
+		virtual/python-cffi[${PYTHON_USEDEP}]


Only one tab please.

bucefal91 · 2018-05-04T19:59:19Z

I've pushed an update that takes care of your feedback.

mgorny · 2018-06-03T07:34:27Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+SLOT="0"
+KEYWORDS="~amd64 ~x86"
+
+DEPEND="${PYTHON_DEPS}


distutils-r1 adds PYTHON_DEPS for you.

mgorny · 2018-06-03T07:34:32Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+inherit distutils-r1
+
+DESCRIPTION="Python binding to the Brotli library"
+HOMEPAGE="https://pypi.python.org/pypi/brotlipy"


Include GitHub as well.

mgorny · 2018-06-03T07:35:26Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+	dev-python/setuptools[${PYTHON_USEDEP}]
+"
+RDEPEND="${DEPEND}
+	virtual/python-cffi[${PYTHON_USEDEP}]


This seems to be a setup-dependency as well.

mgorny · 2018-06-03T07:35:51Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+"
+RDEPEND="${DEPEND}
+	virtual/python-cffi[${PYTHON_USEDEP}]
+"


Run tests.

USE_SHARED_BROTLI=1, don't use bundled libs.

@mgorny After investing a few hours and learning quite a bunch of new things about gentoo and python, I actually do not have much of positive news.

In the tests it uses datasets from brotli (https://github.com/python-hyper/brotlipy/blob/v0.7.0/test/conftest.py#L7 see the TEST_DATA_DIR var). Should I see how I can mock this around and supply somehow the test dataset or should I just give up on the tests? The tests did pass successfully after I manually altered the brotlipy-0.7.0.tar.gz tarball and included necessary files in there. The dataset is a little more than 1Mb of various files that are being compressed/decompressed.

As for USE_SHARED_BROTLI=1 -- unfortunately it was introduced after 0.7.0 tag, i.e. right now it's only in the dev snapshot.

The rest (corrected DEPEND and github homepage) are taken care of by me.

Are you trying to say that the tests are not included in release? Can you use the generated tarball from GitHub instead?

Hello! Sorry to have disappeared for some time.

I have invested extra time into this ebuild. As far as the tests - the problem is that brotlipy tests are using data (files to compress/de-compress) from brotli lib. The path it uses (relative to brotlipy workdir is "'libbrotli/tests/testdata". I certainly could patch the source code of brotlipy and change the dirname to point somewhere else. Is there anyway I could "access" contents of "tests/" of a foreign ebuild while testing "my" ebuild? I ran equery files app-arch/brotli but it did not show the tests filles anywhere.

I also tried once again to use the shared brotli lib. I patched the latest release of brotlipy with necessary patch to have the option of using external lib instead of the bundled one. The patch worked and I was able to compile brotlipy ebuild. But then a failure expected me :) Brotlipy comes with "0.6.0" brotli bundled whereas "app-arch/brotli" within Gentoo offers only brotli-1.0.* versions. They are incompatible and mitmproxy (a tool that depends on brotlipy) refused to start with errors related to brotli when I tried to launch it.

Should I try to include brotli-0.6.* ebuild into Gentoo repo? Or should we give up and use the bundled one? Also, do you have any suggestion/opinion about the tests? Maybe we should upload the test data files somewhere and download them from SRC_URI?

Ok, that looks bad. So we really don't want to use old libs here.

What are the perspectives for updating it to work with brotli-1.*?

As for test data, feel free to just fetch some version of brotli (preferably the newest) in SRC_URI="test? ( ... ) thing.

Yes, I understand you.

To use brotli-1.* the bottleneck appears to be upstream: brotlipy currently does not support it. Here is an issue on their github where a guy asks if they are planning on doing something about brotli-1.* python-hyper/brotlicffi#129

Does it mean I am out of luck to have dev-python/brotlipy in Gentoo tree? I also double checked the latest mitmproxy release now (remember that I am introducing all these packages for mitmproxy?) and the current master of mitmproxy depends on brotlipy>=0.7.0,<0.8. And brotlipy in those versions only works with brotli-0.*

I also got you about the tests. I'll code that part in the coming days :)

Just bundle it then.

Michal :) I hope now this ebuild is in proper shape. I just git push --force an update into this branch.

A few points about the ebuild:

I download brotli library in the exact same commit (state) as it's referenced within brotlipy as a git submodule.

For some reason pypi release of brotlipy did not include configtest.py file and it was required for pytest to successfully run the tests, so I placed it into files subfolder and copy it into appropriate place during src_prepare().

I also tried to use github release of brotlipy (which does contain configtest.py file) and inject brotli lib into it, but the tests were failing for some reason.

I successfully emerged this ebuild with tests both enabled and disabled.

mgorny · 2018-06-24T08:19:02Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+HOMEPAGE="https://github.com/python-hyper/brotlipy/ https://pypi.python.org/pypi/brotlipy"
+SRC_URI="
+	mirror://pypi/${PN:0:1}/${PN}/${P}.tar.gz
+	test? ( https://github.com/google/brotli/archive/${BROTLI_BUNDLED_COMMIT}.zip -> ${P}-tests.zip )


.tar.gz, please. Also, name it after the top directory inside, i.e. brotli-${BROTLI_BUNDLED_COMMIT}.tar.gz. This will make it possible to reuse this distfile if we ever need to package this specific commit of brotli.

mgorny · 2018-06-24T08:20:18Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+
+DEPEND="
+	dev-python/setuptools[${PYTHON_USEDEP}]
+	virtual/python-cffi[${PYTHON_USEDEP}]


I'm pretty sure CFFI is a RDEPEND as well.

mgorny · 2018-06-24T08:20:37Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+	if use test; then
+		# Inject test data into the bundled brotli lib from the upstream
+		# brotli.
+		rm -rf "${WORKDIR}/${P}/libbrotli/tests"


Why -f? Missing ||die.

mgorny · 2018-06-24T08:21:05Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+		# Inject test data into the bundled brotli lib from the upstream
+		# brotli.
+		rm -rf "${WORKDIR}/${P}/libbrotli/tests"
+		ln -s "${WORKDIR}/brotli-${BROTLI_BUNDLED_COMMIT}/tests" "${WORKDIR}/${P}/libbrotli/tests"


mgorny · 2018-06-24T08:22:06Z

dev-python/brotlipy/brotlipy-0.7.0.ebuild

+		ln -s "${WORKDIR}/brotli-${BROTLI_BUNDLED_COMMIT}/tests" "${WORKDIR}/${P}/libbrotli/tests"
+
+		# Inject configtest.py neccesary for pytest.
+		cp "${FILESDIR}/conftest.py" "${WORKDIR}/${P}/test/conftest.py"


Putting this in FILESDIR really, really sucks, especially given it's full file with copyright potential. Could you try to figure out what's wrong with GitHub generated archive?

I've done all of your feedback. now it's using github release and I inject the brotli lib into it. I have next to nothing in my knowledge about python and C, so I took the "stupid" approach to figure out what github release was missing - I simply put myself to compare build dir of portage when it was building from pypi and when it was building from github release. One of the differences that called up my attention was the fact that "libbrotli/python" folder was missing in the case of pypi. I assume the python code somehow interfered with brotlipy. So after removing this folder during src_prepare() the tests ran successfully.
I confirm mitmproxy works successfully on this latest brotlipy-0.7.0.ebuild

:)

I'd say what you did was probably the best way to find the problem. I suspect that pytest was picking up some tests from the bundled brotli Python stuff.

Introduce the dev/python/brotlipy package into Gentoo. Closes: https://bugs.gentoo.org/654854 Package-Manager: Portage-2.3.40, Repoman-2.3.9

gentoo-repo-qa-bot · 2018-06-24T13:18:25Z

Pull request CI report

Report generated at: 2018-06-24 13:18 UTC
Newest commit scanned: 930269a
Status: ✅ good

No issues found

mgorny · 2018-06-24T14:47:40Z

Well, can't say it's perfect since it builds libbrotli multiple times but I guess that's good enough for now. Thanks!

bucefal91 · 2018-06-24T21:46:52Z

Thank you! I do appreciate you walking me through this QA process - I do learn a lot from your feedback!

ConiKost reviewed May 4, 2018

View reviewed changes

dev-python/brotlipy/brotlipy-0.7.0.ebuild Outdated

LICENSE="MIT"

SLOT="0"

KEYWORDS="~amd64 ~x86"

IUSE=""

Copy link

Contributor

ConiKost May 4, 2018

Choose a reason for hiding this comment

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

Drop it, if empty.

ConiKost reviewed May 4, 2018

View reviewed changes

mgorny requested changes Jun 3, 2018

View reviewed changes

bucefal91 mentioned this pull request Jun 6, 2018

dev-python/kaitaistruct: Introducing new package #8263

Closed

bucefal91 force-pushed the 654854-dev-python-brotlipy branch from 95cc2fe to a7321fc Compare June 7, 2018 18:08

bucefal91 force-pushed the 654854-dev-python-brotlipy branch from a7321fc to de21b9d Compare June 23, 2018 20:53

mgorny requested changes Jun 24, 2018

View reviewed changes

bucefal91 force-pushed the 654854-dev-python-brotlipy branch from de21b9d to 930269a Compare June 24, 2018 12:38

dev-python/brotlipy: Introducing the package

930269a

Introduce the dev/python/brotlipy package into Gentoo. Closes: https://bugs.gentoo.org/654854 Package-Manager: Portage-2.3.40, Repoman-2.3.9

mgorny approved these changes Jun 24, 2018

View reviewed changes

gentoo-bot closed this in 413d240 Jun 24, 2018

bucefal91 deleted the 654854-dev-python-brotlipy branch June 24, 2018 21:49

blshkv pushed a commit to pentoo/pentoo-overlay that referenced this pull request Jun 25, 2018

brotlipy: remove in Gentoo's favour, gentoo/gentoo#8261

3c090e7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev-python/brotlipy: Introducing the package #8261

dev-python/brotlipy: Introducing the package #8261

bucefal91 commented May 4, 2018

gentoo-repo-qa-bot commented May 4, 2018

ConiKost May 4, 2018

ConiKost May 4, 2018

ConiKost May 4, 2018

bucefal91 commented May 4, 2018

mgorny Jun 3, 2018

mgorny Jun 3, 2018

mgorny Jun 3, 2018

mgorny Jun 3, 2018

bucefal91 Jun 7, 2018

mgorny Jun 7, 2018

bucefal91 Jun 20, 2018

mgorny Jun 22, 2018

bucefal91 Jun 22, 2018

mgorny Jun 22, 2018

bucefal91 Jun 23, 2018

mgorny Jun 24, 2018

mgorny Jun 24, 2018

mgorny Jun 24, 2018

mgorny Jun 24, 2018

mgorny Jun 24, 2018

bucefal91 Jun 24, 2018

mgorny Jun 24, 2018

gentoo-repo-qa-bot commented Jun 24, 2018

mgorny commented Jun 24, 2018

bucefal91 commented Jun 24, 2018

dev-python/brotlipy: Introducing the package #8261

dev-python/brotlipy: Introducing the package #8261

Conversation

bucefal91 commented May 4, 2018

gentoo-repo-qa-bot commented May 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bucefal91 commented May 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gentoo-repo-qa-bot commented Jun 24, 2018

mgorny commented Jun 24, 2018

bucefal91 commented Jun 24, 2018