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

ber_metaocaml: (re)init at 104 #32494

Merged
merged 2 commits into from Dec 10, 2017
Merged

ber_metaocaml: (re)init at 104 #32494

merged 2 commits into from Dec 10, 2017

Conversation

mpickering
Copy link
Contributor

Motivation for this change

The old derivation was broken as the src pointed to a mutable url. It was also necessary to make some more packaging changes to get it to compile to follow with changes to the normal ocaml build process.

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
    • 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.

@mpickering
Copy link
Contributor Author

Possible reviewers? @vbgl @aske @Zimmi48 ?

I am not an ocaml expert but this appears to work for me.

url = "http://okmij.org/ftp/ML/ber-metaocaml.tar.gz";
sha256 = "1kq1if25c1wvcdiy4g46xk05dkc1am2gc4qvmg4x19wvvaz09gzf";
url = "http://okmij.org/ftp/ML/ber-metaocaml-104.tar.gz";
sha256 = "1gmwlxairxqcmqa2r6kbf8b4dxc7pfhfbh48g1s14d3z20rj8nib";
};

# Needed to avoid a SIGBUS on the final executable on mips
NIX_CFLAGS_COMPILE = if stdenv.isMips then "-fPIC" else "";

patches = optionals stdenv.isDarwin [ ./gnused-on-osx-fix.patch ];
Copy link
Contributor

Choose a reason for hiding this comment

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

This patch does not apply. Is it needed?

"-x11include" x11inc ];

dontStrip = true;
buildInputs = [ncurses] ++ optionals useX11 [ libX11 xproto ];
installFlags = "-i";
installTargets = "install"; # + optionalString useNativeCompilers " installopt";
prePatch = ''
CAT=$(type -tp cat)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this CAT variable used anywhere? If not, you may drop the full prePatch attribute.

@mpickering
Copy link
Contributor Author

mpickering commented Dec 9, 2017

@vbgl as a general point if I have some more time in the future would it be better to modify generic.nix to also allow building ber_metaocaml? There is quite a lot of duplicated logic as it is essentially an ocaml build with some additional patches.

@mpickering
Copy link
Contributor Author

Thanks for the sharp eye @vbgl , I removed the two redundant parts of the expression.

@vbgl
Copy link
Contributor

vbgl commented Dec 9, 2017

Thanks. Your suggestion to refactor generic.nix makes sense.

@vbgl vbgl merged commit d98d944 into NixOS:master Dec 10, 2017
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

3 participants