Skip to content

chore: optimize cmake#362

Open
neko-para wants to merge 3 commits intoBill-Gray:masterfrom
neko-para:feat/optimize_cmake
Open

chore: optimize cmake#362
neko-para wants to merge 3 commits intoBill-Gray:masterfrom
neko-para:feat/optimize_cmake

Conversation

@neko-para
Copy link
Copy Markdown
Contributor

@neko-para neko-para commented Jan 9, 2026

  • Install headers to ${CMAKE_BUILD_TYPE}/include.

  • Set CMAKE_INSTALL_RPATH for demos on macos and linux. Previously the installed demos cannot be executed.

  • Set CMAKE_C_VISIBILITY_PRESET to hidden on macos and linux, which prevent about 60 symbols being exported.

@GitMensch
Copy link
Copy Markdown
Collaborator

@juliusikkala @trexxet @jwinarske Can someone of you have a look at this PR?
(If two agree then I'd pull, but as I'm no cmake person...)

Copy link
Copy Markdown
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

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

only commenting on non-cmake parts...

Comment thread curses.h
Comment on lines +401 to +405
# ifdef PDC_ENABLE_VISIBILITY
# define PDCEX __attribute__((visibility("default"))) extern
# else
# define PDCEX extern
# endif
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

that should be moved out to a separate PR

Comment thread CMakeLists.txt

endif()

install(FILES curses.h panel.h DESTINATION ${CMAKE_BUILD_TYPE}/include)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think those should be in subdirectory pdcurses along with installing term.h and this should also install pdcurses.h as a wrapper with the appropriate defines and including the pdcurses/curses.h - see https://github.com/msys2/MINGW-packages/blob/86e9985a9bccf10920d28741e91394ba44741986/mingw-w64-pdcurses/PKGBUILD#L109-L116

Copy link
Copy Markdown
Contributor

@juliusikkala juliusikkala left a comment

Choose a reason for hiding this comment

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

The CMake portion looks OK to me.

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