crystal-lang 0.24.1 #22091

matiasgarciaisaia · 2017-12-24T06:28:25Z

Have you followed the guidelines for contributing?
Have you checked that there aren't other open pull requests for the same formula update/change?
Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

We've just released Crystal 0.24.1, so this PR updates Homebrew accordingly.

0.24.0 was a pre-release, so 0.24.1 has to be built with 0.23.1 - as the Formula says.

Thanks for the project!

ilovezfs · 2017-12-24T06:41:47Z

Formula/crystal-lang.rb

@@ -1,15 +1,15 @@
 class CrystalLang < Formula
  desc "Fast and statically typed, compiled language with Ruby-like syntax"
  homepage "https://crystal-lang.org/"
-  revision 3
+  revision 2


Why is this 2? If it's a new version, the revision line should be removed, not decremented by 1.

My bad - I thought it was matching the other platforms build numbers, but it was just a coincidence. I've just dropped this 👍

.

ilovezfs · 2017-12-24T08:26:48Z

@matiasgarciaisaia the build fails.

==> brew install --verbose --build-bottle crystal-lang
==> FAILED
/usr/bin/sandbox-exec -f /tmp/homebrew20171224-97438-1lfwch7.sb nice /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/build.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/crystal-lang.rb --build-bottle --verbose
==> Downloading https://github.com/crystal-lang/crystal/archive/0.24.1.tar.gz
Already downloaded: /Users/brew/Library/Caches/Homebrew/crystal-lang-0.24.1.tar.gz
==> Verifying crystal-lang-0.24.1.tar.gz checksum
tar xzf /Users/brew/Library/Caches/Homebrew/crystal-lang-0.24.1.tar.gz
==> Downloading https://github.com/crystal-lang/crystal/releases/download/0.23.1/crystal-0.23.1-3-darwin-x86_64.tar.gz
Already downloaded: /Users/brew/Library/Caches/Homebrew/crystal-lang--boot-0.23.1.tar.gz
==> Verifying crystal-lang--boot-0.23.1.tar.gz checksum
tar xzf /Users/brew/Library/Caches/Homebrew/crystal-lang--boot-0.23.1.tar.gz
==> make crystal release=true
�[33mUsing /usr/local/opt/llvm@4/bin/llvm-config [version=4.0.1]�[0m
clang++ -c  -o src/llvm/ext/llvm_ext.o src/llvm/ext/llvm_ext.cc `/usr/local/opt/llvm@4/bin/llvm-config --cxxflags`
clang -fPIC    -c -o src/ext/sigfault.o src/ext/sigfault.c
ar -rcs src/ext/libcrystal.a src/ext/sigfault.o
./bin/crystal build --release  -o .build/crystal src/compiler/crystal.cr -D without_openssl -D without_zlib
[warn] kq_init: detected broken kqueue; not using.: Undefined error: 0
Assertion failed: (cast<DISubprogram>(Scope)->describes(MF->getFunction())), function getOrCreateRegularScope, file /var/cache/omnibus/src/llvm/llvm-3.8.1.src/lib/CodeGen/LexicalScopes.cpp, line 160.
/private/tmp/crystal-lang-20171224-97440-lbgxr7/crystal-0.24.1/boot/bin/crystal: line 102: 97540 Abort trap: 6           "$INSTALL_DIR/embedded/bin/crystal" "$@"
make: *** [.build/crystal] Error 134

ilovezfs · 2017-12-24T15:04:58Z

@matiasgarciaisaia so I'm not surprised this fails because your CI is using the Homebrew formula, not your bootstrap binary. Bootstrapping 0.24.1 with the Homebrew formula for 0.23.1 does work. I'd suggest you modify your CI procedure to test with your bootstrap binary so this sort of thing is caught before, not after, release.

RX14 · 2017-12-24T15:44:06Z

@ilovezfs this is a known issue with older LLVM versions that the bootstrap build is using, adding --no-debug to the crystal compile options should be a valid workaround.

ilovezfs · 2017-12-24T15:45:45Z

What does that do?

Also, I guess we could do a 2 stage compile if that works.

ilovezfs · 2017-12-24T15:46:46Z

@RX14 And why is this just now an issue but not previously an issue? If this issue is "known," is there an upstream ticket for it?

RX14 · 2017-12-24T16:05:04Z

@ilovezfs crystal-lang/crystal#4719. --no-debug causes crystal not to emit DWARF debugging info for the compiled binary. LLVM has some bugs fixed in LLVM 4 which cause our valid debug info to be mishandled. Since OSX can't have debug info internally to the macho binary anyway, I don't think adding this option should be a problem.

matiasgarciaisaia · 2017-12-24T20:15:45Z

@ilovezfs The suggestion about changing the CI to bootstrap Crystal is a good one, I think. We're probably discussing it and probably changing it.

For the time being, I'm updating the formula so it builds Crystal with --no-debug, as @RX14 mentioned. Since we haven't added a make flag for that, I'm rewriting the build command - we should probably roll it back when we update the makefile or fix the bug for good.

That's a kind-of-random bug we've seen, and seems to be related to the LLVM version we're using for building the bootstrap compiler.

ilovezfs · 2017-12-24T20:34:53Z

By the way, what is the status of llvm 5 support?

RX14 · 2017-12-24T21:01:26Z

@ilovezfs it's fully supported.

RX14 · 2017-12-24T21:07:40Z

Formula/crystal-lang.rb

    if build.with? "release"
-      system "make", "crystal", "release=true"
+      system "bin/crystal", "build", "--release", "--no-debug", "-o", ".build/crystal", "src/compiler/crystal.cr"


Please let's just merge the release and and non-release branches of this if. Just add --release nor not. We should absolutely not be missing -D without_openssl and -D without_zlib on release builds.

FYI no one is using --without-release according to our analytics so perhaps we should just remove the option. The same is true for --without-shards.

ilovezfs · 2017-12-24T21:09:43Z

Then we need to switch to depends_on "llvm"

matiasgarciaisaia · 2017-12-26T15:14:57Z

I've updated the formula to include the without_openssl & without_zlib flags on release builds, as @RX14 noted.

Regarding LLVM5, I'd prefer to release 0.24.1 using LLVM4, and upgrade it in a future release, in which our CI takes tests LLVM5 too.

Homebrew is the only package manager in which we haven't successfully released 0.24.1 yet, so I'd prefer to get it done and upgrade LLVM in a future release.

ilovezfs · 2017-12-26T15:21:38Z

If it can build with llvm 5, we don't want to use the versioned formula.

RX14 · 2017-12-26T15:45:35Z

LLVM 5 is fine, it's been battle tested for months on the arch linux crystal package.

ilovezfs · 2017-12-26T16:02:21Z

Formula/crystal-lang.rb

+    (buildpath/".build").mkpath
+
+    command = ["bin/crystal", "build", "-D", "without_openssl", "-D", "without_zlib", "-o", ".build/crystal", "src/compiler/crystal.cr"]
+    command.concat ["--release", "--no-debug"] if build.with? "release"


Since --no-debug was needed for a successful build, doesn't this imply that the --without-release option will be broken?

@ilovezfs no, --no-debug is only an issue on release builds. LLVM only has this bug when running optimizations.

ilovezfs

depends_on "llvm@4" should be replaced with depends_on "llvm"

matiasgarciaisaia · 2017-12-26T16:51:19Z

I've ran a few tests on LLVM5 and seems to work fine - so we're doing this 👍

ilovezfs · 2017-12-26T16:56:50Z

Living dangerously! Now only ponyc will be on The Wall of Shame.

bash-4.4$ grep -r 'depends_on "llvm@' .
./Formula/crystal-lang.rb:  depends_on "llvm@4"
./Formula/ponyc.rb:  depends_on "llvm@3.9"
bash-4.4$

ilovezfs · 2017-12-26T17:08:34Z

Thanks for the upgrade @matiasgarciaisaia! 🔮 🚢 🎉

ilovezfs previously requested changes Dec 24, 2017

View reviewed changes

ilovezfs changed the title ~~Crystal v0.24.1~~ crystal-lang 0.24.1 Dec 24, 2017

matiasgarciaisaia force-pushed the patch-1 branch from 7058891 to 549d54f Compare December 24, 2017 06:53

ilovezfs added the build failure CI fails while building the software label Dec 24, 2017

ilovezfs added the upstream issue An upstream issue report is needed label Dec 24, 2017

matiasgarciaisaia force-pushed the patch-1 branch from 549d54f to 3379921 Compare December 24, 2017 20:23

RX14 suggested changes Dec 24, 2017

View reviewed changes

matiasgarciaisaia force-pushed the patch-1 branch from 3379921 to 0b2e907 Compare December 26, 2017 15:11

RX14 approved these changes Dec 26, 2017

View reviewed changes

ilovezfs reviewed Dec 26, 2017

View reviewed changes

ilovezfs removed build failure CI fails while building the software upstream issue An upstream issue report is needed labels Dec 26, 2017

ilovezfs suggested changes Dec 26, 2017

View reviewed changes

crystal-lang 0.24.1

44763f1

matiasgarciaisaia force-pushed the patch-1 branch from 0b2e907 to 44763f1 Compare December 26, 2017 16:49

ilovezfs approved these changes Dec 26, 2017

View reviewed changes

ilovezfs added the ready to merge PR can be merged once CI is green label Dec 26, 2017

ilovezfs closed this in 729cfc2 Dec 26, 2017

Homebrew locked and limited conversation to collaborators May 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crystal-lang 0.24.1 #22091

crystal-lang 0.24.1 #22091

matiasgarciaisaia commented Dec 24, 2017

ilovezfs Dec 24, 2017

matiasgarciaisaia Dec 24, 2017

ilovezfs commented Dec 24, 2017

ilovezfs commented Dec 24, 2017

RX14 commented Dec 24, 2017 •

edited

Loading

ilovezfs commented Dec 24, 2017

ilovezfs commented Dec 24, 2017

RX14 commented Dec 24, 2017 •

edited

Loading

matiasgarciaisaia commented Dec 24, 2017

ilovezfs commented Dec 24, 2017

RX14 commented Dec 24, 2017

RX14 Dec 24, 2017 •

edited

Loading

ilovezfs Dec 25, 2017

ilovezfs commented Dec 24, 2017

matiasgarciaisaia commented Dec 26, 2017

ilovezfs commented Dec 26, 2017

RX14 commented Dec 26, 2017

ilovezfs Dec 26, 2017

RX14 Dec 26, 2017

ilovezfs left a comment

matiasgarciaisaia commented Dec 26, 2017

ilovezfs commented Dec 26, 2017

ilovezfs commented Dec 26, 2017

crystal-lang 0.24.1 #22091

crystal-lang 0.24.1 #22091

Conversation

matiasgarciaisaia commented Dec 24, 2017

ilovezfs Dec 24, 2017

Choose a reason for hiding this comment

matiasgarciaisaia Dec 24, 2017

Choose a reason for hiding this comment

ilovezfs commented Dec 24, 2017

ilovezfs commented Dec 24, 2017

RX14 commented Dec 24, 2017 • edited Loading

ilovezfs commented Dec 24, 2017

ilovezfs commented Dec 24, 2017

RX14 commented Dec 24, 2017 • edited Loading

matiasgarciaisaia commented Dec 24, 2017

ilovezfs commented Dec 24, 2017

RX14 commented Dec 24, 2017

RX14 Dec 24, 2017 • edited Loading

Choose a reason for hiding this comment

ilovezfs Dec 25, 2017

Choose a reason for hiding this comment

ilovezfs commented Dec 24, 2017

matiasgarciaisaia commented Dec 26, 2017

ilovezfs commented Dec 26, 2017

RX14 commented Dec 26, 2017

ilovezfs Dec 26, 2017

Choose a reason for hiding this comment

RX14 Dec 26, 2017

Choose a reason for hiding this comment

ilovezfs left a comment

Choose a reason for hiding this comment

matiasgarciaisaia commented Dec 26, 2017

ilovezfs commented Dec 26, 2017

ilovezfs commented Dec 26, 2017

RX14 commented Dec 24, 2017 •

edited

Loading

RX14 commented Dec 24, 2017 •

edited

Loading

RX14 Dec 24, 2017 •

edited

Loading