Skip to content

Comments

tbb: Split into tbb_2020_3 and tbb_2021_8#216991

Closed
davidak wants to merge 1 commit intoNixOS:stagingfrom
davidak:tbb-split
Closed

tbb: Split into tbb_2020_3 and tbb_2021_8#216991
davidak wants to merge 1 commit intoNixOS:stagingfrom
davidak:tbb-split

Conversation

@davidak
Copy link
Member

@davidak davidak commented Feb 18, 2023 •
edited
Loading

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.

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

cc co-author @hesiod

Things done
  • Built on platform(s)
    • x86_64-linux
    • i686-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.

@mweinelt
Copy link
Member

mweinelt commented Feb 18, 2023 •
edited
Loading

Stop spamming me ASAP. I can't deal with it. Remove my account name from bede796 (#216991) NOW.

Screenshot 2023-02-18 at 19-32-10 Build software better together

#215689 (comment)

@davidak davidak force-pushed the tbb-split branch 2 times, most recently from 0fe3c66 to f66f83e Compare February 18, 2023 18:35
@davidak
Copy link
Member Author

davidak commented Feb 18, 2023

@mweinelt done!

I was confused what you meant as i did not create the commit.

Sorry! 😞

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Feb 18, 2023
@ofborg ofborg bot requested review from hesiod and thoughtpolice February 18, 2023 20:05
@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. labels Feb 18, 2023
@davidak davidak marked this pull request as ready for review February 19, 2023 05:26
@davidak
Copy link
Member Author

davidak commented Feb 19, 2023 •
edited
Loading

tbb_2021_8 fails on i686-linux (32-bit) (uxlfoundation/oneTBB#1030) as reported before (#214762 (comment))

this potential fix does not help, so i removed it again:

davidak@317b125#diff-91a76027dc94eb552c6b234f638302d4e71d33b932bb3a5f61d70b7fbc185ed3R31-R33

@davidak davidak mentioned this pull request Feb 20, 2023
Open
@trofi
Copy link
Contributor

trofi commented Feb 20, 2023

@trofi i removed your gcc13 fix as they don't support gcc13 (0e1c997). when they do, it should not be needed

That is a bit unusual reason to remove a patch that fixes a) guaranteed build failure on a compiler that will soon release b) accepted upstream. Does patch fail to apply or cause other issues?

trofi
trofi suggested changes Feb 20, 2023
View reviewed changes
@mweinelt mweinelt mentioned this pull request Feb 20, 2023
Closed
13 tasks
@davidak
Copy link
Member Author

davidak commented Feb 21, 2023 •
edited
Loading

@trofi i applied the patch from uxlfoundation/oneTBB#866 but the build of tbb_2021_8 still fails.

tbb> [ 65%] Building CXX object test/CMakeFiles/conformance_tick_count.dir/conformance/conformance_tick_count.cpp.o
tbb> In file included from /nix/store/0spl4agq94fairydh94xjmsm47zfk684-gcc-12.2.0/include/c++/12.2.0/atomic:41,
tbb>                  from /build/source/test/common/doctest.h:2990,
tbb>                  from /build/source/test/common/test.h:32,
tbb>                  from /build/source/test/tbb/test_concurrent_monitor.cpp:26:
tbb> In member function 'void std::__atomic_base<_IntTp>::store(__int_type, std::memory_order) [with _ITp = bool]',
tbb>     inlined from 'void std::atomic<bool>::store(bool, std::memory_order)' at /nix/store/0spl4agq94fairydh94xjmsm47zfk684-gcc-12.2.0/include/c++/12.2.0/atomic:104:20,
tbb>     inlined from 'void tbb::detail::r1::concurrent_monitor_base<Context>::abort_all_relaxed() [with Context = long unsigned int]' at /build/source/test/tbb/../../src/tbb/concurrent_monitor.h:430:53:
tbb> /nix/store/0spl4agq94fairydh94xjmsm47zfk684-gcc-12.2.0/include/c++/12.2.0/bits/atomic_base.h:464:25: error: 'void __atomic_store_1(volatile void*, unsigned char, int)' writing 1 byte into a region of size 0 overflows the destination [-Werror=stringop-overflow=]
tbb>   464 |         __atomic_store_n(&_M_i, __i, int(__m));
tbb>       |         ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
tbb> compilation terminated due to -Wfatal-errors.
tbb> cc1plus: all warnings being treated as errors
tbb> make[2]: *** [test/CMakeFiles/test_concurrent_monitor.dir/build.make:76: test/CMakeFiles/test_concurrent_monitor.dir/tbb/test_concurrent_monitor.cpp.o] Error 1
tbb> make[1]: *** [CMakeFiles/Makefile2:2234: test/CMakeFiles/test_concurrent_monitor.dir/all] Error 2

The error look similar to uxlfoundation/oneTBB#843. I try the fix.

@ofborg ofborg bot added 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. and removed 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. labels Feb 21, 2023
@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: xfce The Xfce Desktop Environment labels Feb 21, 2023
@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label Feb 21, 2023
@hesiod @mweinelt @davidak
tbb: Split into tbb_2020_3 and tbb_2021_8
7f6825d 
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.

Co-Authored-By: Martin Weinelt <[email protected]>
Co-Authored-By: davidak <[email protected]>
@github-actions github-actions bot removed 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: GNOME GNOME desktop environment and its underlying platform 8.has: documentation This PR adds or changes documentation 6.topic: xfce The Xfce Desktop Environment labels Feb 21, 2023
@davidak
Copy link
Member Author

davidak commented Feb 21, 2023

A rebase added again many unrelated commits and added notification for unrelated people. Sorry

@davidak davidak closed this Feb 21, 2023
@trofi
Copy link
Contributor

trofi commented Feb 21, 2023

I removed extra reviewers.

@davidak
Copy link
Member Author

davidak commented Feb 21, 2023

mweinelt said they still get notifications and that can't get removed. So this PR is wasted, like the other one 😬

@davidak davidak deleted the tbb-split branch February 21, 2023 22:22
@davidak davidak mentioned this pull request Feb 21, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hesiod hesiod Awaiting requested review from hesiod

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

2 more reviewers

@trofi trofi trofi requested changes

@chuangzhu chuangzhu chuangzhu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

8.has: package (new) This PR adds a new package 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@davidak @mweinelt @trofi @chuangzhu @hesiod