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

Better distribution - archives with libs and images #45

Merged
merged 15 commits into from Jan 30, 2015
Merged

Better distribution - archives with libs and images #45

merged 15 commits into from Jan 30, 2015

Conversation

almarklein
Copy link
Member

For better building of a Debian package #40, and to be nice for people with no internet connection #42.

This closes #40, closes #42, closes #39.

@almarklein almarklein changed the title Better distribution - wheels with libs and images Better distribution - archives with libs and images Dec 15, 2014
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 9d555c6 on almarklein:dist into daaacec on imageio:master.

All distribution files are independent of the Python version. The
platform-specific archives contain a few images and the freeimage
library for that platform. These are recommended if you do not want to
rely on an internet connection at runtime.
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean at install time?

Copy link
Member Author

Choose a reason for hiding this comment

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

both, I suppose.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling ef7783b on almarklein:dist into daaacec on imageio:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) when pulling b76f6fb on almarklein:dist into daaacec on imageio:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) when pulling f0ff2bc on almarklein:dist into daaacec on imageio:master.

@almarklein
Copy link
Member Author

@ghisvail Can you confirm that the following would solve the Debian building issues?

  • Platform-specific archives will be provided via pypi. No internet connection should be needed to build/install/test with this package.
  • Also you can now do python setup.py build_with_fi to build image with the freeimage lib and example images. In this case these resources are downloaded at build-time.

@ghisvail
Copy link
Contributor

@almarklein

I just had a look in the dist branch.

Does it mean that:

  • python setup.py sdist_all produces a tarball containing all remote content ?
  • python setup.py build and python setup.py test applied to that tarball no longer requires to fetch any online resource ?

@ghisvail
Copy link
Contributor

I also had a look at the freeimage plugin and saw that you were using embedded copies of libfreeimage for each platforms with some hard-coded names. I have no objections with this design but I was wondering whether it would be possible to alter this design to use a system-wide installed libfreeimage instead of the embedded copy. Reason I am asking is that Debian / Ubuntu already ships with freeimage 3.15 and any project depending on it should use / link with it.

@almarklein
Copy link
Member Author

python setup.py sdist_all produces a tarball containing all remote content ?

It produces a zipfile for the 5 major platforms. And it does not contain all remote content, but enough to run a significant part of the test suite. It does not contain e.g. the ffmpeg exe or avbin lib.

Would it be helpful if there was a setup.py sdist_with_fi command to produce an archive only for the current platform? (it certainly would save time).

python setup.py build and pythonsetup.py test applied to that tarball no longer requires to fetch any online resource ?

Indeed. Except that I have not made a hook to run the tests via setup.py test. I shall look into it.

@almarklein
Copy link
Member Author

About the embedded freeimage libraries vs system level. I found that the system-level freeimage library can be quite old on some systems, giving rise to bugs. Previously we only shipped them for Windows and OSX, but due to this, now also for Linux. Do you think that's a problem?

@ghisvail
Copy link
Contributor

Regarding the setup.py

Regarding the distributed source, I'd need:

  • To get a tarball with all downloadable resources already in. You may choose whatever suits best for you, i.e. with sdist_all or another command.
  • From this tarball, to be able to successively run python setup.py build and the test suite without the need to fetch any online stuff.

Regarding the freeimage:

  • The purpose to force all projects to use the packaged dependencies as opposed to embedded ones is to keep a one stop place where bugs are fixed, i.e. inside the dependency's package.
  • This policy is now more and more strictly enforced because of past security problems. I will ask for imageio but I don't expect a positive answer.
  • I agree that some LTS releases of Linux may end up with outdated versions of libfreeimage. That's why I'd suggest not to drop the use of the embedded libfreeimage entirely, but add support for switching to the system one if it is detected.

Does it sound like too much work ?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) when pulling 162fb14 on almarklein:dist into daaacec on imageio:master.

@almarklein
Copy link
Member Author

Ok, I added the setup.py test command. To create a tarball with resources: setup.py build_with_fi sdist.

I'd suggest not to drop the use of the embedded libfreeimage entirely, but add support for switching to the system one if it is detected.

I'd rather do it the other way around: use the embedded version if it is detected, otherwise use the system one. That way it would be easy for the user to "upgrade" freeimage if necessary. For the Debian build process, this means we'd simply not ship the freeimage library. However, this may cause some tests to fail. Are you ok with figuring out what to do about this in a new PR?

@blink1073
Copy link
Member

You could detect the freeimage version and exit those tests with a warning

@ghisvail
Copy link
Contributor

I'd rather do it the other way around: use the embedded version if it is detected, otherwise use the system one.

That's what I meant.

For the Debian build process, this means we'd simply not ship the freeimage library.

You could still ship it and save yourself the time of producing multiple release tarballs. We have support for stripping the upstream tarball from non-compliant files like pre-compiled binaries.

imageio-provided lib has preference if it is already downloaded.
If version <15, the lib will be downloaded.

This means that on relatively modern Linuxes, that have freeimage
installed, the freeimage lib wont be downloaded at all.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) when pulling 3df1d7a on almarklein:dist into daaacec on imageio:master.

@almarklein
Copy link
Member Author

@ghisvail I did the following: imageio will first try to find a freeimage library on the system. If not found, or if the version is < 15, the imageio prebuild version is auto-downloaded and used. In the first search, the imageio-provided lib has preference. Sounds good?

@blink1073
Copy link
Member

👍

@almarklein
Copy link
Member Author

@ghisvail So you can indeed strip the freeimage lib from the package, and it should work pretty much as Debian likes.

almarklein added a commit that referenced this pull request Jan 30, 2015
Better distribution - archives with libs and images
@almarklein almarklein merged commit 52e1bf6 into imageio:master Jan 30, 2015
@almarklein almarklein deleted the dist branch January 30, 2015 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants