Conversation
579fff9 to
41b908c
Compare
41b908c to
e0d0d1c
Compare
peterhoeg
left a comment
There was a problem hiding this comment.
thank you for taking this on. There are a few things related to the version we keep around as well as comments.
feat: bump 1.6.x to 1.6.1 fix: fetch 12601 patch for 1.3.x-1.6.0 fix: darwin url for versions < 1.2.0
or > 1.5.x and aarch64-linux
|
I'm not really sure how to deal with the current error.
|
|
Also I'm still not super-convinced about compiling it in release mode, as straight-shoota mentioned:
It does take awfully long to compile the compiler AND the compiler spec AND run them when release mode is on. |
so 12601 gets applied successfully
| LLVM_CONFIG = "${llvmPackages.llvm.dev}/bin/llvm-config"; | ||
|
|
||
| FLAGS = [ | ||
| "--release" |
There was a problem hiding this comment.
I guess we no longer need to do this as you're patching the versions that do not support it.
Otherwise good to. Once this is reinstated I'll squash and merge.
Thanks a ton @GeopJr !!!
There was a problem hiding this comment.
The release flag moved to makeFlags (which has the same effect as far as I am aware) from the #173928 PR. Is there another reason to include it in FLAGS?
|
ofborg still has an issue with the platforms that I am too unfamiliar with Nix to solve |
|
Thanks for pushing this. Looks like some failed depending pkgs need updates or pinning to 1.2.
Update: Invidious Tested.
Result: Invidious works great with this as-is. 🎉 Sign-up, login, subscriptions, trending, video viewing all work without error, and upgrading to this version from 1.2 finally cleans up some nasty log noise about unhandled SSL ctrl codes that's been a problem for ages. Ship it! ...at least when the other stuff is fixed! 😃 Result of 3 packages failed to build:
13 packages built:
|
|
Suggested fixes for downstream build errors: scry and mint: Quickfix by using crystal 1.2 for now. diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index 49d1abbf9d05f..ed61424497ce7 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -13455,7 +13455,7 @@ with pkgs;
icr = callPackage ../development/tools/icr { };
- scry = callPackage ../development/tools/scry { };
+ scry = callPackage ../development/tools/scry { crystal = crystal_1_2; };
dasm = callPackage ../development/compilers/dasm { };
@@ -14500,7 +14500,7 @@ with pkgs;
millet = callPackage ../development/tools/millet {};
- mint = callPackage ../development/compilers/mint { };
+ mint = callPackage ../development/compilers/mint { crystal = crystal_1_2; };
mitscheme = callPackage ../development/compilers/mit-scheme
{ stdenv = gcc10StdenvCompat; texLive = texlive.combine { inherit (texlive) scheme-small epsf texinfo; }; };ameba: 1.0.1 -> 1.3.1. Couldn't immediately get 1.0.1 to build on old crystal. 1.3.1 builds fine on new crystal. diff --git a/pkgs/development/tools/ameba/default.nix b/pkgs/development/tools/ameba/default.nix
index 4239f5c0056a9..27068cbc4c49f 100644
--- a/pkgs/development/tools/ameba/default.nix
+++ b/pkgs/development/tools/ameba/default.nix
@@ -2,13 +2,13 @@
crystal.buildCrystalPackage rec {
pname = "ameba";
- version = "1.0.1";
+ version = "1.3.1";
src = fetchFromGitHub {
owner = "crystal-ameba";
repo = "ameba";
rev = "v${version}";
- hash = "sha256-dvhGk6IbSV3pxtoIV7+0+qf47hz2TooPhsSwFd2+xkw=";
+ hash = "sha256-SZ2sBQeZgtPOYioH9eK5MveFtWVGPvgKMrqsCfjoRGM=";
};
format = "make";
0 packages fail to build with the above. |
I believe you can use |
|
Apologies if you know the below already. Throwing in an x86 darwin build of the latest here. Looks like a similar ld error. Result of 13 packages failed to build:
|
|
|
||
| inherit (callPackages ../development/compilers/crystal { | ||
| llvmPackages = if stdenv.system == "aarch64-darwin" then llvmPackages_11 else llvmPackages_10; | ||
| llvmPackages = llvmPackages_13; |
There was a problem hiding this comment.
Something I don't get from the x86-darwing error is that it refers to clang-11, but shouldn't it refer to clang-13?
Maybe we could try sticking with llvmPackages_11 here? I'm not sure for the underlying reason, but the llvm package in nixpkgs points to 11.
Or we are missing some path to use llvmPackages_13.clang
There was a problem hiding this comment.
:/ Closest discussion I could find: https://discourse.nixos.org/t/nixpkgs-aarch64-darwin-build-m1-uses-wrong-old-clang-how-to-fix/ (in our case x86_64 instead of aarch64)
the suggested change adds clang-unwrapped to PATH
nixpkgs/pkgs/build-support/libredirect/default.nix
Lines 35 to 49 in 6de1617
Someone on darwin needs to give it a try otherwise I'll just revert to llvmPackages_11 for x86_64
The nix build definitely downloaded clang-13:
...
/nix/store/5b4s502i0ifbivjfb0pwxjlf7vrwgr2v-clang-13.0.1-lib
...
/nix/store/dx1q2d5zh3f45y8amkisy5lbq9b48zsx-clang-13.0.1
...
/nix/store/myqkicxg2zg8wmc84k048hn6fznilqm9-clang-wrapper-13.0.1
...
|
On macOS 12.6.1 (Monterrey) Building We can see that the missing On top of this branch I attempted to use llvm 13 clang by setting the We can see that If I downgrade I still get the I think the problem might be on libevent package actually. The lib is already built and references Given that curl/curl#5210 is similar and it was resolved eventually with an xcode upgrade maybe the issue here is how libevent was built on x86-64_darwin initially. I attempted to change something in the libevent formula to trigger a rebuilt locally Unfortunately, the I am not sure how to maybe tweak which xcode version should be used. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
(At the time of writing this) While I made the Crystal package use sdk 11 in an attempt to get rid of Unfortunately the |
|
It finished! So our only issue now is |
Quite the rabbit hole - I was trying to find more info on
Awesome! I'll run a review to see if anything is broken on 1.7.1 and then try to move this forward! |
|
x86 darwin: Crystal itself looks like it's out of the woods! Nice! Result of 5 packages failed to build:
8 packages built:
|
|
Thanks for the review @miangraham! Unless someone on darwin wants to debug the failing packages, I'm ok with leaving them as is (if allowed), considering they haven't been built in darwin for ages and might need some workarounds from their maintainers. This doesn't seem to be a Crystal version issue: ameba, oq, thicket, crystal2nix should work on 1.7 and mint is locked to 1.2. |
Description of changes
Changelog
This is a copy of #173928 but for
1.2 -> 1.6with some additional changes.Changes:
libffiis needed nowTMPDIR=$TMP(used in runtime forDir.tempdir)aarch64-linuxas 1.5 is not yet availableNot sure if this should have been a review instead to the original draft but it targets different versions.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes