Skip to content

fix: correctly specify dependency visibility of internal library - #100

Merged
Mikolaj-A-Kowalski merged 1 commit into
mainfrom
quick-fix-dependency
Oct 29, 2026
Merged

Mikolaj-A-Kowalski merged 1 commit into
mainfrom
quick-fix-dependency

Conversation

@Mikolaj-A-Kowalski

Copy link
Copy Markdown
Contributor

This fixes the build issue I had on CSD3.

The problem was in the configuration. It was building on my system since it just happened that netcdf libraries were installed in the same as the other library (I think MPI but I am not sure). Anyway a search path provided with -L when linking was correct by accident.

This is a quick-fix to resolve the problem.

While investigated I have learned about the OBJECT libraries in CMake which may be better way to solve the problem static library is trying to solve. But this should be a follow-up issue.

@jatkinson1000 jatkinson1000 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not run myself, but your reasoning at the code changes look sensible.

Only comment is that I do not quite understand the comment and have suggested what I think to be clearer wording at your discretion.

Comment thread CMakeLists.txt Resolved
Since the internal library is static, we need to propagate the
dependencies to the targets that use it so they can resolve all symbols
when being linked.

The include directories should stay private since we don't expose
dependency headers in the public headers.

By public headers we mean:
 - Grid.hpp
 - Partitioner.hpp
 - DomainUtlis.hpp

Co-authored-by: Jack Atkinson <109271713+jatkinson1000@users.noreply.github.com>
@Mikolaj-A-Kowalski
Mikolaj-A-Kowalski merged commit d4da3b1 into main Oct 29, 2026
3 checks passed
@github-project-automation github-project-automation Bot moved this from Review required to Done in neXtSIM_DG overview Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants