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

srt: Fix wrong srt.pc include path #71669

Merged
merged 1 commit into from Oct 30, 2019

Conversation

nh2
Copy link
Contributor

@nh2 nh2 commented Oct 22, 2019

Motivation for this change

Fixes #70872.

Fix copied from:

# the cmake package does not handle absolute CMAKE_INSTALL_INCLUDEDIR correctly
# (setting it to an absolute path causes include files to go to $out/$out/include,
# because the absolute path is interpreted with root at $out).
cmakeFlags = "-DCMAKE_INSTALL_INCLUDEDIR=include";

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @nh2

FYI @j0sh

@nh2 nh2 self-assigned this Oct 22, 2019
@nh2
Copy link
Contributor Author

nh2 commented Oct 22, 2019

After merging this should be backported to release-19.09.

@j0sh
Copy link

j0sh commented Oct 23, 2019

Thanks for this! Can confirm with a quick test [1] that this fix works.

[1] Overriding the cmakeFlags attribute to insert the new flag:

let
  nixpkgs = import <nixpkgs> {};
  srt = nixpkgs.srt.overrideAttrs (oldAttrs: rec {  cmakeFlags = nixpkgs.srt.cmakeFlags ++ [ "-DCMAKE_INSTALL_INCLUDEDIR=include" ]; }) ;
in
  with nixpkgs;
  stdenv.mkDerivation rec {
    name = "SRT_Fixed";
    env = buildEnv  { name = name; paths = buildInputs; };
    buildInputs = [ pkg-config srt ];
  }

Results in:

$ pkg-config --cflags --libs srt
-I/nix/store/82dkq0hb40ifgqm0kdg2mwkbpnsgjmip-srt-1.3.2/include -I/nix/store/82dkq0hb40ifgqm0kdg2mwkbpnsgjmip-srt-1.3.2/include/srt -L/nix/store/82dkq0hb40ifgqm0kdg2mwkbpnsgjmip-srt-1.3.2/lib -lsrt

@nh2 nh2 merged commit e64c3ab into NixOS:master Oct 30, 2019
worldofpeace added a commit to worldofpeace/nixpkgs that referenced this pull request Feb 26, 2020
@worldofpeace worldofpeace mentioned this pull request Feb 26, 2020
10 tasks
worldofpeace added a commit that referenced this pull request Feb 29, 2020
This fix comes from #71669.

(cherry picked from commit ff41002)
nh2 added a commit that referenced this pull request Nov 2, 2021
@nh2
Copy link
Contributor Author

nh2 commented Nov 2, 2021

Linking issue that collects such CMake path issues: #144170

nh2 added a commit that referenced this pull request Nov 18, 2021
@GaetanLepage GaetanLepage mentioned this pull request Mar 9, 2024
13 tasks
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.

srt pkg-config include path incorrect
2 participants