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

libyamlcpp: don't use multiple outputs #81004

Merged
merged 1 commit into from Feb 25, 2020

Conversation

lopsided98
Copy link
Contributor

@lopsided98 lopsided98 commented Feb 25, 2020

Motivation for this change

This package uses CMake's install(EXPORT ...) command which assumes that libraries can be found relative to the CMake files. This results in the following error when this package is imported by another CMake package:

CMake Error at /nix/store/n77s62wwd9m7cjqd4hky91h2khkiy06n-libyaml-cpp-0.6.3-dev/lib/cmake/yaml-cpp/yaml-cpp-targets.cmake:74 (message):
  The imported target "yaml-cpp" references the file

     "/nix/store/n77s62wwd9m7cjqd4hky91h2khkiy06n-libyaml-cpp-0.6.3-dev/lib/libyaml-cpp.so.0.6.3"

  but this file does not exist.  Possible reasons include:

  * The file was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and contained

     "/nix/store/n77s62wwd9m7cjqd4hky91h2khkiy06n-libyaml-cpp-0.6.3-dev/lib/cmake/yaml-cpp/yaml-cpp-targets.cmake"

  but not all the files it references.

Call Stack (most recent call first):
  /nix/store/n77s62wwd9m7cjqd4hky91h2khkiy06n-libyaml-cpp-0.6.3-dev/lib/cmake/yaml-cpp/yaml-cpp-config.cmake:11 (include)
  src/rviz/CMakeLists.txt:1 (find_package)

Disabling multiple outputs only results in a small increase in closure size (936K -> 1.1M).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

cc @andir

This package uses CMake's install(EXPORT ...) command which assumes that
libraries are installed in the same location as the CMake files.
@lopsided98
Copy link
Contributor Author

nixpkgs-review was successful except for ip2unix which was already broken.

@andir
Copy link
Member

andir commented Feb 25, 2020

I am not really liking this as we would ship the development headers for everyone using the lib. IIRC there was some discussions a few years ago about exactly this and I thought we fixed this. I'll look into this again.

@andir
Copy link
Member

andir commented Feb 25, 2020

Actually since it is just static linking there isn't much of a downside.

Thank you for looking at this 👍

@andir andir merged commit 52a076e into NixOS:master Feb 25, 2020
@lopsided98 lopsided98 deleted the libyamlcpp-single-output branch February 25, 2020 20:03
@veprbl veprbl added the 8.has: port to stable A PR already has a backport to the stable release. label Feb 29, 2020
@veprbl veprbl mentioned this pull request Feb 29, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants