Fix broken CMake-generated pkg-config files #181875
Conversation
Artturin
force-pushed
the
fix-pkg-config-2
branch
from
58bfd68
to
ffba73f
Compare
| # https://github.com/pothosware/SoapySDR/issues/352 | ||
| postPatch = '' | ||
| substituteInPlace lib/SoapySDR.in.pc \ | ||
| --replace '$'{exec_prefix}/@CMAKE_INSTALL_LIBDIR@ @CMAKE_INSTALL_FULL_LIBDIR@ \ |
There was a problem hiding this comment.
''${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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Alright, do you think this is ready to merge?
Artturin
requested review from
misuzu,
dotlambda,
nh2,
bjornfor,
SuperSandro2000,
jtojnar,
Ralith,
jagajaga,
fgaz,
ftrvxmtrx,
matthewbauer and
fufexan
Artturin
force-pushed
the
fix-pkg-config-2
branch
from
ffba73f
to
a164408
Compare
Artturin
force-pushed
the
fix-pkg-config-2
branch
from
69ef428
to
787dd2f
Compare
Artturin
force-pushed
the
fix-pkg-config-2
branch
from
787dd2f
to
3643102
Compare
|
all of these were built successfully list generated with |
|
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 \ |
There was a problem hiding this comment.
Why would pc file of a vendored library be relevant?
|
|
Yeah but I removed the patch in c9b34f7 |
|
Oh, right. I completely missed. Will double-check locally if staging still fails. UPDATE: |
ghost
mentioned this pull request
…ncludedir
Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix
See NixOS/nixpkgs#181875 for some context
…ncludedir
Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix
See NixOS/nixpkgs#181875 for some context
…ncludedir
Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix
See NixOS/nixpkgs#181875 for some context
…ncludedir
Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix
See NixOS/nixpkgs#181875 for some context
…ncludedir
Fixes pkg-config file when CMAKE_INSTALL_{INCLUDE,LIB}DIR is absolute, such as with Nix
See NixOS/nixpkgs#181875 for some context
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.
Description of changes
Cleaned up #172150
See #172150
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes