tbb: Split into tbb_2020_3 and tbb_2021_8 #217585

davidak · 2023-02-21T23:05:33Z

Description of changes

This is based on PR #214762.

For the new release 2021.8, see
https://www.intel.com/content/www/us/en/developer/articles/release-notes/intel-oneapi-threading-building-blocks-release-notes.html
https://github.com/oneapi-src/oneTBB/releases/tag/v2021.5.0
https://github.com/oneapi-src/oneTBB/releases/tag/v2021.6.0
https://github.com/oneapi-src/oneTBB/releases/tag/v2021.7.0

Due to the significant breakage due to the update to TBB 2021.8, instead split the tbb package into tbb_2020_3 and tbb_2021_8, with the default tbb aliased to tbb_2020_3 in order to minimize breakage.

Also fixed build issues and improved code.

The reason I went for this change is that I'd like to package CCTag which depends on TBB 2021. To me this solution seems like a good way to bridge the time until the packages incompatible with TBB 2021.8, but I'm not sure if there are any guidelines regarding splitting packages like this. So please comment if this discouraged for some reason.

I hope I got the split right. In particular, I'm wondering if there's a better solution for sharing meta. Also, I'm open to suggestions regarding the versioned package naming scheme.

Continuing #215689 and #216991

cc co-author @hesiod

Things done

davidak · 2023-02-22T00:10:58Z

Result of nixpkgs-review pr 217585 run on x86_64-linux 1

2 packages built:

tbb_2020_3
tbb_2021_8

Content looks good:

[nix-shell:~/.cache/nixpkgs-review/pr-217585-2]$ tree results/*
results/tbb_2020_3
└── lib
    ├── libtbbmalloc_proxy.so
    ├── libtbbmalloc_proxy.so.2
    ├── libtbbmalloc.so
    ├── libtbbmalloc.so.2
    ├── libtbb.so
    └── libtbb.so.2
results/tbb_2021_8
├── lib
│   ├── libtbbmalloc_proxy.so -> libtbbmalloc_proxy.so.2
│   ├── libtbbmalloc_proxy.so.2 -> libtbbmalloc_proxy.so.2.8
│   ├── libtbbmalloc_proxy.so.2.8
│   ├── libtbbmalloc.so -> libtbbmalloc.so.2
│   ├── libtbbmalloc.so.2 -> libtbbmalloc.so.2.8
│   ├── libtbbmalloc.so.2.8
│   ├── libtbb.so -> libtbb.so.12
│   ├── libtbb.so.12 -> libtbb.so.12.8
│   └── libtbb.so.12.8
└── share
    └── doc
        └── TBB
            └── README.md

4 directories, 10 files

davidak · 2023-02-22T00:12:09Z

Please review and test.

I will build for x86_64-linux, i686-linux and aarch64-linux.

pkgs/top-level/all-packages.nix

pkgs/development/libraries/tbb/common.nix

pkgs/development/libraries/tbb/2021_8.nix

pkgs/development/libraries/tbb/default.nix

davidak · 2023-02-24T02:29:42Z

Result of nixpkgs-review pr 217585 run on x86_64-linux 1

1 package failed to build:

openimagedenoise_1_2_x (was broken before https://github.com/openimagedenoise_1_2_x build fails #216870)

22 packages built:

blender
bowtie2
cloudcompare
easyeffects
embree
embree2
ikos
mirtk
opencascade
openimagedenoise
openturns
openvdb
openvino
osrm-backend
pagmo2
paraview
potreeconverter
prusa-slicer
sysdig
tiledb
tracy
vcmi

Built packages where tbb is a direct dependency.

I have run one binary if the package contain any. No crash except easyeffects, but that is expected (#217972).

davidak · 2023-02-24T07:09:51Z

Somehow the build on i686-linux is blocked by #217976 even tho the package is not requested to be built directly.

davidak · 2023-03-01T04:47:10Z

Build of both packages work now on i686-linux.

Thanks to @trofi for the great feedback!

pkgs/development/libraries/tbb/default.nix

trofi

Looks very good! I added a few more minor comments.

pkgs/development/libraries/tbb/2020_3.nix

trofi · 2023-03-01T06:40:47Z

pkgs/development/libraries/tbb/default.nix

+    # Fix/suppress warnings on gcc12.1 from https://github.com/oneapi-src/oneTBB/pull/866
+    (fetchpatch {
+      url = "https://patch-diff.githubusercontent.com/raw/oneapi-src/oneTBB/pull/866.patch";
+      hash = "sha256-zT+tRPnPVGUqKvob0RWjASRm4bPYYLtaaqTzA4EdU/8=";


Could it be that you used hash from the wrong version of the patch? If failed to fetch for me. I had to change the hash locally as:

--- a/pkgs/development/libraries/tbb/default.nix +++ b/pkgs/development/libraries/tbb/default.nix @@ -32,7 +32,7 @@ stdenv.mkDerivation rec { # Fix/suppress warnings on gcc12.1 from https://github.com/oneapi-src/oneTBB/pull/866 (fetchpatch { url = "https://patch-diff.githubusercontent.com/raw/oneapi-src/oneTBB/pull/866.patch"; - hash = "sha256-zT+tRPnPVGUqKvob0RWjASRm4bPYYLtaaqTzA4EdU/8="; + hash = "sha256-e44Yv84Hfl5xoxWWTnLJLSGeNA1RBbah4/L43gPLS+c="; }) ];

If you still have previous patch locally try to compare what changed if it's an expected difference.

I also get that hash now.

I guess the only change i made is to use hash = instead of sha256 =, but that shouldn't make a difference?

Yeah, hash = vs sha256 = does not make a difference. It's something else: maybe unstable patch generation algorigthm on github's side that fetchpatch can't clean up? or maybe the hash was calculated against fetchurl before it was converted to fetchpatch?

Yes, fetchurl was also used before. Maybe that made the difference. From looking at the GitHub PR, there should not be a change.

pkgs/development/libraries/tbb/default.nix

trofi · 2023-03-01T06:49:55Z

pkgs/top-level/all-packages.nix

-  tbb = callPackage ../development/libraries/tbb { };
+  tbb_2020_3 = callPackage ../development/libraries/tbb/2020_3.nix { };
+  tbb_2021_8 = callPackage ../development/libraries/tbb { };
+  # many packages fail with latest version, so they need to be updated one-by-one
+  tbb = tbb_2020_3;


It's a reasonable change. I have only one complain on the one-by-one bit: updating users one-by-one is not really an option for library users due to diamond dependency problems: https://trofi.github.io/posts/248-how-do-shared-library-collisions-break.html. We will likely need to update most of users in a single go later. Otherwise we will have to debug non-trivial linkage failures or even plain SIGSEGVs.

About 100 packages build and 100 fail (#214762 (comment)). So i thought we could change the working 100 to the new version and leave the other on the old.

You say that will cause unexpected issues? I did not have time to read the post.

So far i understand that when we have 2 libraries, that use the two versions of tbb and one package that use these two libraries, we will have issues. So i would try to update as much as possible and test the program. I guess in case of an issue, they will not even start?

How does this contradict the feature of Nix to be able to have multiple versions of libraries on the same system? Why is it different here than with other dependencies? I guess it would also not be possible to have multiple glibc? I hope such limitations of Nix are documented somewhere.

The whole point of this change (for me) is to be able to use the new version for the CCTag library, that is needed for Meshroom.

What change do you suggest here?

So far i understand that when we have 2 libraries, that use the two versions of tbb and one package that use these two libraries, we will have issues. So i would try to update as much as possible and test the program. I guess in case of an issue, they will not even start?

Yeah, if a single address space loads multiple library versions there will likely be troubles. It's hard to say how those will manifest. At least you will get an early crash. At worst it will not crash at all and will corrupt your data. The typical problem is when data structure is constructed in one library version and processed in another. I usually use lddtree -a on a binary to see if there are multiple versions of a library loaded together.

How does this contradict the feature of Nix to be able to have multiple versions of libraries on the same system? Why is it different here than with other dependencies? I guess it would also not be possible to have multiple glibc? I hope such limitations of Nix are documented somewhere.

You can have multiple versions of glibc as long as you guarantee that a single address space (a single process) does not load two versions of glibc at the same time. nixpkgs tries very hard to do it by defining a single glibc package used everywhere (except the bootstrap). That way as long as you don't mix glibc from different nixpkgs instances in a single derivation it should be mostly OK (mostly because there are some exceptions like /run/opengl-driver that expose a particular glibc version via shared plugin, https://github.com/nixpkgs-architecture/issues/issues/15).

Mixing and matching final binaries from different nixpkgs version normally is safer as each of them usually uses their consistent (either disjoint or identical) closure of shared libraries and configs. There are still creative ways of breaking them like incompatible configs in /etc/ or through other non-hermetic paths like plugins or incompatible database formats in `/var/.

The libraries are just a lot simpler to break if a single nixpkgs provides multiple versions of a library and uses different versions of a library in different subtrees of a package tree. qt (for example) tries to solve the mixing problem by using a package set that pin the whole set of qt libraries into a consistent set (libsForQt5.callPackage instead of callPackage). It's still a mess if your package want to use both qt5 and qt6 at the same time (directly or indirectly).

I suggest avoiding one-by-one phrasing: # many packages still fail with latest version.

if a single address space loads multiple library versions there will likely be troubles

can we solve that by showing an error when evaluating a package that has dependencies that use the same library in different versions? it would be hard to know which package is a library and which is the same library, but in a different version. nix does not even have a concept of a package

we could have metadata for libraries like using outputs = [ "lib" "dev" ]; and define lib as default (out) and a policy to use the same package name and only use different version, so nix can compare them

since nix already knows all dependencies, i think it could be solvable at evaluation time

I think heuristics are possible but might be non-trivial to implement:

nix / nixpkgs does not exactly know it's runtime closures unti the package is built. At eval time we only see built depends in buildInputs / nativeBuildInputs and even that is both a too wide list and an incomplete list. Things like { foo }: stdenv.mkDerivation { .., postInstall = "echo LD_PRELOAD=${foo}/lib/libfoo.so.1 >> something"; } don't have the foo in *Inputs as an expression, but does have it as a derivation input.

There are rare cases when we mix libraries, like cross-compilation, bootstrap and copied libraries (libgcc_s.so is provided by gcc and glibc).

trofi · 2023-03-01T06:56:38Z

Build of both packages work now on i686-linux.

A note on i686-linux: hydra does not run i686-linux builds. Thus many unrelated failures are expected.

What hydra usually runs instead is a small subset of it as wine dependency via pkgsi686Linux.* namespace. Thus package name that targets i686 is normally pkgsi686Linux.tbb_2021_8 and pkgsi686Linux.tbb. They are identical to i686-linux, but they don't imply that you run it on a i686-linux system (I'm pretty sure your host is running a 64-bit x86_64 kernel and not a 32-bit i686 one).

davidak · 2023-03-01T07:20:06Z

Build on aarch64-linux fails because the dependency binutils-wrapper-2.40-aarch64-linux does not build.

davidak · 2023-03-01T08:13:56Z

They are identical to i686-linux, but they don't imply that you run it on a i686-linux system

@trofi what are you suggesting here?

Should i build pkgsi686Linux.tbb instead of tbb --system i686-linux when testing if a package builds on i686-linux? What advantage does that have? (i'm on x86_64-linux)

trofi · 2023-03-01T08:17:59Z

They are identical to i686-linux, but they don't imply that you run it on a i686-linux system

@trofi what are you suggesting here?

Should i build pkgsi686Linux.tbb instead of tbb --system i686-linux when testing if a package builds on i686-linux? What advantage does that have? (i'm on x86_64-linux)

Yeah, I suggest only testing pkgsi686Linux.tbb and pkgsi686Linux.tbb_2021_8 unless you really want to get i686-linux into a better shape.

davidak · 2023-03-01T08:30:06Z

Result of nixpkgs-review pr 217585 run on aarch64-linux 1

2 packages built:

tbb_2021_8
tbb_2021_8.dev (tbb_2021_8.dev.dev)

trofi · 2023-03-01T19:48:21Z

@ofborg build tbb_2020_3 tbb_2021_8 tbb pkgsi686Linux.tbb_2020_3 pkgsi686Linux.tbb_2021_8 pkgsi686Linux.tbb

trofi · 2023-03-01T19:55:20Z

Result of nixpkgs-review pr 217585 run on x86_64-linux 1

2 packages built:

tbb_2020_3
tbb_2021_8

trofi · 2023-03-01T19:56:51Z

aarch64-darwin fails as:

error: unknown warning option '-Werror=stringop-overflow'; did you mean '-Werror=shift-overflow'? [-Werror,-Wunknown-warning-option]

Can you have a look? Probably needs to drop our -Werror=stringop-overflow hack. Or make it conditional on gcc.

This is based on PR NixOS#214762. For the new release 2021.8, see https://www.intel.com/content/www/us/en/developer/articles/release-notes/intel-oneapi-threading-building-blocks-release-notes.html https://github.com/oneapi-src/oneTBB/releases/tag/v2021.5.0 https://github.com/oneapi-src/oneTBB/releases/tag/v2021.6.0 https://github.com/oneapi-src/oneTBB/releases/tag/v2021.7.0 Due to the significant breakage due to the update to TBB 2021.8, instead split the tbb package into tbb_2020_3 and tbb_2021_8, with the default tbb aliased to tbb_2020_3 in order to minimize breakage. Also fixed build issues and improved code. Co-Authored-By: Martin Weinelt <hexa@darmstadt.ccc.de> Co-Authored-By: davidak <git@davidak.de>

davidak · 2023-03-02T10:32:20Z

Can you have a look? Probably needs to drop our -Werror=stringop-overflow hack. Or make it conditional on gcc.

Yes, before i only edited it in the gcc cmake file.

davidak · 2023-03-02T10:42:03Z

Result of nixpkgs-review pr 217585 run on aarch64-linux 1

2 packages built:

tbb_2021_8
tbb_2021_8.dev (tbb_2021_8.dev.dev)

trofi · 2023-03-02T19:02:58Z

https://github.com/ofborg build tbb_2020_3 tbb_2021_8 tbb pkgsi686Linux.tbb_2020_3 pkgsi686Linux.tbb_2021_8 pkgsi686Linux.tbb

davidak · 2023-03-02T21:27:13Z

ofborg build failed

error: building of '/nix/store/srvihjp36ww37lcl83ylxns1s73cxsay-llvm-11.1.0.drv!lib,out' from .drv file timed out after 3600 seconds

error: building of '/nix/store/jg5aciin9lvh2m0264y4dr5hksld14nx-llvm-11.1.0.drv!lib,out' from .drv file timed out after 3600 seconds

@ofborg build tbb

trofi · 2023-03-02T22:09:38Z

nix build --no-link github:NixOS/nixpkgs/pull/217585/merge#pkgsLLVM.tbb{,_2020_3,_2021_8} succeded for me. I think it's a good proxy as a clang build test.

trofi · 2023-03-03T07:54:24Z

Let's declare it good enough for staging and sort the possible darwin failures if those come up in the follow-up PRs.

Pulled as is. Thank you!

davidak · 2023-03-03T13:36:08Z

I think a goal for tbb is to update the default to the latest version, like @mweinelt tried in #214762. But half the packages currently fail.

@trofi can you recommend a stretegy for that? Should we just wait for now for more packages to become compatible? Maybe monitor how other distros handle it.

trofi · 2023-03-03T18:58:02Z

That's a very good question!

It's a challenge to update commonly used libraries even with minor breakages.

Major breakages are especially hard and would require some porting effort. In any case we would need at least:

a PR with the package update attempt
a few build failure examples (logs) collected there
a few build fixes queued for the PR

Perfect fixes would work for both old and new tbb versions and would go upstream first. Picking fixes from other downstreams should be reasonable as well.

To offload some work from doing all the fixes myself I usually do:

post a topic on https://discourse.nixos.org/ with details on how breakages usually look like and how to fix it (sometimes upstreams already provide similar guides)
file bug reports to upstreams with some hints how to do porting to the new version
(once fixed most known breakages) request a hydra jobset against your OR to collect all the nixpkgs failures

Sometimes it works, sometimes it does not :)

A few past examples:

davidak mentioned this pull request Feb 21, 2023

tbb: 2020.3 -> 2021.8.0 #214762

Closed

13 tasks

davidak force-pushed the tbb-split branch from 0ff6b9f to 2f8dfa7 Compare February 21, 2023 23:37

davidak marked this pull request as ready for review February 22, 2023 00:12

trofi suggested changes Feb 22, 2023

View reviewed changes

davidak force-pushed the tbb-split branch from 2f8dfa7 to 6e86c06 Compare March 1, 2023 04:40

trofi reviewed Mar 1, 2023

View reviewed changes

pkgs/development/libraries/tbb/default.nix Show resolved Hide resolved

trofi suggested changes Mar 1, 2023

View reviewed changes

davidak force-pushed the tbb-split branch from 6e86c06 to 8758021 Compare March 1, 2023 08:02

ofborg bot added the 8.has: package (new) This PR adds a new package label Mar 1, 2023

ofborg bot requested review from hesiod and thoughtpolice March 1, 2023 08:28

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Mar 1, 2023

davidak force-pushed the tbb-split branch from 8758021 to ccae386 Compare March 1, 2023 11:54

trofi approved these changes Mar 1, 2023

View reviewed changes

davidak force-pushed the tbb-split branch from ccae386 to cc541cb Compare March 2, 2023 10:32

trofi merged commit a7a1966 into NixOS:staging Mar 3, 2023

davidak deleted the tbb-split branch March 3, 2023 10:01

This was referenced Mar 9, 2023

cctag: init at 1.0.3 #216402

Merged

Meshroom #216403

Draft

NickCao mentioned this pull request Apr 24, 2023

tbb: 2020.3 -> 2021.7.0 #195401

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tbb: Split into tbb_2020_3 and tbb_2021_8 #217585

tbb: Split into tbb_2020_3 and tbb_2021_8 #217585

davidak commented Feb 21, 2023 •

edited

Loading

davidak commented Feb 22, 2023 •

edited

Loading

davidak commented Feb 22, 2023

davidak commented Feb 24, 2023 •

edited

Loading

davidak commented Feb 24, 2023

davidak commented Mar 1, 2023 •

edited

Loading

trofi left a comment

trofi Mar 1, 2023

davidak Mar 1, 2023

trofi Mar 1, 2023

davidak Mar 1, 2023

trofi Mar 1, 2023

davidak Mar 1, 2023

davidak Mar 1, 2023

trofi Mar 1, 2023

davidak Mar 1, 2023

trofi Mar 1, 2023

trofi commented Mar 1, 2023

davidak commented Mar 1, 2023

davidak commented Mar 1, 2023

trofi commented Mar 1, 2023

davidak commented Mar 1, 2023

trofi commented Mar 1, 2023

trofi commented Mar 1, 2023

trofi commented Mar 1, 2023

davidak commented Mar 2, 2023

davidak commented Mar 2, 2023

trofi commented Mar 2, 2023

davidak commented Mar 2, 2023

trofi commented Mar 2, 2023

trofi commented Mar 3, 2023

davidak commented Mar 3, 2023

trofi commented Mar 3, 2023

tbb: Split into tbb_2020_3 and tbb_2021_8 #217585

tbb: Split into tbb_2020_3 and tbb_2021_8 #217585

Conversation

davidak commented Feb 21, 2023 • edited Loading

Description of changes

Things done

davidak commented Feb 22, 2023 • edited Loading

davidak commented Feb 22, 2023

davidak commented Feb 24, 2023 • edited Loading

davidak commented Feb 24, 2023

davidak commented Mar 1, 2023 • edited Loading

trofi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trofi commented Mar 1, 2023

davidak commented Mar 1, 2023

davidak commented Mar 1, 2023

trofi commented Mar 1, 2023

davidak commented Mar 1, 2023

trofi commented Mar 1, 2023

trofi commented Mar 1, 2023

trofi commented Mar 1, 2023

davidak commented Mar 2, 2023

davidak commented Mar 2, 2023

trofi commented Mar 2, 2023

davidak commented Mar 2, 2023

trofi commented Mar 2, 2023

trofi commented Mar 3, 2023

davidak commented Mar 3, 2023

trofi commented Mar 3, 2023

davidak commented Feb 21, 2023 •

edited

Loading

davidak commented Feb 22, 2023 •

edited

Loading

davidak commented Feb 24, 2023 •

edited

Loading

davidak commented Mar 1, 2023 •

edited

Loading