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.

Sign up for GitHub

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

Jump to bottom

Fix setup.py on debian #2853

Merged
merged 2 commits into from
Nov 15, 2017
Merged

Fix setup.py on debian #2853

merged 2 commits into from
Nov 15, 2017

Conversation

wiredfool
Copy link
Member

Fixes #2851 .

Apparently Debian strips out all the platform detection code when building .debs, and we're missing support for some arm variants. But since that support is missing, we pop an error when we don't detect the linux version.

Changes proposed in this pull request:

  • Promote the add_multiarch_paths to run before the manual detection, if we know we're on a debian derivative. This is going to continue to fail on non-ubuntu/non-debian debian derivatives.
  • Refactor a series of elifs into a data structure.

@hugovk
Copy link
Member

hugovk commented Nov 13, 2017

+ # Ubunbu <= Precise fails, >= Trusty succeeds.

Do we care about <= Precise? Trusty is the oldest LTS supported.

image

https://en.wikipedia.org/wiki/Ubuntu_version_history#Table_of_versions

@wiredfool
Copy link
Member Author

For some reason, I thought it was good till next year, even after migrating off of it. It's still in the docker builds, but we could drop that, as we've likely got library coverage for old libraries on the centos build.

@wiredfool
Copy link
Member Author

Ok, I've removed debian from the tests, and force rebased this one on top of that.

@gerardgilliland
Copy link

Wiredfool,

You commented, there's got to be a better way ... But at least you have left enough tracks to help me.
I am attempting to install hyperspy
And thanks to the help of Josh Taillon, Petras Jokubauskas, Magnus Nord, and bochecha
I am down to the last module (I think or at least I hope).

I have the same problem as bochecha:
Can't build Pillow on Debian armhf
#2851
I can't build Pillow on a Raspberry Pi ARM processor which I believe falls under his category.
The bottom line(s):
The headers or library files could not be found for jpeg,
a required dependency when compiling Pillow from source.
And I think the following should say arm rather than 386
File "/tmp/pip-build-3866ru_p/pillow/setup.py", line 577, in build_extensions
raise RequiredDependencyException(f)

I believe your fix includes 2851.
Having never gone beyond the standard install,
How do I avoid waiting for an unknown time for this to show up when I attempt to install ?
This tells me how to contribute (using the master)
https://github.com/python-pillow/Pillow/blob/master/.github/CONTRIBUTING.md
by going to https://github.com/python-pillow/Pillow/pulls and clicking “New Pull Request”
But I think, master plus your fixes are at:
https://github.com/wiredfool
under Contribution activity
with the release and date: Release 4.4.0 Issue: 2018-01-01
Could you please point me to any link that would be:
Pillow master, plus ARM processor selection fix, plus Pillow fixes
That would occur within the next day or two?

Thank you and Peace,
Gerard Gilliland

@gerardgilliland gerardgilliland mentioned this pull request Nov 14, 2017
Closed
@hugovk
Copy link
Member

hugovk commented Nov 14, 2017 •
edited
Loading

You can test this out with:

git clone https://github.com/wiredfool/Pillow/
cd Pillow
git checkout debian-arm

Then building the source as detailed in the docs.

@gerardgilliland gerardgilliland mentioned this pull request Nov 14, 2017
Closed
@gerardgilliland
Copy link

gerardgilliland commented Nov 14, 2017 via email

@hugovk
Copy link
Member

hugovk commented Nov 14, 2017

Once you have the prerequisites in place, don't 'pip install pillow', which installs the last release from PyPI, but instead run 'python setup.py install' from the cloned Pillow directory.

@gerardgilliland
Copy link

gerardgilliland commented Nov 15, 2017 via email

@hugovk
Copy link
Member

hugovk commented Nov 15, 2017

Merging.

@gerardgilliland Would you like to open a new issue?

https://github.com/python-pillow/Pillow/issues/new

@hugovk hugovk merged commit 90eb47d into python-pillow:master Nov 15, 2017
@gerardgilliland
Copy link

Hugo:
You asked: “Would I like to open a new issue ?”

With your help, Yes.

I know how to install and run
git clone https://github.com/wiredfool/Pillow/
cd Pillow
git checkout debian-arm
and run
python setup.py install
from the cloned Pillow directory on Raspberry Pi (with your help).
However, the above fails to build Pillow.
The headers or library files could not be found for jpeg,
a required dependency when compiling Pillow from source.

However, No.

I am simply testing the app and the original failure defined by bochecha still exists.
In other words I was getting this same message the same day #2851 was opened.
And the same other errors as well.
And the same errors with wiredfool’s new code that you linked me to.
So, I would be causing confusion.
I think when other testers on different systems check in to run the same code you linked me to,
They would have the same error as I am getting.
Because I believe bochecha still “Can't build Pillow on Debian armhf”.
If #2853 can’t be changed even though is doesn’t solve the problem,
Then I believe bochecha will open a new issue.
And I will certainly comment on it, that I have the same problem on a Raspberry Pi.

So, Maybe.

I will look at wiredfool’s code in the cloned Pillow folder.
And see if I can get the
python setup.py install
to run to completion.

That is my perspective. Please correct me where I am wrong.

Thank you and Peace,
Gerard

bochecha
bochecha reviewed Nov 22, 2017
View reviewed changes
setup.py
if arch_tp == ("x86_64", "32bit"):
# 32-bit build on 64-bit machine.
# This should be correct on debian derivatives.
if plat.dist()[0].lower() in ('debian', 'ubuntu'):
Copy link

Choose a reason for hiding this comment

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

platform.dist() is deprecated and will be removed in Python 3.7

Copy link
Member Author

Choose a reason for hiding this comment

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

Great. Some batteries included I guess.

@wiredfool wiredfool mentioned this pull request Nov 22, 2017
Merged
@wiredfool wiredfool deleted the debian-arm branch January 8, 2018 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bochecha bochecha bochecha left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wiredfool @hugovk @gerardgilliland @bochecha