ponyc: 2016-07-26 -> 0.5.1 by dipinhora · Pull Request #19457 · NixOS/nixpkgs

dipinhora · 2016-10-11T18:35:45Z

Motivation for this change

Update to latest ponyc release

add more package options for compiling
enable for darwin
enable LTO by default on linux
disable LTO by default on darwin until Clang link time optimization for darwin #19312 is merged

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- OS X
- Linux
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.

mention-bot · 2016-10-11T18:35:55Z

@dipinhora, thanks for your PR! By analyzing the history of the files in this pull request, we identified @wizeman, @doublec and @dezgeg to be potential reviewers.

LnL7 · 2016-10-11T21:12:56Z

pkgs/development/compilers/ponyc/default.nix

this looks a little fragile

How about adding coreutils to testPhase PATH or add it to buildInputs?

Adding coreutils to PATH or buildInputs doesn't help because _test.pony explicitly look for /bin/echo and /bin/cat.

I've updated the search string for the substituteInPlace to be rooted so that it will only match a /bin at the beginning of a path and not in the middle of the path.

Let me know if this still is too brittle or if you guys have any suggestions on how to handle this in a not so brittle manner.

Forgot to mention, the source of the file being patched is at: https://github.com/ponylang/ponyc/blob/master/packages/process/_test.pony with references to /bin on lines 27, 32, 55, 62, 82, 88, 184, 190, 211, 216, 244 and 249.

LnL7 · 2016-10-11T21:13:55Z

pkgs/development/compilers/ponyc/default.nix

!stdenv.isDarwin

Thanks. Fixed.

dipinhora · 2016-10-11T23:26:46Z

@LnL7 I just noticed you added the 6.topic: python to this PR. I think you might have made a mistake because ponyc is not related to python.

kamilchm · 2016-10-14T07:36:22Z

pkgs/development/compilers/ponyc/default.nix

Isn't that something that should go to the upstream?

It's only used for testing on CI and CI has libraries in the standard unix locations. We need it because in nix the libraries (pcre2/crypto/ssl) are not in standard unix locations.

I'll see if I can get upstream to accept the change but this would still be needed for the current release. I'll update to a newer release and remove this logic from the nix expression at that time.

👍 I was just wondering if it's something that can be useful not only in nix.

kamilchm · 2016-10-14T07:45:06Z

pkgs/development/compilers/ponyc/default.nix

rev = version;?

kamilchm · 2016-10-14T07:45:49Z

pkgs/development/compilers/ponyc/default.nix

name = "ponyc-${version}"; version = "0.5.0";

?

dipinhora · 2016-10-16T03:51:22Z

Updated to the recently released 0.5.1.

dipinhora · 2016-10-20T00:34:10Z

Does anything else need to be changed before this can be merged?

kamilchm · 2016-10-20T15:24:20Z

0.6.0 is out.
Anyone from maintainers is willing to merge this? @LnL7 ?

NeQuissimus · 2016-10-20T15:38:37Z

I was going to merge this but when I check out this PR, it does not build:

Verifying
Writing ./stdlib.o
Linking ./stdlib
make: *** [Makefile:620: test-ci] Illegal instruction
builder for ‘/nix/store/r8y6sg3wg4bm1mwjq4gnk6z1n6hdy9qw-ponyc-0.5.1.drv’ failed with exit code 2
error: build of ‘/nix/store/r8y6sg3wg4bm1mwjq4gnk6z1n6hdy9qw-ponyc-0.5.1.drv’ failed
exit 100

dipinhora · 2016-10-20T16:03:33Z

There is a known issue with Virtualbox and Pony (see https://github.com/ponylang/ponyc/blob/master/README.md#virtualbox) that causes an "Illegal instruction" error. Are you running in Virtualbox?

There's also an edge case around AVX2/AVX512 and llvm 3.8 that causes an "Illegal instruction" error (see ponylang/ponyc#1176). Are you on a non-Xeon Skylake CPU?

Also, you didn't mention, are you building on Linux? Mac OS? NixOS? I only tested on Mac OS 10.10.5 and Ubuntu on AWS.

NeQuissimus · 2016-10-20T16:32:48Z

I am directly on hardware, cat /proc/cpuinfo has this:

vendor_id       : GenuineIntel
cpu family      : 6
model           : 78
model name      : Intel(R) Core(TM) m3-6Y30 CPU @ 0.90GHz

No idea if this is a Skylake :D
I am running NixOS.

dipinhora · 2016-10-20T17:14:48Z

It is a Skylake processor (see https://en.wikipedia.org/wiki/List_of_Intel_Core_M_microprocessors#.22Skylake-Y.22_.28SoC.2C_dual-core.2C_14_nm.29).

You're almost definitely running into ponylang/ponyc#1176.

NeQuissimus · 2016-10-20T17:16:52Z

OK, I will merge this. Note that there seems to be a newer version already.
Could we use a different LLVM as suggested in the linked ponyc issue? They seem to suggest that might fix the issue.

dipinhora mentioned this pull request Oct 11, 2016

pony-stable: init at 2016-10-10 #19462

Merged

7 tasks

LnL7 reviewed Oct 11, 2016

View reviewed changes

LnL7 added 6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (update) This PR updates a package to a newer version labels Oct 11, 2016

dipinhora force-pushed the ponyc-update branch from ee021f6 to da370a5 Compare October 11, 2016 22:38

LnL7 removed the 6.topic: python Python is a high-level, general-purpose programming language. label Oct 11, 2016

kamilchm reviewed Oct 14, 2016

View reviewed changes

dipinhora force-pushed the ponyc-update branch from da370a5 to 65364e9 Compare October 14, 2016 15:39

kamilchm approved these changes Oct 14, 2016

View reviewed changes

ponyc: 2016-07-26 -> 0.5.1

66113fb

dipinhora force-pushed the ponyc-update branch from 65364e9 to 66113fb Compare October 16, 2016 03:50

dipinhora changed the title ~~ponyc: 2016-07-26 -> 0.5.0~~ ponyc: 2016-07-26 -> 0.5.1 Oct 16, 2016

dipinhora mentioned this pull request Oct 17, 2016

Clang link time optimization for darwin #19312

Closed

7 tasks

NeQuissimus merged commit d2371e6 into NixOS:master Oct 20, 2016

dipinhora mentioned this pull request Oct 20, 2016

ponyc: 0.5.1 -> 0.6.0 #19727

Merged

7 tasks

Uh oh!

Conversation

dipinhora commented Oct 11, 2016

Motivation for this change

Things done

Uh oh!

mention-bot commented Oct 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dipinhora commented Oct 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dipinhora commented Oct 16, 2016

Uh oh!

dipinhora commented Oct 20, 2016

Uh oh!

kamilchm commented Oct 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NeQuissimus commented Oct 20, 2016

Uh oh!

dipinhora commented Oct 20, 2016

Uh oh!

NeQuissimus commented Oct 20, 2016

Uh oh!

dipinhora commented Oct 20, 2016

Uh oh!

NeQuissimus commented Oct 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kamilchm commented Oct 20, 2016 •

edited

Loading