Skip to content

Fix broken CMake-generated pkg-config files#181875

Merged
Artturin merged 74 commits intoNixOS:stagingfrom
Artturin:fix-pkg-config-2
Sep 8, 2022
Merged

Fix broken CMake-generated pkg-config files#181875
Artturin merged 74 commits intoNixOS:stagingfrom
Artturin:fix-pkg-config-2

Conversation

@Artturin
Copy link
Copy Markdown
Member

Description of changes

Cleaned up #172150
See #172150

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.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Artturin Artturin requested a review from ttuegel as a code owner July 17, 2022 18:47
@github-actions github-actions bot added the 6.topic: qt/kde Object-oriented framework for GUI creation label Jul 17, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jul 17, 2022
@Artturin Artturin requested a review from alexshpilkin July 19, 2022 16:58
Copy link
Copy Markdown
Member Author

@Artturin Artturin Jul 23, 2022

Choose a reason for hiding this comment

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

@alexshpilkin

''${something} is the proper way to escape ${

Please confirm If that is what you were trying to do then I'll change them all to that

Copy link
Copy Markdown
Member

@alexshpilkin alexshpilkin Jul 23, 2022

Choose a reason for hiding this comment

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

No, that wasn’t it. This is more clever than I would like, but it has a purpose: there are two levels of escaping here, from Nix when it’s generating the script and from Bash when it’s executing it. That is to say, the file we’re patching contains the literal string ${exec_prefix}/@CMAKE_INSTALL_LIBDIR@ in it—the ${...} is not shell syntax, it is pkg-config syntax. If you wanted to do this with standard Nix escaping as you suggest, you would end up with \''${exec_prefix} or '''${exec_prefix}', which is less subtle but even harder to parse I think.

(This is when .in files are being patched; take care that in some cases, CMake sources are being patched instead—so the interpolation is passing through Nix, Bash, and CMake on the way to the pkg-config file—and the CMake file being patched is already escaping the variable reference in one of a couple ways I’ve seen.)

If you see a clearer way, go for it, but the change you’re suggesting here wouldn’t work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Alright, do you think this is ready to merge?

Copy link
Copy Markdown
Contributor

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

openal-soft LGTM

@Artturin
Copy link
Copy Markdown
Member Author

Artturin commented Sep 7, 2022

all of these were built successfully

list generated with git log upstream/staging..HEAD --oneline | cut -d' ' -f2 | sd ':' '' | uniq

libiio
libzra
xtensor
xsimd
libcotp
libbaseencode
tinyobjloader
tdlib
spirv-headers
spdlog
soapysdr
shadowsocks-libev
sentencepiece
seexpr
rocm-thunk
rnp
rinutils
reproc
recastnavigation
rabbitmq-c
proj
powercap
orcania
openxr-loader
opendht
opencolorio
olm
obexftp
notcurses
nanomsg
mysocketw
maliit-framework
libunarr
libtsm
libtorrent-rasterbar
libsurvive
libquotient
libnats-c
libmodule
lxqt.lxqt-build-tools
libkeyfinder
libdnf
libcotp
libcork
libbtbb
libbaseencode
google-cloud-cpp
getdns
gbenchmark
entt
eccodes
plasma5Packages.drumstick
cxxopts
cog
cm256cc
cglm
bcg729
bcc
arrow-cpp
libargs
libebml
extra-cmake-modules
libmatroska
ffmpegthumbnailer
openobex
libebur128
openalSoft
usrsctp
wildmidi
libjxl
spirv-tools
gst_all_1.gst-plugins-bad
zxing

@Artturin
Copy link
Copy Markdown
Member Author

Artturin commented Sep 7, 2022

So i am going to merge this

--replace '$'{prefix}/@CMAKE_INSTALL_MANDIR@ @CMAKE_INSTALL_FULL_MANDIR@

# https://github.com/dcreager/libcork/issues/173 but needs a different patch (yay vendoring)
substituteInPlace libcork/src/libcork.pc.in \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would pc file of a vendored library be relevant?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@trofi
Copy link
Copy Markdown
Contributor

trofi commented Sep 9, 2022

openalSoft claims to fail patch hash after patch added in this PR. Does it work for you on staging?

$ nix build --no-link -f. openalSoft
...
error: hash mismatch in fixed-output derivation '/nix/store/2pmlpjzkd3yyayxkfnqhpl84mhbq9i7x-0bf2abae9b2121c3bc5a56dab30eca308136bc29.patch.drv':
         specified: sha256-QFio2LIEbF8VzLL8fRDRRusBkbtYV8qBGjLDbI+XEro=
            got:    sha256-IUm/5Aj8e1rTgvOL9wB08ilLWQXGPptfIRayMjkt2+A=
error (ignored): error: cannot unlink '/tmp/nix-build-util-linux-minimal-2.3

@Artturin
Copy link
Copy Markdown
Member Author

openalSoft claims to fail patch hash after patch added in this PR. Does it work for you on staging?

$ nix build --no-link -f. openalSoft
...
error: hash mismatch in fixed-output derivation '/nix/store/2pmlpjzkd3yyayxkfnqhpl84mhbq9i7x-0bf2abae9b2121c3bc5a56dab30eca308136bc29.patch.drv':
         specified: sha256-QFio2LIEbF8VzLL8fRDRRusBkbtYV8qBGjLDbI+XEro=
            got:    sha256-IUm/5Aj8e1rTgvOL9wB08ilLWQXGPptfIRayMjkt2+A=
error (ignored): error: cannot unlink '/tmp/nix-build-util-linux-minimal-2.3

Yeah but I removed the patch in c9b34f7

@trofi
Copy link
Copy Markdown
Contributor

trofi commented Sep 10, 2022

Oh, right. I completely missed. Will double-check locally if staging still fails. UPDATE: openalSoft works fine in staging now.

@ghost ghost mentioned this pull request Nov 7, 2022
2 tasks
LunNova added a commit to LunNova/sail that referenced this pull request Jan 23, 2023
…ncludedir

Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix

See NixOS/nixpkgs#181875 for some context
LunNova added a commit to LunNova/sail that referenced this pull request Jan 23, 2023
…ncludedir

Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix

See NixOS/nixpkgs#181875 for some context
LunNova added a commit to LunNova/sail that referenced this pull request Jan 23, 2023
…ncludedir

Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix

See NixOS/nixpkgs#181875 for some context
LunNova added a commit to LunNova/sail that referenced this pull request Jan 23, 2023
…ncludedir

Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix

See NixOS/nixpkgs#181875 for some context
LunNova added a commit to LunNova/sail that referenced this pull request Jan 23, 2023
…ncludedir

Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix

See NixOS/nixpkgs#181875 for some context
rtimush pushed a commit to rtimush/nixpkgs that referenced this pull request Sep 21, 2023
NixOS#181875 seems to have made this
package and its downstream dependencies no longer build on
avahi-less systems.  Let's make it possible for them to build again.
@alexshpilkin alexshpilkin mentioned this pull request Jul 17, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: qt/kde Object-oriented framework for GUI creation 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants