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

Fixed custom target dependency for d3_version.h #440

Merged
merged 1 commit into from
Jun 16, 2024

Conversation

pzychotic
Copy link
Contributor

Pull Request Type

  • GitHub Workflow changes
  • Documentation or Wiki changes
  • Build and Dependency changes
  • Runtime changes
    • Render changes
    • Audio changes
    • Input changes
    • Network changes
    • Other changes

Description

cmake/CheckGit.cmake uses configure_file() to create d3_version.h in ${TARGET_DIR}/lib aka ${PROJECT_BINARY_DIR}/lib. But add_custom_command() and add_custom_target() calls in the root CMakeLists.txt use ${PROJECT_BINARY_DIR}/Descent3 as path to that file, expecting it in a different place. While the build still works, since the compiler looks in the right include paths for it, the dependencies between these rules/targets are technically broken.
At least msbuild will generate warnings about missing build outputs. You can see it on CI builds before we switched to Ninja: https://github.com/DescentDevelopers/Descent3/pull/429/checks#step:6:405

Also removed the wrong ALL parameter from add_custom_command(), it's only a valid parameter for add_custom_target().
See: https://github.com/DescentDevelopers/Descent3/pull/429/checks#step:6:406

Related Issues

Screenshots (if applicable)

Checklist

  • I have tested my changes locally and verified that they work as intended.
  • I have documented any new or modified functionality.
  • I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
  • I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

…wrong path to that file.

cmake/CheckGit.cmake uses configure_file() to create d3_version.h in ${TARGET_DIR}/lib aka ${PROJECT_BINARY_DIR}/lib. But add_custom_command() and add_custom_target() calls in the root CMakeLists.txt use ${PROJECT_BINARY_DIR}/Descent3 as path to that file, expecting it in a different place. While the build still works, since the compiler looks in the right include paths for it, the dependencies between these rules/targets are technically broken. At least msbuild will generate warnings about missing build outputs.

Also removed the wrong ALL parameter from add_custom_command(), it's only a valid parameter for add_custom_target().
@winterheart
Copy link
Collaborator

Thanks for pointing it out.

@winterheart winterheart merged commit 87472f4 into DescentDevelopers:main Jun 16, 2024
12 checks passed
@pzychotic pzychotic deleted the fix-build-deps branch June 16, 2024 16:59
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