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

Provide namespaced CMake targets #4737

Merged
merged 4 commits into from
Jan 8, 2025
Merged

Conversation

dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Jan 5, 2025

Description

Projects (e.g. msh3) that use CMake targets from dependencies (here: msquic) should use namespaced target names from those dependencies. This enables CMake to recognize the intention to use a target, not a library name. CMake can report a missing target definition early (at generation time), instead of creating linker instruction which may fail at build time.

msquic already added namespace msquic:: to the exported CMake config. However, this should be complemented with msquic:: ALIAS targets inside the build, for reverse dependencies which integrate msquic via add_subdirectory. This PR adds the missing targets.

In addition the PR shortens the exported name of msquic_platform to msquic::platform.

The msquic::base_link alias is added for use in msh3. (This nicely illustrate how the namespace clarifies the origin of base_link.)

Testing

Covered by src/platform/unittest/external/CMakeLists.txt.

The msquic/msh3 integration with namespaced targets is tested in https://github.com/microsoft/vcpkg/pull/43018/files, with a DLL build of msquic passed to msh3 via CMake config.(Blocked.)

Documentation

No change due to lack of specific CMake usage documentation.
(vcpkg prints an generated hint which will need to be reduced to linking msquic::msquic.)

dg0yt added 3 commits January 5, 2025 15:17
With the export namespace added, the result is 'msquic::platform'.
Allows users to use namespaced CMake targets, regardless of
using a subdirectory or the CMake config package.
@dg0yt dg0yt requested a review from a team as a code owner January 5, 2025 14:44
nibanks
nibanks previously approved these changes Jan 5, 2025
@nibanks nibanks added Area: Build external Proposed by non-MSFT labels Jan 5, 2025
@dg0yt
Copy link
Contributor Author

dg0yt commented Jan 5, 2025

This PR should be good, but the integration in vcpkg cannot complete, nibanks/msh3#235 (comment).

@nibanks
Copy link
Member

nibanks commented Jan 5, 2025

CI is failing

Copy link

codecov bot commented Jan 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.96%. Comparing base (9610803) to head (4c7ff57).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4737      +/-   ##
==========================================
- Coverage   85.71%   84.96%   -0.76%     
==========================================
  Files          56       56              
  Lines       17378    17378              
==========================================
- Hits        14896    14765     -131     
- Misses       2482     2613     +131     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nibanks nibanks enabled auto-merge (squash) January 7, 2025 20:36
@nibanks nibanks merged commit 39c6b52 into microsoft:main Jan 8, 2025
486 of 487 checks passed
@dg0yt dg0yt deleted the cmake-targets branch January 8, 2025 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Build external Proposed by non-MSFT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants