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.

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

Crystal: 0.24.2 -> 0.25.0 #41834

Merged
merged 3 commits into from Jun 18, 2018
Merged

Crystal: 0.24.2 -> 0.25.0 #41834

merged 3 commits into from Jun 18, 2018

Conversation

manveru
Copy link
Contributor

@manveru manveru commented Jun 11, 2018

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@Mic92
Copy link
Member

Mic92 commented Jun 11, 2018

@GrahamcOfBorg build crystal

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: crystal

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: crystal

Partial log (click to expand)

shrinking /nix/store/r4ffxfin754vfr9wqkjh4cyam33kc9hm-crystal-0.25.0/lib/crystal/ext/sigfault.o
wrong ELF type
shrinking /nix/store/r4ffxfin754vfr9wqkjh4cyam33kc9hm-crystal-0.25.0/lib/crystal/llvm/ext/llvm_ext.o
wrong ELF type
gzipping man pages under /nix/store/r4ffxfin754vfr9wqkjh4cyam33kc9hm-crystal-0.25.0/share/man/
patching script interpreter paths in /nix/store/r4ffxfin754vfr9wqkjh4cyam33kc9hm-crystal-0.25.0
checking for references to /build in /nix/store/r4ffxfin754vfr9wqkjh4cyam33kc9hm-crystal-0.25.0...
wrong ELF type
wrong ELF type
/nix/store/r4ffxfin754vfr9wqkjh4cyam33kc9hm-crystal-0.25.0

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: crystal

Partial log (click to expand)

ar -rcs src/ext/libcrystal.a src/ext/sigfault.o
./bin/crystal build --no-debug --release  -o .build/crystal src/compiler/crystal.cr -D without_openssl -D without_zlib
Using /nix/store/bvrz6gfvmgb620knx2d9nr8kvz4l21i7-llvm-5.0.2/bin/llvm-config [version=5.0.2]
./bin/crystal docs -b https://crystal-lang.org/api/latest src/docs_main.cr
Using compiled compiler at `.build/crystal'
installing
post-installation fixup
gzipping man pages under /nix/store/wjqg8sdcikrymh35rv4dxqadxmlbdcsk-crystal-0.25.0/share/man/
patching script interpreter paths in /nix/store/wjqg8sdcikrymh35rv4dxqadxmlbdcsk-crystal-0.25.0
/nix/store/wjqg8sdcikrymh35rv4dxqadxmlbdcsk-crystal-0.25.0

@Mic92
Copy link
Member

Mic92 commented Jun 11, 2018

@GrahamcOfBorg build mint

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: mint

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@Mic92
Copy link
Member

Mic92 commented Jun 11, 2018

mint failed to build for me, I have not checked if it failed before.

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: mint

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Failure on x86_64-linux (full log)

Attempted: mint

Partial log (click to expand)

            ^~~~

in lib/baked_file_system/src/loader/loader.cr:31: undefined method 'stat' for File:Class

        io << "  size:            " << File.stat(path).size << ",\n"
                                            ^~~~


builder for '/nix/store/dhsjd0pfvdkrshhp1xcsj9nnipnfx2jj-mint-0.0.3.drv' failed with exit code 1
error: build of '/nix/store/dhsjd0pfvdkrshhp1xcsj9nnipnfx2jj-mint-0.0.3.drv' failed

@manveru
Copy link
Contributor Author

manveru commented Jun 11, 2018

Yeah, looks like there are some incompatible changes in 0.25, one of them is removal of File.stat in favor of File.info... I'll see if we can get Mint fixed today.

Copy link
Member

@sifmelcara sifmelcara left a comment

Choose a reason for hiding this comment

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

Besides the mint issue, I have some comments


src = fetchurl {
url = "https://github.com/crystal-lang/crystal/archive/${version}.tar.gz";
sha256 = "1l7nrrfgz1yxxjphypwzlxj6dbari20p71zb4l0gix09lmas8l6y";
sha256 = "1pnx21ky6cqfyv6df4mmjnyd1yh1bvcqkdzq6f0mk0yrkcl57k3q";
};

prebuiltName = "crystal-0.24.2-1";
Copy link
Member

Choose a reason for hiding this comment

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

I think we can update prebuilt binaries to 0.25 now? As it have been released 10 hours ago

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good.

@@ -54,9 +54,13 @@ stdenv.mkDerivation rec {
makeFlags = [ "CRYSTAL_CONFIG_VERSION=${version}"
"FLAGS=--no-debug"
"release=1"
"all" "docs"
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... why move make docs to postBuild?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because you couldn't make docs with the old crystal, so the new crystal had to be built first. We probably should disable paralell builds for this too.

Copy link
Member

Choose a reason for hiding this comment

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

Or we can update the prebuilt binary version, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly, that just wasn't available at the time.

@sifmelcara
Copy link
Member

Maybe shards need to be updated at the same time?

@manveru
Copy link
Contributor Author

manveru commented Jun 16, 2018

@sifmelcara should all be working now

@sifmelcara
Copy link
Member

sifmelcara commented Jun 16, 2018

Maybe we should ask mint's maintainer to release a version that is compatible to crystal 0.25?
Using a revision in mint's non-master branch feels not right and it might broken in the future if the branch got deleted.

@manveru
Copy link
Contributor Author

manveru commented Jun 16, 2018

OK, asked him, and he'll release a new version when kemalcr/kemal#452 is merged. So it'll take a bit longer still.

@manveru
Copy link
Contributor Author

manveru commented Jun 17, 2018

Mint 0.0.4 was just released, so I think we're ready for takeoff 💃

@sifmelcara
Copy link
Member

     wrapProgram $out/bin/crystal \
+        --suffix PATH : ${clang}/bin \
         --suffix CRYSTAL_PATH : lib:$out/lib/crystal \

Why add clang to crystal's PATH?

Besides this, looks good to me (tested on NixOS x86_64)

@manveru
Copy link
Contributor Author

manveru commented Jun 17, 2018

Basically, this allows you to use crystal build without having to add clang to your own PATH, since that's needed for building anything i thought it might be more user-friendly. It doesn't really make the closure larger, since you still need to have llvm anyway.

@sifmelcara
Copy link
Member

Ok. I think this is ready to get merged but you may need to squash the commits into 3 commits (crystal, shards, mint) :)

@manveru
Copy link
Contributor Author

manveru commented Jun 17, 2018

Done :)

@Mic92 Mic92 merged commit 0eee2f7 into NixOS:master Jun 18, 2018
@manveru manveru deleted the crystal-2.25.0 branch June 18, 2018 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants