Skip to content

CMake install should use the Tracy:: namespace#326

Merged
wolfpld merged 1 commit intowolfpld:masterfrom
Lectem:install_namepace
Feb 12, 2022
Merged

CMake install should use the Tracy:: namespace#326
wolfpld merged 1 commit intowolfpld:masterfrom
Lectem:install_namepace

Conversation

@Lectem
Copy link
Copy Markdown
Collaborator

@Lectem Lectem commented Feb 11, 2022

If we do not add the namespace to the install config, it means we can not use Tracy::TracyClient as target name when using find_package while we already have the alias (when using add_subdirectory).

This would yield the following error:

CMake Error at CMakeLists.txt:105 (add_library):
  Target "sedica" links to target "Tracy::TracyClient" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?

This is also an issue the case where you install a library that builds tracy with add_subdirectory and links Tracy::TracyClient then installs itself.

Note that this is a breaking change, as I suppose people were linking TracyClient instead of Tracy::TracyClient (which is dangerous because there is no guarantee it would link the version found by CMake without using an alias).
The issue is that CMake will not show any error but not propagate the includes if it does not find the target when not using an alias (which is exactly what we're trying to avoid).
So in the short term it might be an issue, but in the long term I think this is better.

@Lectem
Copy link
Copy Markdown
Collaborator Author

Lectem commented Feb 11, 2022

Pinging @Honeybunch who seems to have done the install target changes for VCPKG

@Honeybunch
Copy link
Copy Markdown
Contributor

I actually just realized this when building a new project that uses Tracy from vcpkg. This change is good and I'll create a PR to update the vcpkg patch to 0.7.8

@wolfpld
Copy link
Copy Markdown
Owner

wolfpld commented Feb 12, 2022

I see discrepancy between this PR (NAMESPACE Tracy::) and the vcpkg one (NAMESPACE Tracy). Which one should be used?

@Lectem
Copy link
Copy Markdown
Collaborator Author

Lectem commented Feb 12, 2022

It has to be Tracy:: as it is only prepended to the target name, and the columns aren't automatically added. (think of it as a prefix)

@wolfpld wolfpld merged commit 72ae4d2 into wolfpld:master Feb 12, 2022
@Lectem Lectem deleted the install_namepace branch February 12, 2022 00:15
@Honeybunch
Copy link
Copy Markdown
Contributor

I see discrepancy between this PR (NAMESPACE Tracy::) and the vcpkg one (NAMESPACE Tracy). Which one should be used?

Yeah that was my bad. I was moving a bit too fast trying to come up with that change but it's corrected now :)

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.

3 participants