tribits/cmake scripts: avoid generating <x>Config.cmake with the same name as upstream

[Neumann-A]

Enhancement

@trilinos/tribits ? (cmake helper scripts as far as I can tell)

First, please refactor the generation of <x>Config.cmake to generate TPL<x>Config.cmake instead (or some other prefix) to avoid collisions with <x>Config.cmake own version. Second, please refactor the usage of external targets to be also TPL:: prefixed to avoid collisions with other external targets. E.g. fmt::fmt from tribits collides with fmt::fmt from fmt itself and CMake rightfully complains about the fact that it find two configs with different targets.

[rppawlo]

@bartlettroscoe

[jhux2]

@Neumann-A Maybe post at https://github.com/TriBITSPub/TriBITS/issues?

[bartlettroscoe]

@Neumann-A, there are larger considerations here.

See the last paragraph in:

• https://tribitspub.github.io/TriBITS/build_ref/index.html#using-packages-from-the-build-tree-in-downstream-cmake-projects

and see:

• https://tribitspub.github.io/TriBITS/maintainers_guide/index.html#tricky-considerations-for-tribits-generated-tplname-config-cmake-files

What specific problem are you having? How can it be reproduced? Let's focus first on the real problem before jumping to an assumed solution.

[Neumann-A]

@Neumann-A Maybe post at https://github.com/TriBITSPub/TriBITS/issues?

Hmm ok. I thought this was the relevant toplevel project.

https://tribitspub.github.io/TriBITS/build_ref/index.html#using-packages-from-the-build-tree-in-downstream-cmake-projects

I am using an installed tree (as port within vcpkg).

https://tribitspub.github.io/TriBITS/maintainers_guide/index.html#tricky-considerations-for-tribits-generated-tplname-config-cmake-files

So the issue happend when I was pulling in external VTK (with external SEACASIoss) in ParaView complaining that the there are basically two different fmtConfig.cmake defining the same target fmt::fmt with a different setup. But from the link and the discussion I can already tell that the scripts are written by somebody without a clue about the problems. Would it have been so difficult to rename the generated configs for external packages to something like <ProjectPrefix><PackageName>Config.cmake to avoid all those discussions at the given link? I mean there is a reason why e.g. Qt6 uses Wrap<PackageName>.cmake for find modules and defines Wrap<Packagename>:: targets to avoid those conflicts. So originally I applied the following patch to the tribits parts in seacas to add TPL prefixes fix_tpl_libs.txt. I doubt it is acceptable for upstream since I myself consider it more a hack than a full solution.
I wanted to switch seacas/zoltan/kokkos now to a single trilinos port for VTK instead of having those three as separate ports but after discovering that this project already fails in discovering Zlib on windows I should probably just drop all of them and deactivate the modules in VTK pulling this in as a dependency.
Not only do the generated cmake Configs for imported targets miss CMAKE_BUILD_TYPE (i.e. IMPORTED_LOCATION_RELEASE|DEBUG) related context but not taking advantage of existing cmake modules or upstream provided config files is a no go.

So sry to tell you, from what I have seen up until now, tribits seems to be trash for handling external dependencies.

How can it be reproduced?

It is probably enough to have

find_package(fmt CONFIG)
find_package(<WhateverPackageInTriliniosPullsInTheGeneratedfmtConfig.cmake>)

And have both fmt and <WhateverPackageInTriliniosPullsInTheGeneratedfmtConfig.cmake (probably SEACAS)> installed into the same prefix. The reason is that:

if (TARGET fmt::all_libs)
  return()
endif()
add_library(fmt::fmt IMPORTED UNKNOWN)
set_target_properties(fmt::fmt PROPERTIES
  IMPORTED_LOCATION "${SOME_PREFIX}/lib/fmt.lib")

add_library(fmt::all_libs INTERFACE IMPORTED)
target_link_libraries(fmt::all_libs
  INTERFACE fmt::fmt
  )
target_include_directories(fmt::all_libs SYSTEM
  INTERFACE "${SOME_PREFIX}/include"
  )

will still try to generate the additional imported target add_library(fmt::fmt IMPORTED UNKNOWN) conflicting with fmt::fmt from fmt itself.

Also seems like I cannot use this project at all since it has two aux.* files which are undeletable on windows without admin rights after extraction.

[bartlettroscoe]

CC: @gsjaardema

@Neumann-A

So the issue happend when I was pulling in external VTK (with external SEACASIoss) in ParaView complaining that the there are basically two different fmtConfig.cmake defining the same target fmt::fmt with a different setup.

Okay, I see the problem at:

https://github.com/sandialabs/seacas/blob/69a1652a8929b189fd83e1239d7135c634cfe358/TPLsList.cmake#L72

and

https://github.com/sandialabs/seacas/blob/69a1652a8929b189fd83e1239d7135c634cfe358/cmake/TPLs/FindTPLfmt.cmake#L57-L60

We had the exact same situation with some CUDA:: targets that was resolved in:

