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

Link libz only for static builds #265

Merged
merged 1 commit into from May 15, 2018

Conversation

Projects
None yet
5 participants
@nehaljwani
Copy link
Contributor

nehaljwani commented May 14, 2018

If a user is not building libxml2 and is linking to it dynamically, then
there is no additional need to link to libz, since libxml2 will be
depending on libz anyway.

Link libz only for static builds
If a user is not building libxml2 and is linking to it dynamically, then
there is no additional need to link to libz, since libxml2 will be
depending on libz anyway.
@scoder

This comment has been minimized.

Copy link
Member

scoder commented May 15, 2018

Thanks!

@scoder scoder merged commit 2d07213 into lxml:master May 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@blueyed

This comment has been minimized.

Copy link

blueyed commented on 9f0878e Jun 25, 2018

@nehaljwani
This causes an issue for when using Alpine Linux (3.7) Docker images, and using a hack to allow for wheels to be installed in general (echo "manylinux1_compatible = True" > /usr/lib/python3.6/_manylinux.py):

% from lxml import etree
ImportError: Error relocating /usr/lib/python3.6/site-packages/lxml/.libs/libz-a147dcb0.so.1.2.3: __fprintf_chk: symbol not found

I've not checked if it is really due to this commit, but it looks very likely. But maybe it's instead to the wheels being rebuilt themselves?!

Anyway, I know that enabling wheels this way is a hack, and will switch to Alpine's own py3-lxml package instead.

This comment has been minimized.

Copy link
Member

scoder replied Jun 25, 2018

It this fails with 4.2.2 but not 4.2.1, then it's definitely this commit. Are you saying that you built the wheels yourself and are not using the PyPI provided ones? How did you build them? STATIC_DEPS=true python setup.py bdist_wheel?

This comment has been minimized.

Copy link

blueyed replied Jun 26, 2018

The wheels are not built manually, but come from PyPI.

This comment has been minimized.

Copy link

Bahus replied Jun 26, 2018

I'm not quite sure, but it seems that this fix brake Pillow on ubuntu 14.04. When saving a PNG file Pillow (PIL) uses libz to compress an image. To reproduce it you need ubuntu 14.04 docker image and Python 3.6.* installed:

Example with lxml version 4.2.2 (installed from whl files):

> pip install Pillow=5.1.0
Collecting pillow
  Downloading https://files.pythonhosted.org/packages/5f/4b/8b54ab9d37b93998c81b364557dff9f61972c0f650efa0ceaf470b392740/Pillow-5.1.0-cp36-cp36m-manylinux1_x86_64.whl (2.0MB)
    100% |################################| 2.0MB 636kB/s
Installing collected packages: pillow
Successfully installed pillow-5.1.0
> pip install lxml==4.2.2
> python
>>> from lxml import etree   # dynamically load of libz?
>>> from PIL import Image
>>> Image.new('1', (100, 100)).save('1.PNG')
Traceback (most recent call last):
  File "/root/.pyenv/versions/3.6.2/lib/python3.6/site-packages/PIL/ImageFile.py", line 477, in _save
    fh = fp.fileno()
AttributeError: '_idat' object has no attribute 'fileno'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/root/.pyenv/versions/3.6.2/lib/python3.6/site-packages/PIL/Image.py", line 1935, in save
    save_handler(self, fp, filename)
  File "/root/.pyenv/versions/3.6.2/lib/python3.6/site-packages/PIL/PngImagePlugin.py", line 822, in _save
    [("zip", (0, 0)+im.size, 0, rawmode)])
  File "/root/.pyenv/versions/3.6.2/lib/python3.6/site-packages/PIL/ImageFile.py", line 496, in _save
    raise IOError("encoder error %d when writing image file" % s)
OSError: encoder error -8 when writing image file

The same problem is not reproduced with version 4.2.1:

> pip install lxml==4.2.1
> python
>>> from lxml import etree
>>> from PIL import Image
>>> Image.new('1', (100, 100)).save('1.PNG')
>>> # OK

@nsoranzo nsoranzo referenced this pull request Jun 27, 2018

Merged

Update dev dependencies #6403

@kmike

This comment has been minimized.

Copy link

kmike commented Jun 27, 2018

@scoder

This comment has been minimized.

Copy link
Member

scoder commented Jun 27, 2018

I reverted this change for lxml 4.2.3, just released.

@nehaljwani

This comment has been minimized.

Copy link
Contributor

nehaljwani commented Jun 27, 2018

But why didn't the 4.2.2 wheels link to libz statically?

@scoder

This comment has been minimized.

Copy link
Member

scoder commented Jun 28, 2018

Short answer: I don't know, and reverting the change was a much quicker solution than anything else.

nsoranzo added a commit to nsoranzo/galaxy that referenced this pull request Jul 6, 2018

Update cliff and other related dependencies
cliff now has a universal wheel.
Remove unused Pipfile.lock files.
Skip broken lxml 4.2.2, see lxml/lxml#265 .
Fix more oscillating environment markers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment