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

cmake: export targets in config files #2800

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RMZeroFour
Copy link
Contributor

This PR reworks Notcurses' CMake package configuration to provide logical targets (Notcurses::NotcursesCore, Notcurses::Notcurses, Notcurses::Notcurses++), following modern CMake conventions, while retaining the previously exported variables for compatibility.

The names of these targets match the proposed Conan recipe targets (conan-io/conan-center-index#4036), allowing developers to use the same target names when the Conan recipe is eventually resolved.
Alias targets have also been added to allow superbuilds (using Notcurses as a git submodule) with the same target names.

Looking into the git history, I can see that Notcurses did at one point export logical targets (#1146), which was later reverted (#1149), since it broke compatibility by not defining variables like before.
This PR fixes that by splitting the logical targets generated by CMake into Notcurses*Targets.cmake files, with the Notcurses*Config.cmake files importing those and also providing the original variables for compatibility.

I've tested these changes both using find_package and config files, and using add_subdirectory and a git submodule. Please verify on your end and let me know if any adjustments are needed.

@dankamongmen
Copy link
Owner

taking a look at it, thanks!

@RMZeroFour
Copy link
Contributor Author

Hello! Just checking in to see if you've had a chance to review the PR, @dankamongmen.

@dankamongmen
Copy link
Owner

after a length delay, looking at this afresh. sorry for the radio silence! let me see if it can easily be brought up to the modern master...

@RMZeroFour
Copy link
Contributor Author

No worries on the delay, take your time. I can help with the rebase after the New Year if needed.

@dankamongmen
Copy link
Owner

yeah if you can bring it up to speed with current master this goes in before the new year

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.

2 participants