-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
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
mlton: refactor and adds mlton20180207Binary mlton20180207 mltonHEAD #44386
Conversation
|
||
usr_prefix = if stdenv.isDarwin then "usr/local" else "usr"; | ||
|
||
dynamic_linker = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use stdenv.cc.bintools.dynamicLinker
for this purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I'll change that.
@GrahamcOfBorg build mlton mltonHEAD |
Failure on x86_64-linux (full log) Attempted: mlton, mltonHEAD Partial log (click to expand)
|
No attempt on aarch64-linux (full log) The following builds were skipped because they don't evaluate on aarch64-linux: mlton, mltonHEAD Partial log (click to expand)
|
Timed out, unknown build status on x86_64-darwin (full log) Attempted: mlton, mltonHEAD Partial log (click to expand)
|
Ok, I see Looks like the x86_64-darwin build succeeded, but ran out of time when it was running tests. Not super surprising, building mlton is pretty intensive. |
Are you still working on fixing this? The /usr/bin/env bash lines should be able to be fixed by the same |
Yeah I'm going to update this. |
20130715.nix is exactly the same as the old default.nix other than pulling the meta data out into its own file.
This works the same as the binary package in the old derivation, by patching the release binary. The difference is that this derivation is also exposed at the top level.
Updates from-git-source.nix to use the one-liner to fix up the bash shebang lines.
This commit adds mltonHEAD. Summary of all the changes in this branch: * mlton20130715 - for this one I just copy and pasted the old default.nix - I have tested some projects that compile with this version and don't compile with the newer version, so I think it makes sense to keep the old version * mlton20180207Binary - This is used to build the source derivations * mlton20180207 - latest release * mltonHEAD - latest commit to the master branch of the github repo at the time of creating this derivation
I remade the PR. Fixed merge conflicts (without reverting the other changes). Added the one-liner to fix the source files in the other derivation, which enabled it to build successfully (with tests) for me on a box with nix.useSandbox enabled. If it times out while running the tests again, I could push up a PR disabling the tests. |
Thanks! |
This was merged, but @srdqty can you actually list which projects don't compile with the new version but compiled with the old? |
Sure. It was a personal project: https://github.com/srdqty/sexpr-pp Seems that you cannot redefine the
|
So, it turns out that while that's supported by SMLNJ, it is expressly forbidden by the spec. http://mlton.org/SMLNJDeviations What I'd like to eventually do is be able to get rid of 2013, so I was wondering about any deviations between the versions. |
Motivation for this change
Newer versions of the MLton compiler in nixpkgs.
Things done
Adds the following derivations:
mlton20130715
compile with the newer version, so I think it makes sense to keep
the old version
mlton20180207Binary
mlton20180207
mltonHEAD
creating this derivation
Everything seemed to build fine using Nix 2.0 on NixOS. I haven't tried building it on macOS.
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)