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.

Sign up for GitHub

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

Jump to bottom

Add support for header-only libraries with tribits_add_library() #625

Open
bartlettroscoe opened this issue Feb 17, 2025 · 3 comments
Open

Add support for header-only libraries with tribits_add_library() #625

bartlettroscoe opened this issue Feb 17, 2025 · 3 comments
Labels
component: core type: enhancement

Comments

@bartlettroscoe
Copy link
Member

bartlettroscoe commented Feb 17, 2025 •
edited
Loading

Description

With the move to modern CMake, it should now be easy to support header-only libraries with tribits_add_library(). The idea would be to provide a new keyword HEADERONLY that would allow the SOURCES <src0> <src1> ... argument to be missing. Then an interface-only CMake target would get created.

The hardest parts of adding this new feature is adding a test case for all of the supported use cases and adding documentation.

Motivating user is the STK package in Trilinos.
@bartlettroscoe
Copy link
Member Author

I need to decide how to test this. I order to ensure that all use cases are supported and to make it easy for people to find this example, it might be good to just add tribits_add_library( ... HEADERONLY ...) to one of the existing example/tests projects like TribitsExampleProject or TribitsExampleProject2. The pros/cons of adding this to one of these existing example projects:

  • Pro: Ensures gets tested in all use cases (including the generation of exported <Package>ConfigTargets.cmake files).
  • Con: May require updates of a lot of existing tests?

I think that if I am crafty, I may not need to update the existing tests much if at all. I will see how this goes.

@bartlettroscoe bartlettroscoe moved this to In Progress in TriBITS Feb 17, 2025
@bartlettroscoe bartlettroscoe mentioned this issue Feb 18, 2025
Merged
@bartlettroscoe
Copy link
Member Author

FYI: This addresses #51

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
WIP: Add failing test for INTERFACE library not being in all_libs (Tr…
f1e4b76 
…iBITSPub#625)

This failing test shows that an INTERFACE library was not getting picked up
and linked to the <Package>::all_libs target.

The reason the existing tests all passed after adding the initial support for
HEAERONLY INTERFACE libraries was that I added the interface library
'mixedlang_vector' before the main 'mixedlang' library.  So the new
'mixedlang_vector' library getting written to the MixedLangTargets.cmake file
and was getting linked to 'mixedlang' and 'mixedlang' was getting liked to
MixedLang::all_libs.  So, anyone that was linking against MixedLang::all_libs
(from internal or IMPORTED target) was getting the include directories being
specified in the 'mixedlang_vector' library.

But this implementation fails for TriBITS packages that only has INTERFACE
libraries.  Those never get added to the <Package>::all_libs target or
indirectly to targets that do.  This is why the STKEmend package failed
because it has just one HEADERONLY INTERFACE library.

This test ensures that the INTERFACE library does get linked to the
<Package>::all_libs target and will fail if it does not.

The next commit will add the code in TriBITS to fix this test :-)

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
Add INTERFACE_LIBRARY targets to <Package>::all_libs target (TriBITSP…
2fe0f8b 
…ub#625)

This fixes the previously added failing test :-)

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
WIP: Add failing test for INTERFACE library not being in all_libs (Tr…
5cdb929 
…iBITSPub#625)

This failing test shows that an INTERFACE library was not getting picked up
and linked to the <Package>::all_libs target.

The reason the existing tests all passed after adding the initial support for
HEAERONLY INTERFACE libraries was that I added the interface library
'mixedlang_vector' before the main 'mixedlang' library.  So the new
'mixedlang_vector' library is getting written to the MixedLangTargets.cmake
file and is getting linked to 'mixedlang' and 'mixedlang' was getting liked to
MixedLang::all_libs.  So, anyone that was linking against MixedLang::all_libs
or just the 'mixedlang' LIB (from internal or IMPORTED target) was getting the
include directories being specified in the 'mixedlang_vector' library.

But this implementation fails for TriBITS packages that only have INTERFACE
libraries.  Those are never getting added to the <Package>::all_libs target or
indirectly to targets that do.  This is why the STKEmend package failed
because it has just one HEADERONLY INTERFACE library.

This new added test case ensures that the INTERFACE library does get linked to
the <Package>::all_libs target and will fail if it does not.

The next commit will add the code in TriBITS to fix this test :-)

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
Add INTERFACE_LIBRARY targets to <Package>::all_libs target (TriBITSP…
a10a508 
…ub#625)

This fixes the previously added failing test :-)

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
WIP: Add failing test for INTERFACE library not being in all_libs (Tr…
2912c5a 
…iBITSPub#625)

This failing test shows that an INTERFACE library was not getting picked up
and linked to the <Package>::all_libs target.

The reason the existing tests all passed after adding the initial support for
HEAERONLY INTERFACE libraries was that I added the interface library
'mixedlang_vector' before the main 'mixedlang' library.  So the new
'mixedlang_vector' library is getting written to the MixedLangTargets.cmake
file and is getting linked to 'mixedlang' and 'mixedlang' was getting liked to
MixedLang::all_libs.  So, anyone that was linking against MixedLang::all_libs
or just the 'mixedlang' LIB (from internal or IMPORTED target) was getting the
include directories being specified in the 'mixedlang_vector' library.

But this implementation fails for TriBITS packages that only have INTERFACE
libraries.  Those are never getting added to the <Package>::all_libs target or
indirectly to targets that do.  This is why the STKEmend package failed
because it has just one HEADERONLY INTERFACE library.

This new added test case ensures that the INTERFACE library does get linked to
the <Package>::all_libs target and will fail if it does not.

The next commit will add the code in TriBITS to fix this test :-)

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
Add INTERFACE_LIBRARY targets to <Package>::all_libs target (TriBITSP…
8b0ce25 
…ub#625)

This fixes the previously added failing test :-)

Also, this change required the update of some other tests.  Now that
INTERFACE_LIBRARY taragets are getting pulled down, the <SubPkg>::all_libs for
subpackage targets are getting added the <ParentPkg>::all_libs target for
parent packages.  This is an intereting case but it works.

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
Add INTERFACE_LIBRARY targets to <Package>::all_libs target (TriBITSP…
7fb3cbe 
…ub#625)

This fixes the previously added failing test :-)

I also had to add logic to not generate the package-config files when doing
installation testing where you are only building the tests and examples.  This
was generating a strange error about export sets.  You should not be
generating packge-config files in these cases.

Also, this change required the update of some other tests.  Now that
INTERFACE_LIBRARY taragets are getting pulled down, the <SubPkg>::all_libs for
subpackage targets are getting added the <ParentPkg>::all_libs target for
parent packages.  This is an intereting case but it works.

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
WIP: Add failing test for INTERFACE library not being in all_libs (Tr…
61e2239 
…iBITSPub#625)

This failing test shows that an INTERFACE library was not getting picked up
and linked to the <Package>::all_libs target.

The reason the existing tests all passed after adding the initial support for
HEAERONLY INTERFACE libraries was that I added the interface library
'mixedlang_vector' before the main 'mixedlang' library.  So the new
'mixedlang_vector' library is getting written to the MixedLangTargets.cmake
file and is getting linked to 'mixedlang' and 'mixedlang' was getting liked to
MixedLang::all_libs.  So, anyone that was linking against MixedLang::all_libs
or just the 'mixedlang' LIB (from internal or IMPORTED target) was getting the
include directories being specified in the 'mixedlang_vector' library.

But this implementation fails for TriBITS packages that only have INTERFACE
libraries.  Those are never getting added to the <Package>::all_libs target or
indirectly to targets that do.  This is why the STKEmend package failed
because it has just one HEADERONLY INTERFACE library.

This new added test case ensures that the INTERFACE library does get linked to
the <Package>::all_libs target and will fail if it does not.

The next commit will add the code in TriBITS to fix this test :-)

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Feb 21, 2025
@bartlettroscoe
Add INTERFACE_LIBRARY targets to <Package>::all_libs target (TriBITSP…
fdcb586 
…ub#625)

This fixes the previously added failing test :-)

I also had to add logic to not generate the package-config files when doing
installation testing where you are only building the tests and examples.  This
was generating a strange error about export sets.  You should not be
generating packge-config files in these cases.

Also, this change required the update of some other tests.  Now that
INTERFACE_LIBRARY taragets are getting pulled down, the <SubPkg>::all_libs for
subpackage targets are getting added the <ParentPkg>::all_libs target for
parent packages.  This is an intereting case but it works.

Signed-off-by: Roscoe A. Bartlett <[email protected]>
bartlettroscoe added a commit that referenced this issue Feb 21, 2025
@bartlettroscoe
Merge pull request #626 from bartlettroscoe/625-headeronly-libs
c26021e 
Add support for tribits_add_library( ... HEADERONLY ... )  (#625)
@bartlettroscoe
Copy link
Member Author

FYI: This is needed for:

bartlettroscoe added a commit to trilinos/Trilinos that referenced this issue Feb 24, 2025
@bartlettroscoe
Merge branch 'tribits-snapshot-2025-02-23' into stk-snapshot
036a6bd 
This brings in support for tribits_add_library( ... HEADERONLY ...).  See
TriBITSPub/TriBITS#625.

Signed-off-by: Roscoe A. Bartlett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: core type: enhancement
Projects
TriBITS
Status: In Progress
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bartlettroscoe