Skip to content

Conversation

@dougiesquire
Copy link

@dougiesquire dougiesquire commented Jan 6, 2026

Description

In the CMake build, when using WITH_YAML:

  • if a kind is specified, no linking is done against libyaml
  • if a kind is not specified, the following is done:
    target_link_libraries(${libTgt}_c PRIVATE libyaml_C)
    set_target_properties(${libTgt}_c PROPERTIES COMPILE_FLAGS "-L${LIBYAML_LIBRARIES} -lyaml")
    But, I think the final FMS library (${libTgt}) just copies the object files and the link dependencies are not propagated anyway:
    add_library(${libTgt} STATIC $<TARGET_OBJECTS:${libTgt}_c>
                                 $<TARGET_OBJECTS:${libTgt}_f>)

Thus, though the FMS library compiles, when I try to build an executable, I see:

  >> 172    yaml_output_functions.c:(.text+0x54): undefined reference to `yaml_scalar_event_initialize'
  >> 173    yaml_output_functions.c:(.text+0x63): undefined reference to `yaml_emitter_emit'
  ...

This PR tries to fix this by:

  • Linking ${libTgt}, rather than ${libTgt}_c, against libyaml
  • Adding libyaml as a dependency to FMSConfig.cmake
  • Installing Findlibyaml.cmake alongside the exported configuration UPDATE: libyaml is now found using pkg-config so Findlibyaml.cmake has been removed.

How Has This Been Tested?
With these changes I am able to install our ocean model with FMS yaml support, where previously I wasn't.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

@dougiesquire
Copy link
Author

dougiesquire commented Jan 6, 2026

Note, we could also get rid of Findlibyaml.cmake and use pkg-config to handle the dependency, but note that the .pc file for libyaml is strangely named yaml-0.1.pc:

-  find_package(libyaml REQUIRED)
+  find_package(PkgConfig REQUIRED)
+  pkg_check_modules(YAML REQUIRED IMPORTED_TARGET "yaml-0.1")

rem1776
rem1776 previously approved these changes Jan 6, 2026
Copy link
Contributor

@rem1776 rem1776 left a comment

Choose a reason for hiding this comment

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

LGTM!

@rem1776
Copy link
Contributor

rem1776 commented Jan 6, 2026

Note, we could also get rid of Findlibyaml.cmake and use pkg-config to handle the dependency, but note that the .pc file for libyaml is strangely named yaml-0.1.pc:

-  find_package(libyaml REQUIRED)
+  find_package(PkgConfig REQUIRED)
+  pkg_check_modules(YAML REQUIRED IMPORTED_TARGET "yaml-0.1")

No issues here with using pkg-config instead if you would like to include that change.

@dougiesquire
Copy link
Author

No issues here with using pkg-config instead if you would like to include that change.

Thanks @rem1776. Done in b57d653.

@rem1776
Copy link
Contributor

rem1776 commented Jan 7, 2026

Looks good and works on my system but is failing in our CI testing. I'll take a look and try to get it passing as it should, its probably from the github workflow messing with the shell.

rem1776
rem1776 previously approved these changes Jan 7, 2026
Copy link
Contributor

@uramirez8707 uramirez8707 left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this.

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