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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

libtiff: don't propagate unnecessary build inputs #292568

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from

Conversation

imincik
Copy link
Contributor

@imincik imincik commented Mar 1, 2024

Description of changes

During a review of #290556 we discovered, that libtiff is unncecessary
propagating some build inputs. This change is solving this issue and is fixing
packages which depended on those propagated dependencies (such as libgeotiff
depending on propagated zlib).

  • libtiff: don't propagate unnecessary build inputs
  • libtiff: adopt package under geospatial team maintenance
  • libgeotiff: add missing build dependency

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 馃憤 reaction to pull requests you find important.

@vcunat
Copy link
Member

vcunat commented Mar 3, 2024

When I look at the .pc file, I think that static builds are at most risk here:

Libs: -L${libdir} -ltiff
Libs.private: -llzma -ljpeg -ldeflate -lz -lm 
Cflags: -I${includedir}
Requires.private: liblzma libjpeg libdeflate zlib 

@imincik
Copy link
Contributor Author

imincik commented Mar 4, 2024

I successfully ran libtiff passthru tests on top of latest master branch with imread disabled (which is broken since 2024-01-14).

diff --git a/pkgs/development/libraries/libtiff/default.nix b/pkgs/development/libraries/libtiff/default.nix
index d291be76627c..9e38322863cf 100644
--- a/pkgs/development/libraries/libtiff/default.nix
+++ b/pkgs/development/libraries/libtiff/default.nix
@@ -77,7 +77,7 @@ stdenv.mkDerivation (finalAttrs: {
   passthru = {
     tests = {
       inherit libgeotiff imagemagick graphicsmagick gdal openimageio freeimage;
-      inherit (python3Packages) pillow imread;
+      inherit (python3Packages) pillow;
       pkg-config = testers.hasPkgConfigModules {
         package = finalAttrs.finalPackage;
       };
nix-build -A libtiff.tests
/nix/store/lisq5y6r0vi0d8b8cbi08cy43dx7wbda-freeimage-unstable-2021-11-01
/nix/store/byijifay3w72j97f16crcymigwz51swb-gdal-3.8.4
/nix/store/iw8pj2lyb1almbnw9drf37rmrrlkqb5v-graphicsmagick-1.3.42
/nix/store/hy92fr0xbq19xs64vxdkrphv0ibhba22-imagemagick-7.1.1-29
/nix/store/v5a7ccwfmfkns7yp94ks5sx0r8l27ww4-libgeotiff-1.7.1
/nix/store/rn3d7lq7c4i1mm8qdccjlmrzdcqy4dcp-openimageio-2.5.5.0-bin
/nix/store/jwxzs2kwm8in88hmwf4kfq7rqwpjhsl5-python3.11-pillow-10.2.0
/nix/store/9i8zxxxmdq7bn95l1s3c5y1acb8gw37z-check-pkg-config-libtiff-4

@imincik
Copy link
Contributor Author

imincik commented Mar 5, 2024

I tried to run nixpkgs-review on top of nixpkgs-unstable branch (in hope of reducing number of packages to build), but it still wants to build 30k of packages and needs to download 60k :). I can't do that.

@l0b0 l0b0 mentioned this pull request Mar 5, 2024
13 tasks
@imincik imincik marked this pull request as ready for review March 6, 2024 09:27
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/week-in-geospatial-team/37035/8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants