Skip to content

Use relative cmake paths#366

Merged
GitMensch merged 4 commits intoBill-Gray:masterfrom
trexxet:cmake-fetchcontent
Apr 21, 2026
Merged

Use relative cmake paths#366
GitMensch merged 4 commits intoBill-Gray:masterfrom
trexxet:cmake-fetchcontent

Conversation

@trexxet
Copy link
Copy Markdown
Contributor

@trexxet trexxet commented Jan 24, 2026

Replace CMAKE_SOURCE_DIR with relative path counterparts.

These changes should allow using the library as an external dependency with the FetchContent, as for that case CMAKE_SOURCE_DIR points to the top-level CMakeLists.txt.

@Bill-Gray
Copy link
Copy Markdown
Owner

Thank you! I know virtually nothing of cmake, and was hoping somebody more knowledgeable about it would step in here.

It appears not to have broken the AppVeyor builds (which use cmake; the Github Actions builds all rely on more 'traditional' Makefiles). Those builds don't test the actual ability to use the library with FetchContent. But I'd think that if this Works On Your Machine™ and with the AppVeyor builds, and no one points out problems, we could merge this and close the underlying issue.

@Bill-Gray Bill-Gray marked this pull request as ready for review January 25, 2026 15:28
@trexxet
Copy link
Copy Markdown
Contributor Author

trexxet commented Jan 25, 2026

Thanks. There are a couple more things to fix (include paths etc.), so don't merge it now.

@trexxet trexxet marked this pull request as draft January 25, 2026 21:05
- add scopes to target_link_libraries
- add pdcurses_include_dirs interface target
@trexxet trexxet marked this pull request as ready for review January 26, 2026 19:37
@trexxet
Copy link
Copy Markdown
Contributor Author

trexxet commented Jan 26, 2026

Ok, seems to be working now, and CI is fine.

By the way, is there any reason to have PDC_SDL2_BUILD, PDC_SDL2_DEPS_BUILD & PDC_GL_BUILD enabled by default?

@GitMensch
Copy link
Copy Markdown
Collaborator

GitMensch commented Apr 9, 2026

@juliusikkala @neko-para @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...)

Note: I also think that the SDL + GL parts should be optional (needs an adjustment to appveyor.yml when conditional)

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.

Looks good to me, and I also agree that SDL + GL should be optional.

Although, it looks like I'm not able to build the project on my system anymore (OpenSUSE Tumbleweed). It's been a while since I last worked on my PDCursesMod-dependent project. I don't think these problems are caused by this PR; in fact, this removes some of the errors I see when trying to build master, so that's a good sign at least. It looks like the SDL & zlib deps need to be updated, as the old versions appear to not work with newer CMake versions that broke compatibility with CMake < 3.5. But that's not something that has to be dealt with in this PR, so I opened a new separate PR for updating those.

@GitMensch
Copy link
Copy Markdown
Collaborator

@Bill-Gray if you happen to be online, please cancel all older Appveyor builds as these take very long - and while I can start them with a push to a branch or by allowing the workflow to run, I can't do anything on Appveyor itself (if you want to, you should be able to adjust that at https://ci.appveyor.com/team)

@GitMensch GitMensch merged commit f32932f into Bill-Gray:master Apr 21, 2026
4 checks passed
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.

4 participants