Skip to content

Comments

tbb: Split into tbb_2020_3 and tbb_2021_8#215689

Closed
hesiod wants to merge 102 commits intoNixOS:stagingfrom
hesiod:tbb-split
Closed

tbb: Split into tbb_2020_3 and tbb_2021_8#215689
hesiod wants to merge 102 commits intoNixOS:stagingfrom
hesiod:tbb-split

Conversation

@hesiod
Copy link
Contributor

@hesiod hesiod commented Feb 10, 2023 •
edited
Loading

Description of changes

This is based on PR #214762 by @mweinelt.

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.

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.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@hesiod hesiod mentioned this pull request Feb 10, 2023
Closed
13 tasks
@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Feb 10, 2023
@ofborg ofborg bot requested a review from thoughtpolice February 10, 2023 14:43
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 10, 2023
@hesiod hesiod requested review from alyssais and mweinelt February 10, 2023 14:50
This was referenced Feb 15, 2023
Merged
Draft
@davidak
Copy link
Member

davidak commented Feb 15, 2023

For some reason the build fails: #216403 (comment)

Also, merge conflict.

@hesiod
Copy link
Contributor Author

hesiod commented Feb 17, 2023 •
edited
Loading

I fixed the merge conflict while rebasing, not sure why Github says there's still a conflict... nevermind, there was another change to TBB on master just a few hours ago

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 17, 2023
@hesiod hesiod force-pushed the tbb-split branch 2 times, most recently from a5826c8 to e58b743 Compare February 17, 2023 13:53
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 17, 2023
@davidak
Copy link
Member

davidak commented Feb 17, 2023 •
edited
Loading

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

1 package failed to build:
  • tbb_2021_8
1 package built:
  • tbb_2020_3

See build log: curl https://termbin.com/y7w3m

chuangzhu
chuangzhu reviewed Feb 18, 2023
View reviewed changes
Copy link
Contributor

@chuangzhu chuangzhu left a comment

Choose a reason for hiding this comment

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

GCC 12 isn't supported in v2021.8.0 according to uxlfoundation/oneTBB#823, and patches from uxlfoundation/oneTBB#866 doesn't seem to be applicable :(

@davidak
Copy link
Member

davidak commented Feb 18, 2023 •
edited
Loading

@chuangzhu i see. v2021.8.0 supports

GNU Compilers (gcc) 4.8.5 - 11.1.1

Source: https://github.com/oneapi-src/oneTBB/blob/v2021.8.0/SYSTEM_REQUIREMENTS.md#supported-compilers

while v2020.3 supports

GNU Compilers (gcc) 4.8 - 9.1

Source: https://github.com/oneapi-src/oneTBB/blob/v2020.3/doc/Release_Notes.txt#L93

So i think we should use the supported versions.

I hope it's not an issue we have GCC 11.3.0 and GCC 9.5.0.

I will do the change.

@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 18, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Feb 18, 2023
marsam and others added 8 commits February 18, 2023 19:02
@marsam @davidak
nodejs-19_x: 19.6.0 -> 19.6.1
b8438b5 
https://github.com/nodejs/node/releases/tag/v19.6.1
@r-ryantm @davidak
cargo-valgrind: 2.1.0 -> 2.1.1
a9a9f80
@iblech @davidak
yt-dlp: 2023.1.6 -> 2023.2.17
ac51c5e
@davidak
maintaniers: remove dizfer
9b3f256
@marsam @davidak
flexget: 3.5.23 -> 3.5.24
7ebc968
@hesiod @davidak
tbb: Split into tbb_2020_3 and tbb_2021_8
aad02ff 
This is based on PR NixOS#214762 by @mweinelt.

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.
@davidak @chuangzhu
tbb_2021_8: use gcc11
2bb0593 
Supported Compilers:

- GNU Compilers (gcc) 4.8.5 - 11.1.1

Source: https://github.com/oneapi-src/oneTBB/blob/v2021.8.0/SYSTEM_REQUIREMENTS.md#supported-compilers

Co-authored-by: Chuang Zhu <[email protected]>
@davidak
tbb_2020_3: use gcc9
07b34cd 
Supported Compilers:

- GNU Compilers (gcc) 4.8 - 9.1

Source: https://github.com/oneapi-src/oneTBB/blob/v2020.3/doc/Release_Notes.txt#L93

Also remove patch for GCC 13 compatibility as we don't use that (yet).
@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 18, 2023
@davidak davidak changed the base branch from master to staging February 18, 2023 18:06
@mweinelt
Copy link
Member

mweinelt commented Feb 18, 2023

Can you not mention my account name in the commit message? It causes lots of sticky notifications, that I have to manually mark as read on every commit/branch/fork.

Instead, please add me as co-author at the end of the commit message.

Co-Authored-By: Martin Weinelt <[email protected]

@davidak
Copy link
Member

davidak commented Feb 18, 2023

I followed the rebase instructions, but somehow everything is messed up. That is not intentional!

https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#rebasing-between-branches-ie-from-master-to-staging

@mweinelt
Copy link
Member

mweinelt commented Feb 18, 2023

I'm locking and closing this pull request, because through the rebase you inadvertently requested many maintainers for review, which subscribed them to update notifications, resulting in unnecessary spam inside everyone's inbox. While we can remove their review requests, we sadly cannot unsubscribe anyone.

Please create a new pull request and for the next time remember to set your PR to draft status before rebasing. In draft status, you can preview the list of maintainers that are about to be requested for review, which allows you to sidestep this issue.

@mweinelt mweinelt closed this Feb 18, 2023
@NixOS NixOS locked as resolved and limited conversation to collaborators Feb 18, 2023
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

@mweinelt mweinelt Awaiting requested review from mweinelt

@alyssais alyssais Awaiting requested review from alyssais

@zowoq zowoq Awaiting requested review from zowoq

@jtojnar jtojnar Awaiting requested review from jtojnar

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@adisbladis adisbladis Awaiting requested review from adisbladis

@jbedo jbedo Awaiting requested review from jbedo

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

1 more reviewer

@chuangzhu chuangzhu chuangzhu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 8.has: clean-up This PR removes packages or removes other cruft 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.