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

Move git sha header to better location #1303

Closed
wants to merge 1 commit into from

Conversation

chapman39
Copy link
Collaborator

@chapman39 chapman39 commented Dec 27, 2024

I've been testing a serac-only lido build and noticed something weird about the git sha header... in serac, git_sha.hpp is being configured into ${PROJECT_BINARY_DIR}/include/serac/infrastructure, but in smith (which is what lido has been testing against) configures it in an entirely different location: ${CMAKE_BINARY_DIR}/include/serac/infrastructure.

Smith shouldn't need to configure this file in the first place, so it has been removed in the associated Smith MR.

I've updated Serac and Smith so that this header is being configured to a consistent location: ${CMAKE_BINARY_DIR}/include/serac

@chapman39 chapman39 added the bug Something isn't working label Dec 27, 2024
@chapman39 chapman39 self-assigned this Dec 27, 2024
@chapman39 chapman39 added the build Build system label Dec 27, 2024
@chapman39
Copy link
Collaborator Author

I'm not sure if this conflicts with what you were trying to do in this PR @white238 #637 , but we can talk about it after break.

@chapman39
Copy link
Collaborator Author

Please do not update this branch with develop until it's finalized, since Id like to get this change into lido without doing a tpl rebuild.

@white238
Copy link
Member

We should be using PROJECT_BINARY_DIR instead of CMAKE_BINARY_DIR in every place we can because it guarantees there will never be a conflict with downstream projects.

This is should also be fixed:

$<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/include>

Were there conflicts you seeing or was it inconsistency?

@chapman39
Copy link
Collaborator Author

Were there conflicts you seeing or was it inconsistency?

This is the error I was seeing, since the LiDO build was relying on Smith to configure the git sha header again in a different location.

/usr/workspace/meemee/lido-2.0/repo/smith/serac/src/serac/infrastructure/about.cpp:47:10: fatal error: 'serac/infrastructure/git_sha.hpp' file not found
#include "serac/infrastructure/git_sha.hpp"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

@chapman39
Copy link
Collaborator Author

This is should also be fixed:

I think that one fix is all that was needed after all

@chapman39
Copy link
Collaborator Author

see #1304

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working build Build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants