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 fix #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Cmake fix #1

wants to merge 2 commits into from

Conversation

kristux
Copy link

@kristux kristux commented Sep 4, 2024

No description provided.

@@ -157,6 +157,12 @@ set(common_includes
${TRACY_PUBLIC_DIR}/common/TracyUwp.hpp
${TRACY_PUBLIC_DIR}/common/TracyYield.hpp)

if(CMAKE_SYSTEM_NAME STREQUAL "Windows")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the best way to do this. I need to differentiate otherwise it will overwrite the files for different os.

@kristux
Copy link
Author

kristux commented Sep 10, 2024

There are probably better ways to do this. I didn't really want to include our branch to grab the cmake utilities we have their, rather keep this independent but feel free to rip this apart. We do need to differentiate where we put the config though otherwise the "last thing unzipped" will just overwrite it and in our case we were trying to import a .dll on a windows system

@kristux kristux marked this pull request as ready for review September 10, 2024 08:50
@CCP-Aporia
Copy link

Wait, why are we modifying Tracy here? Those changes are a maintenance burden we don't want to take on. Any changes that are required because of our vendoring constraints need to happen in our own pipelines when we do the vendoring, and not as part of the library we vendor.

@kristux
Copy link
Author

kristux commented Sep 10, 2024

Agreed and there is probably another way to do this that I'm not seeing. We install the tracy config into

install(FILES ${CMAKE_CURRENT_BINARY_DIR}/TracyConfig.cmake
        DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/Tracy)

Which happens both for windows and macos. What it installs is in Config.cmake.in. Oh we can probably just change the CMAKE_INSTALL_PREFIX to include windows and macos and then do the "branching" in CcpGlobalSettings?

@kristux
Copy link
Author

kristux commented Sep 10, 2024

or really just set the CMAKE_INSTALL_DATAROOTDIR

@CCP-Aporia
Copy link

or really just set the CMAKE_INSTALL_DATAROOTDIR

Perhaps this is possible, and would be a good approach. Alternately, if that doesn't work out, then we might be able to copy files around during the publish process. I am not familiar with how that is currently set up, but usually that isn't very difficult as it can be as simple as an additional build step in the given TeamCity configuration.

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