• Change to use find_package(CUDAToolkit) and some other changes (trilinos/Trilinos#10954) TriBITSPub/TriBITS#528

The solution is to change:

• https://github.com/sandialabs/seacas/blob/master/cmake/TPLs/FindTPLfmt.cmake

to call find_package(fmt) as explained in Creating FindTPL<tplName>.cmake using find_package() with IMPORTED targets. Looks like that might be as simple as:

find_package(fmt REQUIRED)
tribits_extpkg_create_imported_all_libs_target_and_config_file(
  fmt
  INNER_FIND_PACKAGE_NAME fmt
  IMPORTED_TARGETS_FOR_ALL_LIBS fmt::fmt )

Can you give that a try?

So sry to tell you, from what I have seen up until now, tribits seems to be trash for handling external dependencies.

A TriBITS project is only as good at finding/resolving external dependencies as the FindTPL<tplName>.cmake files themselves.
TriBITS itself does not force how external packages are found. (The work now is to upgrade the the existing FindTPL<tplName>.cmake files to use modern CMake as per described here. But it has only been very recent that many of these packages are supporting proper modern <Package>Config.cmake files with modern IMPORTED CMake targets. So please be patent as we identify where these upgrades need to occur.)

[Neumann-A]

But it has only been very recent that many of these packages are supporting proper modern Config.cmake files with modern IMPORTED CMake targets. So please be patent as we identify where these upgrades need to occur.)

From what I see that is not a requirement. I can just use find_package(PkgConfig) in conjunction pkg_check_modules(... IMPORTED_TARGET ...) and then just do the same with the imported PkgConfig:: target.

Still wondering why Zlib is not doing that..... Considering it has been in CMake since version 3.1.

The aux.* files however are a blocker. Could those please be renamed.

But thanks I see a simple way now to fix my linking and library search issues with trilinios.

[Neumann-A]

The aux.* files however are a blocker. Could those please be renamed.

Ok the trick is probably to not use a official release and go some later commit

[bartlettroscoe]

The aux.* files however are a blocker. Could those please be renamed.

@Neumann-A, what are aux.* files?

[Neumann-A]

<buildprefix>\packages\muelu\research\caglusa\aux.mtx
Zugriff verweigert (Access denied in engl.)
<buildprefix>\packages\muelu\research\caglusa\aux.xml
Zugriff verweigert

Seems to have been renamed in a more recent commit. aux is a reserved identifier on windows.

[bartlettroscoe]

Still wondering why Zlib is not doing that..... Considering it has been in CMake since version 3.1.

Partially because using modern IMPORTED targets with external packages/TPLs with TriBITS projects was only possible in Trilinos with the merge of:

