Update dependency uxlfoundation/oneTBB#3606
Update dependency uxlfoundation/oneTBB#3606Alexandr-Solovev wants to merge 8 commits intouxlfoundation:mainfrom
Conversation
Alexandr-Solovev
force-pushed
the
dev/asolovev_tbb_upd
branch
from
cf050a5 to
f1c4790
Compare
Alexandr-Solovev
added
the
dependencies
Alexandr-Solovev
requested review from
ahuber21,
avolkov-intel,
david-cortes-intel,
ethanglaser,
homksei,
icfaust and
napetrov
as code owners
There was a problem hiding this comment.
Pull request overview
This PR updates oneTBB integration across the repository’s build tooling (Make, Bazel, CI) and removes legacy handling for the old oneTBB directory layout, aiming to standardize on the newer lib/-based structure.
Changes:
- Removed old TBB layout detection/mapping logic from Make and Bazel configuration.
- Bumped oneTBB version references (scripts and CI) to
v2022.3.0. - Adjusted TBB download/unpack behavior to flatten older layouts into the new layout when needed.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/daal/cpp/mpi/makefile_lnx | Drops legacy lib/intel64/gcc4.8 handling and uses $(TBBROOT)/lib. |
| makefile | Removes OLD_TBB_LAYOUT branching and simplifies TBB path selection logic. |
| dev/download_tbb.sh | Updates oneTBB version and adds a compatibility flatten step for older extracted layouts. |
| dev/bazel/repos.bzl | Removes hardcoded local-root mapping selection logic for older package keywords. |
| dev/bazel/deps/tbb.bzl | Removes TBB local_mapping that supported older layouts. |
| .ci/pipeline/ci.yml | Updates CI variable TBB_VERSION to v2022.3.0. |
| .ci/env/tbb.sh | Adds an apt --fix-broken install step during environment setup. |
Comments suppressed due to low confidence (1)
dev/bazel/repos.bzl:158
- With the current logic, when
root_env_varis set,mappingis always{}andrepo_ctx.attr._local_mappingis never consulted. Sincelocal_mappingis still part ofprebuilt_libs_repo_rule()and_local_mappingis still declared as an attr, this is now dead configuration and can confuse future dependency updates. Either wiremapping = repo_ctx.attr._local_mappingback in for local roots (if still needed), or remove thelocal_mappingparameter/attr entirely to match the new behavior.
def _prebuilt_libs_repo_impl(repo_ctx):
root = repo_ctx.os.environ.get(repo_ctx.attr.root_env_var)
if root:
mapping = {}
else:
if repo_ctx.attr.url or repo_ctx.attr.urls:
root = _download(repo_ctx)
mapping = repo_ctx.attr._download_mapping
| RELEASEDIR.tbb.libia := $(RELEASEDIR.tbb)/lib$(if $(OS_is_mac),,/$(_IA)$(if $(OS_is_win),/vc_mt,/$(TBBDIR.libia.lnx.gcc))) | ||
| RELEASEDIR.tbb.soia := $(if $(OS_is_win),$(RELEASEDIR.tbb)/redist/$(_IA)/vc_mt,$(RELEASEDIR.tbb.libia)) | ||
| endif | ||
| RELEASEDIR.tbb.libia := $(RELEASEDIR.tbb)/lib$(if $(OS_is_mac),,$(if $(OS_is_win),/vc_mt,/$(TBBDIR.libia.lnx.gcc))) |
| elif "2021.2-gold_236" in root: | ||
| mapping = repo_ctx.attr._local_mapping | ||
| else: | ||
| mapping = {} |
| sudo apt-get update | ||
| sudo apt --fix-broken install -y | ||
| sudo apt-get install build-essential gcc gfortran cmake -y |
| tbb_prefix=${tbb_prefix:-${ONEDAL_DIR}/__deps/tbb-$target_arch} | ||
|
|
||
| sudo apt-get update | ||
| sudo apt --fix-broken install -y |
There was a problem hiding this comment.
@Alexandr-Solovev What kind of error does this aim to solve? Do you have a log?
There was a problem hiding this comment.
In building tbb step:
Reading package lists...
Reading package lists...
Building dependency tree...
Reading state information...
build-essential is already the newest version (12.9ubuntu3).
build-essential set to manually installed.
gcc is already the newest version (4:11.2.0-1ubuntu1).
gfortran is already the newest version (4:11.2.0-1ubuntu1).
You might want to run 'apt --fix-broken install' to correct these.
The following packages have unmet dependencies:
cmake : Depends: libjsoncpp25 (>= 1.9.5) but it is not going to be installed
Depends: librhash0 (>= 1.2.6) but it is not going to be installed
Depends: cmake-data (= 3.22.1-1ubuntu1.22.04.2) but it is not going to be installed
qemu-user-binfmt : Breaks: qemu-user-static
E: Unmet dependencies. Try 'apt --fix-broken install' with no packages (or specify a solution).
There was a problem hiding this comment.
I guess the problem might be caused by this:
https://github.com/Alexandr-Solovev/oneDAL/blob/5266602d376f49f80fc1856b2dba1634c93295e1/.ci/env/apt.sh#L83
It shouldn't be manually downloading .deb files from the debian main repository and then installing them with dpkg, since it is running ubuntu.
The ubuntu equivalent would be the qemu-user package.
@Alexandr-Solovev Could you try installing the correct ubuntu package and seeing if that eliminates the need for such workarounds?
There was a problem hiding this comment.
Note that it might require updating to the latest ubuntu. The CI uses 22.04, which probably doesn't have that package.
There was a problem hiding this comment.
My guess, that the reason, that previously we skipped this step for a long time, due to
TBB_CACHE_DIR : $(Pipeline.Workspace)/tbb-riscv64-clang was not empty.
In the main
But I will take a look on your guess today
Alexandr-Solovev
force-pushed
the
dev/asolovev_tbb_upd
branch
from
1b5a088 to
638f220
Compare
Description
Summary
This PR updates the TBB integration and cleans up outdated configuration across the build system.
Changes
1. Removed legacy TBB layout variables
2. Updated TBB to the latest version
3. Updated Bazel mapping
Notes
.sofiles), which is now handled in the build systemChecklist:
Completeness and readability
Testing
Performance