Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tbb: Split into tbb_2020_3 and tbb_2021_8 #215689

Closed
wants to merge 102 commits into from
Closed

Conversation

hesiod
Copy link
Contributor

@hesiod hesiod commented Feb 10, 2023

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
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 the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Feb 10, 2023
@hesiod hesiod requested review from mweinelt and alyssais February 10, 2023 14:50
This was referenced Feb 15, 2023
@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

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

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

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

@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 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 10.rebuild-linux: 1-10 labels Feb 18, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 101-500 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Feb 18, 2023
@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 6.topic: systemd 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

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

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.
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 6.topic: systemd 8.has: clean-up 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 10.rebuild-linux: 101-500 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.