timbertson / opam2nix

Generate nix expressions from opam packages
MIT License
93 stars 28 forks source link

How to `patchShebangs` while building a package (`digestif`) #60

Closed jyssh closed 3 years ago

jyssh commented 3 years ago

I am trying to to build dream with opam2nix.

digestif.opam, one of dream's dependencies, needs to run an ./install/install.ml file as a build step:

# digestif.opam

build: [
  [ "dune" "build" "-p" name "-j" jobs ]
  [ "./install/install.ml" ]
  [ "dune" "runtest" "-p" name "-j" jobs ] {with-test}
]

The install.ml uses #!/usr/bin/env ocaml. As /usr/bin/env is not available during the nix-build process, the build process fails (I found this out the hard way!).

From what I understood, nix's way of tackling this is to use patchShebangs to patch such invocations.

However, I cannot figure out where I would do this in case of opam2nix's case. Can you help me figure it out?

You could try to build digestif in order to reproduce this issue.

jyssh commented 3 years ago

Ok. After a bit of digging around, I figured it out.

packages/dream/default.nix:

{ stdenv, ocaml, opam2nix }:

let
  selection = opam2nix.build {
    inherit ocaml;
    selection = ./opam-selection.nix;
    override = { }: {
      digestif = super: super.overrideAttrs (attrs: {
        preBuildPhases = ["preBuildPhase"];
        preBuildPhase = if stdenv.isLinux then 
          ''
          patchShebangs ./install/install.ml
          ''
          else "";
      });
    };
  };
in
selection.dream

Now, patchShebangs replaces #!/usr/bin/env ocaml with #!/nix/store/<hash>-ocaml-<version>/bin/ocaml in digestif/install/install.ml.

I had to add the stdenv.isLinux conditional to avoid patching the shebang on macos, because:

  1. macOS either doesn't recognize or allow the patched shebang to work. It erroneously executes the install.ml as a bash script instead. I suspect the reason is as described here: "This works in linux systems, but does not work on BSD flavored systems like OS X since they do not allow another shell script to play a role as an interpreter." or here: "It's concerned with the fact that scripts cannot be used as an interpreter of other scripts on BSD". True enough, /nix/store/<hash>-ocaml-<version>/bin/ocaml is a shell script in disguise (wrapper?). head -1 /nix/store/<hash>-ocaml-<version>/bin/ocaml gives: #!/nix/store/<hash>-ocaml-<version>/bin/ocamlrun.
  2. /usr/bin/env remains available in macOS during nix-build anyway. So the package builds in macOS without patching the shebang.
timbertson commented 3 years ago

Ahh yep, it sucks that this is necessary - I wish nix would special-case /usr/bin/env since it's so ubiquitous and harmless.

I'd accept an override in https://github.com/timbertson/opam2nix/blob/8d787c5cc26b29ff3874f73a2f6b64f21764859f/nix/overrides/default.nix if you want to make a PR.

Given that this seems like a pretty common issue, I'd even consider just blanketly patch-shebang-ing all executable files before running build commands. I think that happens on the output in fixupPhase anyway, so there's precedent.

jyssh commented 3 years ago

I'd accept an override in https://github.com/timbertson/opam2nix/blob/8d787c5cc26b29ff3874f73a2f6b64f21764859f/nix/overrides/default.nix if you want to make a PR.

Sure. I will try to get in a PR are during weekend.

On a side note, are you planning to upstream opam2nix to nixpkgs? Thinking you had gone AWOL (sorry!), only yesterday I had posted a related question on Nix Discourse: https://discourse.nixos.org/t/upstreaming-handwritten-v-s-2nix-generated-derivation-ocaml-opam2nix/13746. Would you be willing to chime in there?

We even had a discussion going on Reason Discord (https://discord.com/channels/235176658175262720/235176658175262720/856499048235794432), which eventually petered out without conclusion.

timbertson commented 3 years ago

Haha no offence, I come and go in terms of how much attention I can (and choose) to give to open source projects, and I juggle a few so there's often quiet periods. I'll respond over in discourse.