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.

Sign up for GitHub

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

onnxruntime: fix aarch64-darwin #287255

Merged
merged 1 commit into from
Feb 10, 2024
Merged

onnxruntime: fix aarch64-darwin #287255

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions pkgs/development/libraries/eigen/default.nix
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, right, many people use Eigen...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I hope I avoided the mass ping issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbourjau I retargeted the PR for staging because it's a lot of rebuilds; if you need onnxruntime on aarch64-darwin working right now, you might have to submit a temporary hotfix (TM) for the master, doing essentially the ad hoc override much like the one you started with (only without fetchcontent)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might it make sense to only apply the patch for darwin since the build seems to be just fine on other systems?

The original MR doesn't seem darwin-specific, and I personally dislike conditional patches (they're kind of like extra moving pieces, it's too real for my small reptile brain). We wouldn't save much either because once you have to bootstrap for one platform it's already "expensive"? I'm open to hear other opinions though

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{ lib
, stdenv
, fetchFromGitLab
, fetchpatch
, cmake
}:

Expand All @@ -17,6 +18,20 @@ stdenv.mkDerivation rec {

patches = [
./include-dir.patch

# Fixes e.g. onnxruntime on aarch64-darwin:
# https://hydra.nixos.org/build/248915128/nixlog/1,
# originally suggested in https://github.com/NixOS/nixpkgs/pull/258392.
#
# The patch is from
# ["Fix vectorized reductions for Eigen::half"](https://gitlab.com/libeigen/eigen/-/merge_requests/699)
# which is two years old,
# but Eigen hasn't had a release in two years either:
# https://gitlab.com/libeigen/eigen/-/issues/2699.
(fetchpatch {
url = "https://gitlab.com/libeigen/eigen/-/commit/d0e3791b1a0e2db9edd5f1d1befdb2ac5a40efe0.patch";
hash = "sha256-8qiNpuYehnoiGiqy0c3Mcb45pwrmc6W4rzCxoLDSvj0=";
})
];

nativeBuildInputs = [ cmake ];
Expand Down
Loading