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: remove a warning about scope #1468

Merged
merged 1 commit into from
Jan 7, 2025
Merged

cmake: remove a warning about scope #1468

merged 1 commit into from
Jan 7, 2025

Conversation

illwieckz
Copy link
Member

Remove this warning:

CMake Warning (dev) at CMakeLists.txt:136 (set):
  Cannot set "NACL_TARGETS": current scope has no parent.
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Warning (dev) at /usr/share/cmake-3.28/Modules/CMakeDependentOption.cmake:89 (message):
  Policy CMP0127 is not set: cmake_dependent_option() supports full Condition
  Syntax.  Run "cmake --help-policy CMP0127" for policy details.  Use the
  cmake_policy command to set the policy and suppress this warning.
Call Stack (most recent call first):
  CMakeLists.txt:167 (cmake_dependent_option)
This warning is for project developers.  Use -Wno-dev to suppress it.

That lines look like a mistake, everything is either in the same scope, either passed through command line to the child cmake, there is nothing to bring up.

@slipher
Copy link
Member

slipher commented Dec 19, 2024

NACL_TARGETS is set in Daemon's CMakeLists, but consumed by DaemonGame.cmake which is executed in the context of Unvanquished's CMakeLists. So I expect this to be needed for the variable to show up (including daemon with add_subdirectory is what makes it have a separate scope). Otherwise we would have the same problem as with NACL_VMS_PROJECTS being empty.

Probably the right thing to move the code seting NACL_TARGETS inside DaemonGame.cmake so we don't run this code for building gamelogic when only the engine is built.

@illwieckz illwieckz force-pushed the illwieckz/cmake branch 2 times, most recently from d6735a4 to 40dab6d Compare December 19, 2024 22:28
@illwieckz
Copy link
Member Author

Ah, that may explain why I added that line back in the days.

Is the new patch better?

@slipher
Copy link
Member

slipher commented Dec 19, 2024

I mean move the entire block of code that builds the target list in there.

@illwieckz
Copy link
Member Author

OK, now it may be correct.

cmake/DaemonGame.cmake Outdated Show resolved Hide resolved
@illwieckz illwieckz force-pushed the illwieckz/cmake branch 2 times, most recently from 8a503ec to d43d77b Compare January 6, 2025 23:25
@illwieckz illwieckz merged commit 328e89e into master Jan 7, 2025
9 checks passed
@illwieckz illwieckz deleted the illwieckz/cmake branch January 7, 2025 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants