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

freeimage: unstable-2021-11-01 -> 3.18.0-unstable-2024-04-18 #369766

Merged
merged 5 commits into from
Mar 21, 2025

Conversation

usertam
Copy link
Contributor

@usertam usertam commented Dec 31, 2024

Update to latest svn head. Brought in some CVE patches from Fedora. The CVEs are from Debian Security Tracker.

Patches: https://src.fedoraproject.org/rpms/freeimage/tree/f39
Tracker: https://security-tracker.debian.org/tracker/source-package/freeimage

There are 20+ more CVEs so patching them is more or less a lost cause anyway.

These packages may fail on x86_64-linux, and always fail on all other systems: sbclPackages.cl-freeimage sbclPackages.clinch-freeimage

$ nixpkgs-review...
--------- Report for 'aarch64-darwin' ---------
2 packages failed to build:
sbclPackages.cl-freeimage sbclPackages.clinch-freeimage

3 packages built:
freeimage gamecube-tools perceptualdiff

Should close #298114. (Not the security issues of course.)

Closes #225150

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

@usertam usertam requested a review from risicle December 31, 2024 20:03
@emilazy
Copy link
Member

emilazy commented Dec 31, 2024

cc @LeSuisse who I think wanted to try and get rid of this at some point (but I dunno if that’s practical right now or not).

Thanks for patching a few more of these vulnerabilities; it’s incredible to see how long the CVE list is becoming…

@usertam
Copy link
Contributor Author

usertam commented Jan 1, 2025

Also I observed reproducibility issues with autoSignDarwinBinariesHook, and the hook can be removed without issues on my setup. Any ideas why it is needed in the first place? The original PR adding this didn't provide much context: fd0989d

@emilazy
Copy link
Member

emilazy commented Jan 1, 2025

I think we might not need autoSignDarwinBinariesHook for almost anything any more; cc @reckenrode?

Copy link
Member

@LeSuisse LeSuisse left a comment

Choose a reason for hiding this comment

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

I love starting the year with an update of the vulnerabilities of freeimage 🙃

Changes looks good to me, it builds fine. No new failures from what I can see.

Feel free to merge once the Darwin stuff is figured out :) .

cc @LeSuisse who I think wanted to try and get rid of this at some point (but I dunno if that’s practical right now or not).

Yeah but it is still better than we have right now so I do not see a reason to not merge this change.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 369766


x86_64-linux

⏩ 1 package marked as broken and skipped:
  • deepin.deepin-screen-recorder
❌ 4 packages failed to build:
  • arrayfire
  • colmapWithCuda
  • cudaPackages_11.cuda-samples
  • deepin.deepin-camera
✅ 19 packages built:
  • colmap
  • deepin.deepin-album
  • deepin.deepin-image-viewer
  • deepin.image-editor
  • emulationstation
  • emulationstation-de
  • forge
  • freeimage
  • gamecube-tools
  • kew
  • perceptualdiff
  • pgf_graphics
  • posterazor
  • rucksack
  • sbclPackages.cl-freeimage
  • sbclPackages.clinch-freeimage
  • slade
  • sladeUnstable
  • trenchbroom

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 1, 2025
@reckenrode
Copy link
Contributor

I think we might not need autoSignDarwinBinariesHook for almost anything any more; cc @reckenrode?

I suspect not. Cleaning up all the Darwin signing hooks is something I’d like do once the bootstrap tools are updated.

@emilazy
Copy link
Member

emilazy commented Jan 2, 2025

Let’s just drop it from here for now.

@usertam
Copy link
Contributor Author

usertam commented Jan 2, 2025

OK! I have also taken the liberty to move it to pkgs/by-name since the hook isn't needed anymore. Thank you to everyone involved in the review. Happy new year!

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 2, 2025
@usertam
Copy link
Contributor Author

usertam commented Mar 16, 2025

I still want to get this in. Anyone to approve/merge this?
ping @L-as

@usertam usertam requested a review from L-as March 16, 2025 18:12
@usertam
Copy link
Contributor Author

usertam commented Mar 17, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review --extra-nixpkgs-config '{ permittedInsecurePackages = [ "freeimage-unstable-2024-04-18" ]; }'


x86_64-linux

⏩ 1 package marked as broken and skipped:
  • deepin.deepin-screen-recorder
❌ 3 packages failed to build:
  • arrayfire
  • cudaPackages_11.cuda-samples
  • deepin.deepin-camera
✅ 20 packages built:
  • colmap
  • colmapWithCuda
  • deepin.deepin-album
  • deepin.deepin-image-viewer
  • deepin.image-editor
  • emulationstation
  • emulationstation-de
  • forge
  • freeimage
  • gamecube-tools
  • kew
  • perceptualdiff
  • pgf_graphics
  • posterazor
  • rucksack
  • sbclPackages.cl-freeimage
  • sbclPackages.clinch-freeimage
  • slade
  • sladeUnstable
  • trenchbroom

aarch64-linux

⏩ 1 package marked as broken and skipped:
  • deepin.deepin-screen-recorder
❌ 2 packages failed to build:
  • arrayfire
  • deepin.deepin-camera
✅ 18 packages built:
  • colmap
  • colmapWithCuda
  • deepin.deepin-album
  • deepin.deepin-image-viewer
  • deepin.image-editor
  • emulationstation
  • emulationstation-de
  • forge
  • freeimage
  • gamecube-tools
  • kew
  • perceptualdiff
  • pgf_graphics
  • posterazor
  • sbclPackages.cl-freeimage
  • sbclPackages.clinch-freeimage
  • slade
  • sladeUnstable

