-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
rocmPackages.llvm: add setuptools #325909
Conversation
Ah alright, didn't notice that PR when I was searching for this failure for some reason... I was gonna try pinning to 311 but saw the first module error was able to be worked around with setuptools (according to docs). Looks like doronbehar ran into the same thing in that PR... |
distutils removed in python 3.12, need to add setuptools.
@superherointj This way seems to build it for me. But, it looks like it'll be a bigger rebuild... I'll try running a nixpkgs-review on it to see how it goes. Unless someone has a more clever way of attaching the
|
Le sad. Result of 11 packages marked as broken and skipped:
74 packages failed to build:
17 packages built:
|
Libcxx is failing on 3 tests. I see there's a large ignore tests file... sounds tempting...
|
Decided to update my system today of all days so i'm trying your fork, will post updates. |
Result of 11 packages marked as broken and skipped:
6 packages failed to build:
85 packages built:
|
rocmlir packages fail with [3067/3788] Building CXX object mlir/lib/Dialect/Rock/Transforms/CMakeFiles/obj.MLIRRockTransforms.dir/ViewToTransform.cpp.o
FAILED: mlir/lib/Dialect/Rock/Transforms/CMakeFiles/obj.MLIRRockTransforms.dir/ViewToTransform.cpp.o
/nix/store/13bz9lziadqbfrf14301zbkk8yj99fwg-rocm-llvm-clang-wrapper-6.0.2/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LIBCPP_ENABLE_ASSERTIONS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/build/source/build/mlir/lib/Dialect/Rock/Transforms -I/build/source/mlir/lib/Dialect/Rock/Transforms -I/build/source/external/llvm-project/llvm/include -I/build/source/build/external/llvm-project/llvm/include -I/build/sou>
In file included from /build/source/mlir/lib/Dialect/Rock/Transforms/ViewToTransform.cpp:14:
/build/source/mlir/include/mlir/Conversion/TosaToRock/TosaToRock.h:21:10: fatal error: 'mlir/Conversion/RocMLIRPasses.h.inc' file not found
#include "mlir/Conversion/RocMLIRPasses.h.inc"
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated. |
Looks like the profiler one was just a timeout during the mass rebuild. Result of 22 packages marked as broken and skipped:
5 packages failed to build:
86 packages built:
|
This is currently limited in scope to the _6 packages that were failing. Did we want to include the change to _5 or break them up? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on my host with AMD GPU and indeed it fixes the build [that was broken] and system is working.
If no other better solution is known atm, I think we should go ahead with this. Particularly because this unblock upgrades for those having AMD GPU.
Up to you. |
I'll break it up to let this unblock the currently broken packages that are affecting people, I dont know how many are using _5 atm. EDIT: looks like 3 dependencies of the _5 version only so I'll spin up another PR for that one. |
@acowley @mschwaig @lovesegfault @Flakebi @Madouura @doronbehar @SuperSandro2000 @ulrikstrid I think, the PR is ready for more reviews. Please, review it if you can. (And whoever has an interest in this.) I'd like to speed this merge because this is breaking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While trying to fix this I noticed the sources could use an update, perhaps you'd like to check them while at it.
@@ -86,7 +86,7 @@ in stdenv.mkDerivation (finalAttrs: { | |||
cmake | |||
ninja | |||
git | |||
python3Packages.python | |||
(python3Packages.python.withPackages (p: [ p.setuptools ])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What bothers me a bit is that recommonmark used a bit down below is not included in this environment. But if it works, I guess it is OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the first iteration of the PR was including it similar to that package, I dont understand why this works and that didn't...
@doronbehar Do you mean an actual version update from 6.0.2 to a new 6.1.2 or something? |
Yes. |
I'd say let's do one thing at a time. Getting it fixed and unblocking AMD GPU users is more important imo. As I understand it, the update is also a bit more involved, so could take additional time. |
Yeah, If I were to try bumping i'd be interested in trying to remove the disabled tests and might have to update patches to see what succeeds and the compile times for this are pretty long. EDIT: With that said, I am curious about bumping it so I'll try doing it locally and see how it goes and if I am able to get through it I'll open a PR for the bump. |
@doronbehar Can we merge this and unblock stable? While khaneliman calmly works in an upgrade solution? And then, we have to re-test. (Which also takes a long time.) We don't know yet how long this is going to take. My upgrade is not blocked because I have cherry-picked commits, my concern is mostly minding unstable users. |
Result of 22 packages marked as broken and skipped:
5 packages failed to build:
86 packages built:
|
From what i’ve seen this blocks not only AMD users but also steam users, as steam FHS adds all gpu drivers no matter what, including rocm. |
Fix copied from: https://github.com/NixOS/nixpkgs/pull/325909/files Related: NixOS/nixpkgs#325909 and NixOS/nixpkgs#253751
distutils removed in python 3.12, need to add setuptools.
EDIT: marked as draft because it still failed adding next to lit. even though https://docs.python.org/3.11/library/distutils.html#module-distutils says setuptools should resolve it. Not sure if it needs to be added somewhere else.Added setuptools to python scope for rocmPackages.llvm.
Added a few more tests to the list of excluded tests blocking libcxx.
Description of changes
Resolves #325907 #324235
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.