• TriBITS: Switch to modern cmake targets (TriBITSPub/TriBITS#299) #10614

There is a lot of catching up to do. (The good news is that most TPLs require just two statements.)

[bartlettroscoe]

Seems to have been renamed in a more recent commit. aux is a reserved identifier on windows.

@Neumann-A, trilinos does not have any maintained automated builds for Windows so it is difficult for the project to consistently support Windows (even though the basic CMake build system for Trilinos works fine on Windows has has since the beginning of CMake usage in Trilinos dating back to 2008).

Thanks for making us aware of these issues!

[bartlettroscoe]

FYI: The driver for this seems to be:

• [seacas/vtkm/vtk/paraview] Add/update ports.  microsoft/vcpkg#27928 (comment)

It seems that Microsoft is maintaining a Trilinos package for deployment on Windows and other systems?

Did anyone in Trilinos know about this?

[Neumann-A]

It seems that Microsoft is maintaining a Trilinos package for deployment on Windows and other systems?

I am not Microsoft just a contributor to vcpkg. The only reason I created a trilinos port is to de-vendor seacas from the VTK port.

[bartlettroscoe]

I am not Microsoft just a contributor to vcpkg. The only reason I created a trilinos port is to de-vendor seacas from the VTK port.

I was just commenting on the existence of Trilinos in https://github.com/microsoft/vcpkg (which is under the "microsoft" GitHub organization so one would assume this has something to do with Microsoft).

[bartlettroscoe]

@Neumann-A, FYI: after some more thought on this issue, I created the TriBITS issue:

• TriBITS-generated <tplName>::<libname> IMPORTED targets conflicting with those from related package config and find modules (#299) TriBITSPub/TriBITS#548

and just merged the PR:

• Prefix generated TPL IMPORTED targets with tribits:: (#548, #299) TriBITSPub/TriBITS#549

to TriBITS 'master'.

I will next update the TriBITS snapshot into Trilinos 'develop' and then the TriBITS changes in microsoft/vcpkg#27928 should no longer be needed. And then this GitHub issue should be resolved.

I just want to make clear that there is no need to rename the TriBITS-generated files <tplName>Config.cmake as suggested above. Those files are under a different subdir and are not found by any find_package() command (see Tricky considerations for TriBITS-generated Config.cmake files). If you disagree with this or have seen a case where the wrong <x>Config.cmake file was found by a call to find_package(<x>), please let me know. The only way that could happen is if the directory <installDir>/lib/external_packages got explicitly added to CMAKE_PREFIX_PATH when configuring downstream CMake projects.

[bartlettroscoe]

FYI: The Trilinos PR:

• TriBITS Snapshot 2022-12-12 working toward TriBITSPub/TriBITS#63 #11380

should contain the fix for this.

[Neumann-A]

Those files are under a different subdir and are not found by any find_package() command (see Tricky considerations for TriBITS-generated Config.cmake files). If you disagree with this or have seen a case where the wrong Config.cmake file was found by a call to find_package(), please let me know.

I find it extremely fishy to have, e.g. two fmt-configs.cmake even though it is just included. It itself is setting fmt_DIR and doing a find_dependency(fmt) call which basically forces all subsequent find_dependency(fmt) calls to be resolved to the same config. Maybe <packagename>_DIR needs to be unset afterwards.

A second collision i see: if there are two tribits project individually build with the same external dependency (but differently resolved) installed into the same install prefix. In this case the generated and installed -config.cmake might be different and one project would override the other. As such it is not only enough to have a -config.cmake named different from upstream but you actually need a per project unique -config.cmake or a per project unique path to that -config.cmake. That is the reason i prefixed the generated configs with TPL-Seacas-${TPL_NAME}/TPL-Seacas-${TPL_NAME} instead of just ${TPL_NAME}/${TPL_NAME}.

[bartlettroscoe]

I find it extremely fishy to have, e.g. two fmt-configs.cmake even though it is just included. It itself is setting fmt_DIR and doing a find_dependency(fmt) call which basically forces all subsequent find_dependency(fmt) calls to be resolved to the same config.

@Neumann-A, that is not entirely true. Even if fmt_DIR has been set, if the requirements provided in a find_package(fmt [other args]) call are not satisfied by the fmtConfig.cmake file give by the location in fmt_DIR, then will be ignored and searching will continue from scratch. That does not seem to be documented but that is the reported behavior by Kitware (see CMake issue #23685).

A second collision i see: if there are two tribits project individually build with the same external dependency (but differently resolved) installed into the same install prefix.

I can see how that might happen. But if you are creating any combined (static) executables, then that will not give a valid build anyway. For example, you can't be linking a single executable combining libraries that point to two different implementations of BLAS. In that case, you would like to perhaps detect that such a mistake has happened and error out. (And that is something that we can likely catch by putting in checks if we are worried about that.)

But one might resolve a case like this in general by putting in relative include paths to the TriBITS-generated <installDir>/external_pacakges/<tplName>/<tplName>Config.cmake file in cases where we know that a TriBITS package is using a specific TriBITS TPL. That would allow using multiple different installations of the same external package/TPL in the same downstream CMake project. (And that might be valid if not creating combined executables or if using only shared libs with proper encapsulation.)

That is the reason i prefixed the generated configs with TPL-Seacas-${TPL_NAME}/TPL-Seacas-${TPL_NAME} instead of just ${TPL_NAME}/${TPL_NAME}.

But note that a TriBITS generated <tplName>Config.cmake file is a valid implementation for a call to find_package(<tplName> CONFIG ...). And while you would not want to find that <tplName>Config.cmake file by default, someone may choose to use that for a downstream CMake project calling find_package(<tplName>) by setting <tplName>_ROOT to that directory <installDir>/external_packages/<tplName>.

All of this may need some more tweaking as we work out all of these use cases. Most of the issues described in this GitHub issue are implementation details that can and not break backward compatibility. Therefore, there is opportunity to perform these tweaks as we learn more.

What we are building with updated TriBITS in a general purpose way to glue together CMake packages in various states of modern CMake compliance and giving us the flexibility to build CMake-based packages in different aggregate CMake projects as best suits our current needs. That is the best case scenario for development and deployment processes and systems. (And we can do this with just some CMake macros and not requiring a heavy weight package management system.)

[bartlettroscoe]

Maybe <packagename>_DIR needs to be unset afterwards.

@Neumann-A, actually, the TriBITS generated <tplName>Config.cmake files already unset <upstreamTpl>_DIR after calling find_dependency(<upstreamTpl>). See:

https://github.com/TriBITSPub/TriBITS/blob/64309c90aca04996cf483d633baf469585284bf4/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake#L356

and

https://github.com/TriBITSPub/TriBITS/blob/64309c90aca04996cf483d633baf469585284bf4/test/core/TribitsExternalPackageWriteConfigFile_UnitTests.cmake#L1157

Again, much of this can be tweaked without breaking backward compatibility as we learn more.

[bartlettroscoe]

FYI: PR #11380 was just merged so the core problem raised in this issue should be addressed.

@Neumann-A, using this version of Trilinos 'develop' should avoid the heavy modifications to TriBITS in the microsoft/vcpkg#27928 (comment).

I will put this issue in review.

[bartlettroscoe]

@Neumann-A,

FYI: PR #11458 that brings back this fixed was just merged. (I had to back it out over the holiday break due to a few issues.)

[github-actions]

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

[github-actions]

This issue was closed due to inactivity for 395 days.