Fix tbb config

[hjmjohnson]

No description provided.

[hjmjohnson]

@tbbdev These changes are listed as a temporary homeberew patch at:
Homebrew/homebrew-core#36175

This should also resolve the problem posted here: oneapi-src/oneDPL#16

[AlexVeprev]

According to this change _tbb_root is set during the TBBConfig generation stage, but the previous implementation sets _tbb_root dynamically.
Is the generation of TBBConfig intended to be done on user system with absolute path in @TBB_ROOT@ as a part of TBB package installation? Or the generation is supposed to be done on a system where the package is prepared?

Anyway it doesn't look suitable for TBBConfig files in official TBB packages (which are portable, actually).

As far as I understand a relative path in @TBB_ROOT@ won't work without CMAKE_CURRENT_LIST_FILE as CMake will try to find TBB files relatively to user project location in this case.

[hjmjohnson]

@AlexVeprev I see your point.

The current code is also wrong when -DSAVE_TO=lib/cmake/TBB (i.e. placing the files in the conventional relationship to the libraries as is done by the homebrew implementation.

It should be possible to configure the file with something like

"cmake", "-DTBB_ROOT=#{prefix}",
                    "-DTBB_OS=Darwin",
                    "-DSAVE_TO=/some/very/long/path/reference//with/arbitrary/levels/lib/cmake/TBB",
                    "-P", "cmake/tbb_config_generator.cmake"

https://github.com/Homebrew/homebrew-core/blob/master/Formula/tbb.rb

"cmake", "-DTBB_ROOT=#{prefix}",
                    "-DTBB_OS=Darwin",
                    "-DSAVE_TO=lib/cmake/TBB",
                    "-P", "cmake/tbb_config_generator.cmake"

=======
I am willing to help get the code correct, but it is quite difficult to determine the intended behaviors of tbb installation. tbb is one of the most difficult packages I have every work with in regards to incorporating it's deployment. (The library is great once installed, but integrating into existing builds for purposes like travis continous integration on github are extreemly unconventional).

If you can provide some guidance on the intended workflow for 1) Downloading, 2) building, 3) installing, and then 4) building our applications against TBB I would be eternally grateful.

Thanks,
Hans

[AlexVeprev]

Hi @hjmjohnson
 
Speaking about GitHub TBB distribution: you can build TBB from sources or download packages with prebuilt binaries.
There are no special installation steps, but to use TBB you can:

• Run tbbvars.sh/.csh/.bat to set environment variables (TBBROOT, LIBRARY_PATH, etc.) or
• Use find_package from CMake with TBB_DIR=<tbb_root>/cmake as described here.

TBBMakeConfig.cmake was originally implemented to generate TBBConfig files for TBB packages distributed on GitHub. That's why it looks "buggy" when you use it in Homebrew. But now it is seen that TBBConfig generation is suitable for other TBB packages. I think it makes sense to work in this direction.
I didn't investigate this deep enough and cannot say what is better: to "fix" existing TBBConfig generator or to create a new one applicable for TBB GitHub/Homebrew/APT/YUM/Conda/etc. packages. I'd really appreciate if you share your ideas about it.

Thanks,
Alexey

[hjmjohnson]

@AlexVeprev,

You instructions are OK for building single binary as a developer that wants to understand TBB. I need to install TBB in a way that allows integration with a hierarchy of other libraries (ITK, VTK, BRAINSTools, Slicer) across compilers/OS/ and development platforms (Not all unix or command line driven).

The time tested and validated paradigm to achieve this is supported by meson, cmake, autoconf tools:

-- Configure specifying 
        -- the compiler/IDE development environment: (CC, CXX, CFLAGS ....)
        -- the desired install prefix (i.e. --prefix=/usr/myfavoriteplace) will result in a standard directory layout rooted at '/usr/myfavoriteplace

-- build (ninja/make/nmake) 
-- install

The result should be that with 3 commands a redistributable set of binary files suitable for the current platform can be created:

${installprefix}/inlcude/tbb/<<< *.h >>>                                         #TBB header files
${installprefix}/lib/<<< libTBBB***[_debug].[.so|dylib|a]>>>>>  #TBB binary files
${installprefix}lib/cmake/TBB/***.cmake                                        #TBB cmake config files referencing this ${installprefix}

=======
NOTE: : Feel free to ignore. It seems that the build system for TBB is optimized for the convenience of the TBB library developers, and not the end users of TBB (or at least other developers that want to depend on TBB libraries). The build environment for TBB seems to be a one-off makefile hand crafted script. The most disturbing concern is that package developers (home-brew/ RPM/ debian ... etc) each have different implementations and wrapper scripts around the TBB build environment to coerce the creation of binaries from TBB into a standard install that is compatible with their distribution.
<\end_critique> :)

========

I've been fighting with TBB on and off for 6 months now. I can always build TBB by hand on my computers. Then I need to spend hours hand building and resolving linkage issues for each of the many packages that use TBB for our application. I need a scriptable install that works in continuous integration environment builds without manual interventions. I need a consistent pattern for finding system installed versions of TBB.

My current problems are 2 fold 1) The current homebrew install of TBB fails because the pathing is wrong in the TBBMakeConfig.cmake that is used by the home-brew package maintainers (and they are hesitant to make a change that is not in TBB upstream.) 2) The recommended way to include TBB using cmake tools depends on access to api.github.com in order to get version information and package tarball file information, but api.github.com is blocked on continuous integration environment (Travis) for security reasons presumably.

=======
I'm working on a more complete fix, but I don't want to expend a lot of time making a solution that fits my vision only to find that it will not be accepted.

[AlexVeprev]

@hjmjohnson, thank you for the detailed explanation!

The paradigm configure/build/install allows to create the library/application from sources and install it, but from my perspective the more common use case for TBB is use of pre-built binaries. If I'm right then a TBB user is interested in installing TBB via his favorite package manager / installer and integration of the installed TBB into his project, e.g. using TBBConfig.

On the other hand for package maintainers the configure/build/install steps are truly applicable. And actually configure/build steps are covered by Makefile scripts in TBB (maybe not so conveniently as it is needed for package maintainers).
If we consider the existing build infrastructure: can it be improved with small investments, but with high value for package maintainers or users who wants to build TBB from source and install it?
E.g. add install target into Makefile which will copy headers/binaries to ${installprefix} and create ${installprefix}/lib/cmake/TBB/TBBConfig[Version].cmake. Or just another target creating TBBConfig[Version].cmake in the requested location.

A proper CMake support allows to simply implement the configure/build/install paradigm, but existing Makefile infrastructure in TBB is quite big and complex, so smooth and full migration to CMake will require a lot of resources.

[anton-malakhov]

@hjmjohnson Please note the build.py script which is intended for package maintainers usage. So far, it is used by Conda-Forge, Anaconda, and Intel itself in order to assemble tbb, tbb-devel, and tbb4py packages. BTW, conda's package management is good for any language, Conda-Forge provides a lot of C/C++ libraries as openly-compiled packages and it is pretty easy to add new recipes.

[hjmjohnson]

@anton-malakhov thanks but that is not a viable option. I can not add python or conda as a dependency for my applications.

[hjmjohnson]

@AlexVeprev I have pushed a set of changes that add a script to accomplish a standard install of TBB. I'd appreciate if you could review these changes.

[AlexVeprev]

@hjmjohnson, thank you! I've started the review. I'd like to understand the changes deeply enough and try them practically. So, it will take some time.

Thanks,
Alexey

[AlexVeprev]

Hi @hjmjohnson, I've partly reviewed your changes (TBBMakeConfig).

Seems you completely changed behavior in case of CONFIG_FOR_SOURCE is false and you moved all legacy code under a condition "CONFIG_FOR_SOURCE is true".

But currently TBBMakeConfig can do the following:

1. CONFIG_FOR_SOURCE is true: generate TBBConfig files from TBBBuild.cmake:189
2. CONFIG_FOR_SOURCE is false: generate TBBConfig files for TBB release packages using tbb_config_generator.cmake.

I didn't verified the case 1 with your updates, but I was not able to generate a proper config for TBB release packages.
I understand that you probably don't have much interest in this functionality, but anyway it is used now and the problem needs somehow to be addressed before the PR can be merged.

Let's try to clarify your requirements for TBBConfig generator.
Please correct me if I'm wrong:

1. Input:
   • relative path from TBBConfig.cmake to TBB binaries (release/debug?)
   • relative path from TBBConfig.cmake to TBB headers
   • TBB version (+interface version)
2. Output:
   • TBBConfig.cmake
   • TBBConfigVersion.cmake
3. Only one set of TBB binaries is supposed to be included into the package.
4. *Binaries/headers presence in a system is not required during the generation.

[AlexVeprev]

Why did you extend the list of GCC versions? This loop is needed to correctly choose compiler subdirectory in a TBB package for Linux, the latest package has only three subdirectories: gcc4.1, gcc4.4, gcc4.7.

[hjmjohnson]

When building with gcc 7.3.0 this code caused failure for the built version I installed. It means that this code only works for the binaries in the build configuration that are produced and delivered in binary form, not the source code builds.

This code is really concerning to me. Source code builds should not be tied to lists like this.

[AlexVeprev]

TBB_BUILD_DIR is not quite clear to me, what did you mean? A directory like /usr? If yes, I'd use more clear name, e.g. INSTALL_DIR

[hjmjohnson]

TBB_BUILD_DIR is a temporary build directory where binaries are built before being installed.

In standard cmake paradigm

git clone <URL>/mypkg.git   # This is the source tree
mkdir mypkg-bld   # <- this is the out of source location for build artifacts (including .o files)
cd mypkg-bld &&  cmake ../mypkg -DCMAKE_INSTALL_PREFIX:PATH=/usr/local
make                      # This is building all the build products (.o intermeidiate files & lib*.a files)
make install           # This moves files from source tree and build tree to desired install tree

OR IN autoconf tools

git clone <URL>/mypkg.git   # This is the source tree
mkdir mypkg-bld 
cd mypkg-bld &&  ../mypkg/configure --prefix=/usr/local
make                      # This is building all the build products (.o intermeidiate files & lib*.a files)
make install           # This moves files from source tree and build tree to desired install tree

[AlexVeprev]

Is this option supposed to be controlled by user? If no, it is not clear for me how @TBB_CHOOSE_COMPILER_SUBDIR@ is used when TBB_CONFIG_FOR_SOURCE is ON.

[hjmjohnson]

This was an attempt to maintain backward compatibility with what was there. I was inferring how the tool might have been used. I needed different behavior from the default to support building from source.

[AlexVeprev]

This FATAL_ERROR can appear only if TBB_FIND_REQUIRED AND TBB_FIND_REQUIRED_${_tbb_component}. I.e. only if the requested component is required.

With your change you also will get FATAL_ERROR in case of find_package(TBB QUIET REQUIRED):

CMake Error at CMakeLists.txt:7 (find_package):
  Found package configuration file:

    /tmp/tbb2019_20181203oss/cmake/TBBConfig.cmake

  but it set TBB_FOUND to FALSE so package "TBB" is considered to be NOT
  FOUND.

Could you please explain a use case when this change has effect?

[hjmjohnson]

YES!

I want to attempt to use a custom FindTBB.cmake that exists in my project in order to overcome problems with broken os installed versions of TBB.

Use case "Use system version of TBB if it can be found and configured correctly, else (disable tbb capabilites|build tbb from source|use custom FindTBB.cmake that does more brute force introspection of the system with different rules).

The current homebrew installed version of TBB uses "TBBMakeConfig.cmake" to generate a TBBConfig.cmake file with incorrect hard-coded paths to the wrong directories for finding the libraries. If TBB is installed on the system, there is no recovery in any way.

I'm trying to get a TBB dependant tool to build and be tested on the Travis continuous integration environment. Installing TBB from homebrew causes build failure for the reasons above. Trying to install TBB from source fails as well because the instructions for building from source for a cmake project require access to api.github.com (which is prohibited from CI environments).

[AlexVeprev]

Ok, thank you for explanation, I think this change (removal of fatal error) is ready to be integrated.

[hjmjohnson]

Let's try to clarify your requirements for TBBConfig generator.
Please correct me if I'm wrong:

1. Input:
   • relative path from TBBConfig.cmake to TBB binaries (release/debug?)
   • relative path from TBBConfig.cmake to TBB headers
   • TBB version (+interface version)
2. Output:
   • TBBConfig.cmake
   • TBBConfigVersion.cmake
3. Only one set of TBB binaries is supposed to be included into the package.
4. Binaries/headers presence in a system is not required during the generation.

Yes.

Regarding point 3 above, many packages can be created, but each package contains only the materials for 1 binary support.

basedir=/usr/local # or /usr, or /opt or /myapplications or ${HOME}/.linuxbrew
===pkg 2019_U3_2
${basedir}/Cellar/tbb/2019_U3_2/lib/cmake/TBB
${basedir}/Cellar/tbb/2019_U3_2/inlcude
===pkg 2019_U3_1
${basedir}/Cellar/tbb/2019_U3_1/lib/cmake/TBB
${basedir}/Cellar/tbb/2019_U3_1/inlcude
===pkg 2020_U3_1
${basedir}/Cellar/tbb/2020_U3_1/lib/cmake/TBB
${basedir}/Cellar/tbb/2020_U3_1/inlcude

[AlexVeprev]

Officially TBB doesn't support static libraries. There is a special case for Big Iron platform. So if you'd like to cover this case it would be better to add Big Iron as an option for SYSTEM_NAME. In this case TBB_BUILD_STATIC and TBB_BUILD_SHARED flags are not needed at all.

[akukanov]

Just for information, "Big Iron" is not really a platform name; it's a common name for mainframe computers and similar HPC systems. Often dynamic (shared) libraries are avoided or prohibited on such systems.

It is true that building and using TBB as a static library is highly discouraged, as it opens the door to have multiple copies of TBB in the same process, with potential negative impact on performance or even correctness of the program.

[AlexVeprev]

Is it a real use case to install x64/x86 binaries into one CMAKE_INSTALL_PREFIX but in different subdirectories?

[hjmjohnson]

@AlexVeprev I don't understand the question in the context pointed to. Sorry.

[hjmjohnson]

Could I get a status update on the progress of fixing the tbb config? This is holding up the incorporation of TBB in several projects.

[AlexVeprev]

@hjmjohnson your PR is quite big and some parts need to be considered more deeply, for example cmake/TBBBuildInstallScript.cmake has the following concerns:

• static TBB linkage;
• possible conflicts with TBB installed in scope of Intel products.

The installation script is a reasonable thing, but it may delay acceptance of the PR.

The fix in TBBMakeConfig is more critical problem than installation script, isn't it?
But it also has problems: it doesn't work for making configs for TBB releases on GitHub.

I can propose the following: I'll update TBBMakeConfig according to the requirements, but in compliant with existing TBB infrastructure. Do you agree?

[hjmjohnson]

Sure. I've already spent several days effort trying to get TBB to build in a way that is consistent with other build and package managers.

My critique still holds: TBB's support of external communities that need "standard" procedures for building, installing, and deploying TBB as part of other packages is extremely fragile. Adoption of TBB in major medical imaging software packages has been stymied for several years now because each time we demonstrate the greatness of the the TBB library, we end up abandoning the efforts because we can not reliably produce an automated cross platform build for inclusion.

1. We need a buildable version of TBB that is compatible with continuous integration (For example, we can not build TBB on travis CI because we do not have access to the api.github.com web site that is required for using the current macros).

2. The CMake support is current broken for installing TBB, or making a valid package (NOTE: mac home-brew install of TBB is severely broken with regards to cmake package management).

========

TBB should follow conventions of building like: fftw, jsoncpp, ITK, dcmtk,

========

I don't care if any of my code is used, I just want TBB build, install, and inclusion in 3rd party tools following the standard install process that is consistent with autoconf, cmake, meson, bazel, etc. build manager systems. It should be as easy as:

PKG_URL:  GitHub url
PKG_SRC: Where source code was downloaded
PKG_BLD: The binary build directory where object, and targets of the build should go
PKG_INSTALL_DIR:  Where to install the binary objects suitable for distribution
PKG_NAME:  A package name

git clone ${PKG_URL};
mkdir ${PKG_BLD};
cd ${PKG_BLD};
cmake -DCMAKE_INSTALL_PREFIX:PATH=${PKG_INSTALL_DIR}  ${PKG_SRC}
make
make test
make install

OR

git clone ${PKG_URL};
mkdir ${PKG_BLD};
cd ${PKG_BLD};
${PKG_SRC}/configure --prefix=${PKG_INSTALL_DIR}  
make
make test
make install

The result should be

${PKG_INSTALL_DIR}/include  # <- The header files for the tool
${PKG_INSTALL_DIR}/include/${PKG_NAME}   # <- (OPTIONAL) The header files for the tool

${PKG_INSTALL_DIR}/lib         # <-- Where library objects go
${PKG_INSTALL_DIR}/lib/cmake/${PKG_NAME}/  # <- ${PKG_NAME}Config.cmake ${PKG_NAME}ConfigVersion.cmake
${PKG_INSTALL_DIR}/share/info     # <-- where command cmake files go

Perhaps https://www.gnu.org/prep/standards/html_node/DESTDIR.html can help with much of this effort of maintaining makefiles as the primary build environment.

[anton-malakhov]

I've already spent several days effort trying to get TBB to build in a way that is consistent with other build and package managers.

@hjmjohnson I guess running build.py is still not an option, is it? Travis, Circle CI, Appveyor all have python or even conda pre-installed. Even if it is not available, installing miniconda is a question of 65Mb download size and running a simple script. So, if the build works well for TBB on Conda-Forge (which is not python-specific package manager), why it doesn't work for your package managers?

[hjmjohnson]

@anton-malakhov I hope I can avoid the python dependency for building, but perhaps that is a mandatory requirement.

I did try

python build.py  --tbbroot ~/src/tbb --prefix /tmp/tbb_install --install-libs --install-devel --install-docs

but it does not create the cmake package files.

Hans

[anton-malakhov]

it might not be a requirement if TBB team is willing to accept an alternative implementation, which covers all the functionality of what build.py is doing now. I guess nobody wants to support two implementations of the very same install functionality. build.py was designed to support more package managers than just conda, e.g. I looked up for what APT and homebrew are doing so their recipes can be rewritten to use build.py. If anything missing, like cmake files, it can certainly be added - the script is simple and you are welcome to modify it (or reimplement using another cross-platform scripting language).

[hjmjohnson]

Abandoning. This effort was not supported, and other work around solutions seem to be more popular. Using a python wrapping around make, layered with cmake fix-ups seems to be the desired approach.

See #127 for the alternative approach.