Skip to content

haskellPackages.hermes-json: fix vendored simdjson with clang-19#355529

Merged
emilazy merged 1 commit intoNixOS:llvm-19from
paparodeo:hermes-json-fix-vendored-simjson
Nov 13, 2024
Merged

haskellPackages.hermes-json: fix vendored simdjson with clang-19#355529
emilazy merged 1 commit intoNixOS:llvm-19from
paparodeo:hermes-json-fix-vendored-simjson

Conversation

@paparodeo
Copy link
Contributor

@paparodeo paparodeo commented Nov 12, 2024

updated code taken from simdjson/simdjson@5d35e7c
tried to un-vendor but the tests failed so created a patch

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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.

Add a 👍 reaction to pull requests you find important.

@emilazy

@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Nov 12, 2024
@paparodeo
Copy link
Contributor Author

paparodeo commented Nov 13, 2024

could un-vendor by taking ToT from https://github.com/velveteer/hermes. one hacky way to do it is pulling patches.

diff --git a/pkgs/development/haskell-modules/configuration-common.nix b/pkgs/development/haskell-modules/configuration-common.nix
index c89549e52162..e2aff42ce78a 100644
--- a/pkgs/development/haskell-modules/configuration-common.nix
+++ b/pkgs/development/haskell-modules/configuration-common.nix
@@ -2585,7 +2585,29 @@ self: super: {
 
     # vendored simdjson breaks with clang-19
     # https://github.com/simdjson/simdjson/commit/5d35e7ca1f1727ca57d31d4ae5f3954fe96337e3
-    patches = drv.patches or [] ++ [ ./patches/hermes-json-clang-19.diff ];
+    #patches = drv.patches or [] ++ [ ./patches/hermes-json-clang-19.diff ];
+    patches = drv.patches or [] ++ [
+      (fetchpatch {
+        url = "https://github.com/velveteer/hermes/commit/6fd9904d93a5c001aadb27c114345a6958904d71.patch";
+        hash = "sha256-Pv09XP0/VjUiAFp237Adj06PIZU21mQRh7guTlKksvA=";
+        excludes = [
+          ".github/*"
+          "hermes-bench/*"
+        ];
+      })
+      (fetchpatch {
+        url = "https://github.com/velveteer/hermes/commit/ca8dddbf52f9d7788460a056fefeb241bcd09190.patch";
+        hash = "sha256-tDDGS0QZ3YWe7+SP09wnxx6lIWL986ce5Zhqr7F2sBk=";
+        excludes = [
+          "README.md"
+          ".github/*"
+          "hermes-bench/*"
+        ];
+      })
+    ];
+    postPatch = drv.postPatch or "" + ''
+      ln -fs ${pkgs.simdjson.src} simdjson
+    '';
   }) super.hermes-json;
 
   # Disabling doctests.

@maralorn
Copy link
Member

I’d prefer that hack, if you don’t mind.

@emilazy
Copy link
Member

emilazy commented Nov 13, 2024

I take it there's no way to give this package a pre built simdjson at all?

@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. labels Nov 13, 2024
@paparodeo
Copy link
Contributor Author

I take it there's no way to give this package a pre built simdjson at all?

https://github.com/velveteer/hermes/blob/master/hermes-json.cabal

tho note that simdjson is not built as a shared library, contains a 4MB header, 2MB cpp file and the .a file is 88K

@emilazy
Copy link
Member

emilazy commented Nov 13, 2024

OK, works for me then. @maralorn Is there a way to just use a GitHub source directly for a Haskell package rather than reconstructing it with patches?

@maralorn
Copy link
Member

Sure there is, just override the src, but we generally prefer patching because it makes it more transparent how we are modifying packages and it automatically fails to apply when the fix is released by upstream.

@emilazy
Copy link
Member

emilazy commented Nov 13, 2024

Fair enough. I am fine with @paparodeo’s proposed diff, then.

@emilazy emilazy merged commit 8616ed9 into NixOS:llvm-19 Nov 13, 2024
@paparodeo paparodeo deleted the hermes-json-fix-vendored-simjson branch November 13, 2024 14:37
emilazy added a commit to emilazy/nixpkgs that referenced this pull request Nov 16, 2024
emilazy added a commit to emilazy/nixpkgs that referenced this pull request Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 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

Development

Successfully merging this pull request may close these issues.

3 participants