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

PR: Improve calls to libpng-config on Ubuntu/Debian #2398

Merged
merged 2 commits into from
Jul 6, 2020

Conversation

andfoy
Copy link
Contributor

@andfoy andfoy commented Jul 6, 2020

Fixes #2392

This PR detects if libpng was installed via apt on Ubuntu/Debian Linux distributions, if so, then the call libpng-config --libdir is disabled, since that flag is not available on those distributions.

@andfoy andfoy changed the title PR: Improve detection of libpng-config on Ubuntu/Debian PR: Improve calls to libpng-config on Ubuntu/Debian Jul 6, 2020
@codecov
Copy link

codecov bot commented Jul 6, 2020

Codecov Report

Merging #2398 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2398   +/-   ##
=======================================
  Coverage   70.65%   70.65%           
=======================================
  Files          94       94           
  Lines        7897     7897           
  Branches     1241     1241           
=======================================
  Hits         5580     5580           
  Misses       1934     1934           
  Partials      383      383           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e212cc8...6af46d8. Read the comment docs.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Thanks!

@fmassa fmassa merged commit d96832a into pytorch:master Jul 6, 2020
@andfoy andfoy deleted the fix_png_debian branch July 6, 2020 21:27
@t-vi
Copy link
Contributor

t-vi commented Jul 9, 2020

This breaks building on Debian testing and unstable. These do not feature a VERSION field in /etc/os-release.
And what is the version used for?

@fmassa
Copy link
Member

fmassa commented Jul 10, 2020

@t-vi the version is necessary because we need a recent-enough version of libpng, otherwise we have errors such as #2301 (comment), which requires libpng 1.6 or higher. I misread the VERSION you were talking about. I don't know, @andfoy can you comment on Thomas question?

@andfoy can you have a look at @t-vi comment?

@t-vi
Copy link
Contributor

t-vi commented Jul 10, 2020

Sorry, I was to terse there! (After waiting for compiling PyTorch to finish I was too impatient.)
So if you don't need the VERSION from os-release, I'd just rip it out the code reading it, but would not want to do that without asking. If you (plan to) use it, we could make up a default.

@fmassa
Copy link
Member

fmassa commented Jul 10, 2020

I think we needed that to fix a behavior in another PyTorch CI, but @andfoy can comment on it. I'm all in for simplifying as much as possible this pipeline.

@andfoy
Copy link
Contributor Author

andfoy commented Jul 10, 2020

Hi @t-vi, thanks for reporting this problem, I think I can simplify the detection logic, so that we don't depend on reading distribution files that might change between versions

de-vri-es pushed a commit to fizyr-forks/torchvision that referenced this pull request Aug 4, 2020
* Improve detection of libpng-config on Ubuntu/Debian

* Do not disable libdir on Mac
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.

build error in ubuntu with libpng
3 participants