x86_64-darwin

❌ 2 packages failed to build:
  • sbclPackages.cl-freeimage
  • sbclPackages.clinch-freeimage
✅ 3 packages built:
  • freeimage
  • gamecube-tools
  • perceptualdiff

aarch64-darwin

❌ 2 packages failed to build:
  • sbclPackages.cl-freeimage
  • sbclPackages.clinch-freeimage
✅ 3 packages built:
  • freeimage
  • gamecube-tools
  • perceptualdiff

@L-as
Copy link
Member

L-as commented Mar 17, 2025

Changes seem ok for the most part but I haven’t tested it and it seems like some things fail to build according to the above comment.

Also, why remove it from top-level?

@ruro
Copy link
Contributor

ruro commented Mar 17, 2025

FYI, cuda-samples failing to build isn't a very big deal. That package is currently quite broken anyway and we're probably going to stop building the samples in cuda-samples that depend on freeimage by default anyway.

Can't comment about the other failing packages though.

@L-as
Copy link
Member

L-as commented Mar 17, 2025

Unfortunate that there seemingly isn't a way to see the build log without running the build again locally.

@usertam
Copy link
Contributor Author

usertam commented Mar 18, 2025

For arrayfire it is more of a boost problem:

arrayfire> In file included from include/boost/compute/detail/meta_kernel.hpp:39,
arrayfire>                  from include/boost/compute/iterator/buffer_iterator.hpp:26,
arrayfire>                  from include/boost/compute/algorithm/detail/copy_on_device.hpp:18,
arrayfire>                  from include/boost/compute/algorithm/copy.hpp:26,
arrayfire>                  from /build/source/src/backend/opencl/./kernel/sort_by_key_impl.hpp:25,
arrayfire>                  from /build/source/src/backend/opencl/kernel/sort_by_key/sort_by_key_impl.cpp:10:
arrayfire> include/boost/compute/detail/sha1.hpp: In member function 'boost::compute::detail::sha1::operator std::string()':
arrayfire> include/boost/compute/detail/sha1.hpp:41:26: error: cannot convert 'unsigned int [5]' to 'unsigned char (&)[20]'

For deepin-camera it is another third-party dep libcam:

deepin-camera> /build/source/libcam/libcam_encoder/mp4.c: In function 'mp4_write_packet':
deepin-camera> /build/source/libcam/libcam_encoder/mp4.c:122:13: error: implicit declaration of function 'set_video_time_capture' [8;;https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Warning-Options.html#index-Wimplicit-function-declaration-Wimplicit-function-declaration8;;]
deepin-camera>   122 |             set_video_time_capture((double)(pts)/1000/1000000);
deepin-camera>       |             ^~~~~~~~~~~~~~~~~~~~~~

For sbclPackages.cl-freeimage and sbclPackages.clinch-freeimage the build is trying to open libpango.dylib:

sbcl-cl-freeimage> BUILD FAILED: Unable to load foreign library (LIBFREEIMAGE).
sbcl-cl-freeimage>   Error opening shared object "libpango.dylib":
sbcl-cl-freeimage>   dlopen(libpango.dylib, 0x000A): tried: '/nix/store/pnmjz6qj1v3lq9f5sj914mz8jvlx21y8-freeimage-unstable-2024-04-18/lib/libpango.dylib' (no such file), '/usr/lib/libpango.dylib' (no such file, not in dyld cache), 'libpango.dylib' (no such file).

None of them are really related to freeimage itself, so I think it is better to open separate PRs for them.

@usertam
Copy link
Contributor Author

usertam commented Mar 18, 2025

Also, why remove it from top-level?

The by-name implementation will traverse the directory to import it, no need for explicit call. I took the opportunity to remove it after autoSignDarwinBinariesHook is gone.

@usertam
Copy link
Contributor Author

usertam commented Mar 18, 2025

Btw I am working on colmap which depends on freeimage: #390412

I was hoping to get this in first, then do a rebase there.

@L-as
Copy link
Member

L-as commented Mar 18, 2025

But did they work before? Odd failures. You can just mark them as broken I suppose.

@usertam
Copy link
Contributor Author

usertam commented Mar 18, 2025

They used to before by looking at hydra, but builds stopped since freeimage is marked insecure. Most of them are rather under-maintained, and also when their dependancies get updated nixpkgs-review will not build and take anything related to freeimage under consideration because it's marked as insecure.

And also deepin.deepin-camera is now removed:

error: 'deepin.deepin-camera' has been removed as it was unmaintained in nixpkgs, Please use 'snapshot' instead

EDIT: arrayfire is marked broken here: #383432

@usertam
Copy link
Contributor Author

usertam commented Mar 18, 2025

All OK now.

Copy link
Member

@L-as L-as left a comment

Choose a reason for hiding this comment

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

LGTM

@wegank wegank added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Mar 19, 2025
Follow versioning convention.

Co-authored-by: Yueh-Shun Li <shamrocklee@posteo.net>
@ShamrockLee ShamrockLee changed the title freeimage: unstable-2021-11-01 -> unstable-2024-04-18 freeimage: unstable-2021-11-01 -> 3.18.0-unstable-2024-04-18 Mar 21, 2025
@ShamrockLee ShamrockLee merged commit 218681f into NixOS:master Mar 21, 2025
32 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update request: freeimage unstable-2021-11-01 → unstable-2023-05-20
8 participants