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

Fix broken CMake-generated pkg-config files #172150

Closed
wants to merge 99 commits into from

Conversation

alexshpilkin
Copy link
Member

@alexshpilkin alexshpilkin commented May 9, 2022

Description of changes

Fixes #167528 and several other cases of similarly broken pkg-config files generated with CMake. The case of zxing-cpp was discussed in review comments of #172046.

I tried to patch things in such a way that the build won’t break even once an upstream fix is in place.

Upstream bugs: jiixyj/libebur128#121, libjxl/libjxl#1400, kcat/openal-soft#696, sctplab/usrsctp#662, Mindwerks/wildmidi#236, zxing-cpp/zxing-cpp#335, KhronosGroup/SPIRV-Tools#3905, openobex#66, dirkvdb/ffmpegthumbnailer#215, Matroska-Org/libmatroska#62, extra-cmake-modules!268, Matroska-Org/libebml#97.

I only patched things that I found by grepping the Nix store on my machine, so it’s probable there are many more cases of this problem in CMake build systems. Someone who has access to a full build of Nixpkgs should probably make a systematic review (rg -g '*.pc' '}//nix' /nix/store will probably find most cases).

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@alexshpilkin
Copy link
Member Author

I don’t even have the disk space to run nixpkgs-review on this, so if someone would do that I’d be grateful :-/

@gador
Copy link
Member

gador commented May 9, 2022

Running rg -g '*.pc' '}//nix' /nix/store additionally outputs:

  • openobex
  • libeml
  • libffmpegthumbnailer
  • libmatroska
  • libKActivities
  • KF5CalendarCore
  • libeml

@alexshpilkin
Copy link
Member Author

@gador Thanks!

(Also, damn it, I’ve already read enough CMake today to last me a year... I’ll get to those as well, I promise, I really will.)

Right, my system also had kwayland, I forgot about that. The KDE people use some sort of .pc generator written in CMake that seems to produce uniformly broken pkg-config files, except I’m not sure if they are actually shipping them. Someone more versed in CMake should probably have a look at it.

@SuperSandro2000
Copy link
Member

Either we find the package that is causing so many rebuilds or we send everything trough staging.

@alexshpilkin
Copy link
Member Author

@SuperSandro2000 My money is on either libjxl or spirv-tools being deep in the dependency tree of some desktop environment or widget toolkit, but I’m honestly clueless here. Is there even a way to view the list ofborg computed?

@nh2
Copy link
Contributor

nh2 commented May 9, 2022

Is there even a way to view the list ofborg computed?

You can click on

ofborg-eval — ^.^!

in the Github Actions list:

image

That brings up the changed paths: https://gist.github.com/GrahamcOfBorg/67f0505e1d8f9c25d56caa663fbb64ad

@nh2
Copy link
Contributor

nh2 commented May 9, 2022

dependency tree of some desktop environment or widget toolkit

Yes, gtk4 is in the list because of gstreamer:

% nix why-depends nixpkgs.gtk4 nixpkgs.gst_all_1.gst-plugins-bad
/nix/store/699xjlyv5n4k4qcwhgxiv0vhv4m5y2kx-gtk4-4.4.1
╚═══lib/gtk-4.0/4.0.0/media/libmedia-gstreamer.so: …gins-base-1.18.4/lib:/nix/store/m3jdcmxgn44v5xz9qikj3hjkcda58awf-gst-plugins-bad-1.18.2/lib:/nix…
    => /nix/store/m3jdcmxgn44v5xz9qikj3hjkcda58awf-gst-plugins-bad-1.18.2

@alexshpilkin
Copy link
Member Author

@nh2 Right, the smiley! ^^ I knew I saw that list at one point, but could not guess which line of the report to poke at and started doubting myself.

@SuperSandro2000 So, I was right that this is a GUI cascade but wrong about the source.

The majority of the rebuilds are because of both gtk4 and phonon-backend-gstreamer depending on gst-plugins-bad (which this touches in an incidental drive-by manner) which in turn depends on wildmidi (which this touches in an essential manner). Some of the Qt stuff is triggered by libkdegames depending on openal-soft (which this also touches, indepenently). Other parts of the Qt stuff seem to be spurious as they’re marked broken.

Overall, I think punting the wildmidi and the gst-plugins-bad changes to staging should redirect most of the avalanche—or we can just give up and decide this is going to be a large rebuild. Again, I’m clueless here, so please tell me if you want me to cherry-pick some things to a separate branch or something like that.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/extend-include-search-path-for-a-specific-package/19438/2

let

joinpaths-src = fetchurl {
url = "https://AnotherFoxGuy.com/CMakeCM/modules/JoinPaths.cmake";
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

done in #181875

@Artturin
Copy link
Member

fix'd up and resolved conflicts #181875

@Artturin
Copy link
Member

Artturin commented Sep 8, 2022

Merged #181875
Thanks @alexshpilkin !

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.

OpenAL gives wrong include and libs path with pkg-config
10 participants