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

yaml-cpp-config.cmake.in: apply patch to fix #774 #1037

Closed
wants to merge 1 commit into from

Conversation

Artturin
Copy link

@jbeder
Copy link
Owner

jbeder commented Sep 28, 2021

Can you add a test that confirms this fixes the issue?

@Artturin
Copy link
Author

i do not know cpp

i was updating the rstudio package for nixpkgs and i got this error during configuring

CMake Error at src/cpp/CMakeLists.txt:222 (message):
  yaml-cpp not found (re-run dependencies script to install)

and applying the patch in this pr to yaml-cpp fixed it

  patches = [
    # https://github.com/jbeder/yaml-cpp/issues/774
    (fetchpatch {
      url = "https://github.com/jbeder/yaml-cpp/commit/4f48727b365962e31451cd91027bd797bc7d2ee7.patch";
      sha256 = "sha256-jarZAh7NgwL3xXzxijDiAQmC/EC2WYfNMkYHEIQBPhM=";
    })
  ];


# Our library dependencies (contains definitions for IMPORTED targets)
include("${YAML_CPP_CMAKE_DIR}/yaml-cpp-targets.cmake")

# These are IMPORTED targets created by yaml-cpp-targets.cmake
set(YAML_CPP_LIBRARIES "@EXPORT_TARGETS@")
get_target_property(YAML_CPP_INCLUDE_DIR yaml-cpp INTERFACE_INCLUDE_DIRECTORIES)
get_target_property(YAML_CPP_LIBRARIES yaml-cpp IMPORTED_LOCATION_RELEASE)

Choose a reason for hiding this comment

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

Suggested change
get_target_property(YAML_CPP_LIBRARIES yaml-cpp IMPORTED_LOCATION_RELEASE)
set(YAML_CPP_LIBRARIES "yaml-cpp")

note that this only works with CMAKE_BUILD_TYPE=Release. As the value is static anyhow we could just set it.

@jbeder
Copy link
Owner

jbeder commented Dec 15, 2021

It sounds like this is important to get submitted, but I don't understand it. @Artturin, I think what you're trying to fix is an installation step, right?

If so, can you add a case to the github action: https://github.com/jbeder/yaml-cpp/blob/master/.github/workflows/build.yml

If there's a bash command you can run that does the thing you want, that would help ensure this fixes the issue and nothing breaks it down the road.

@Artturin
Copy link
Author

better solution in #1077

@Artturin Artturin closed this Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants