Skip to content

packaging: remove unused dependency on onetbb#14735

Closed
uninsane wants to merge 1 commit into
NixOS:masterfrom
uninsane:pr-no-onetbb
Closed

packaging: remove unused dependency on onetbb#14735
uninsane wants to merge 1 commit into
NixOS:masterfrom
uninsane:pr-no-onetbb

Conversation

@uninsane
Copy link
Copy Markdown

@uninsane uninsane commented Dec 8, 2025

#14509 configured meson to not use any onetbb features. nixpkgs exposes an option to blake3 to remove onetbb from the runtime closure as well, so enable that.

Motivation

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@uninsane uninsane requested a review from edolstra as a code owner December 8, 2025 18:16
@uninsane uninsane requested a review from Mic92 December 8, 2025 18:17
@edolstra edolstra enabled auto-merge December 8, 2025 18:38
@xokdvium
Copy link
Copy Markdown
Contributor

xokdvium commented Dec 8, 2025

Isn't onetbb used to make hash calculation multithreaded?

NixOS#14509 configured meson to not use any
onetbb features. nixpkgs exposes an option to blake3 to remove onetbb
from the runtime closure as well, so enable that.

Co-authored-by: Philip Taron <philip.taron@gmail.com>
auto-merge was automatically disabled December 8, 2025 18:40

Head branch was pushed to by a user without write access

@xokdvium
Copy link
Copy Markdown
Contributor

xokdvium commented Dec 8, 2025

We are not using any of the features ourselves, but iirc blake3 relies on it for parallel hash computation. Would be nice to clarify this.

Copy link
Copy Markdown
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to better understand
the implications of this.

@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Dec 8, 2025

cc @silvanshade

@philiptaron
Copy link
Copy Markdown
Contributor

We are not using any of the features ourselves, but iirc blake3 relies on it for parallel hash computation. Would be nice to clarify this.

You are correct, however there is no meson option to turn this on.

#ifdef BLAKE3_USE_TBB

So unless there's something out-of-tree, it's effectively dead code at the moment.

@xokdvium
Copy link
Copy Markdown
Contributor

xokdvium commented Dec 8, 2025

So unless there's something out-of-tree, it's effectively dead code at the moment.

That define comes from blake3. There's no option needed to turn it on

@xokdvium
Copy link
Copy Markdown
Contributor

xokdvium commented Dec 8, 2025

Specifically from the pkg-config: https://github.com/BLAKE3-team/BLAKE3/blob/308b95dfa15d5a0aa8cb3c5534ffd90d76122c46/c/CMakeLists.txt#L250C35-L250C49

cat result-dev/lib/pkgconfig/libblake3.pc
prefix="/nix/store/nvzryxzj8yqbsws3a9inmpqs9xrl8v58-libblake3-1.8.2"
exec_prefix="${prefix}"
libdir="/nix/store/nvzryxzj8yqbsws3a9inmpqs9xrl8v58-libblake3-1.8.2/lib"
includedir="/nix/store/nb6955lvlxl0a5hs99jchgjcnzadjh4g-libblake3-1.8.2-dev/include"

Name: libblake3
Description: BLAKE3 C implementation
Version: 1.8.2

Requires: tbb >= 2022.3.0
Libs: -L"${libdir}" -lblake3 -lstdc++
Cflags: -I"${includedir}" -DBLAKE3_DLL -DBLAKE3_USE_TBB

@philiptaron
Copy link
Copy Markdown
Contributor

Ah, I understand. So the proposed PR disabled that by changing the pkg-config file that libblake3 shipped.

@Ericson2314
Copy link
Copy Markdown
Member

OneTTB is an optional dependency of Blake3. I don't care if our experimental blake3 hashing is fast or not. Nixpkgs is free to hard-disable OneTBB in blake3. Nix I think should still support Blake3 built either way in case other non-Nix packaging of blake3 enables this, but we don't need to test that, we can just keep the small bit of CPP around as a minimal effort attempt.

@Ericson2314
Copy link
Copy Markdown
Member

https://github.com/NixOS/nix/pull/14509/files this change is about the C++ standard library, and should be viewed as completely orthogonal knob --- all combinations of blake3 with/without TBB, and libstdc++ with/without TBB should work. This just exists to undo an (IMO poor form) header hack impl in libstc++ to auto-enable this feature.

@Ericson2314
Copy link
Copy Markdown
Member

I don't think we need to do this PR because I think Nix should just use whatever blake3 Nixpkgs wants to give us. I want to purely follow Nixpkg's lead here.

@uninsane uninsane closed this Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants