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

[haskell-updates] hackagePackages.neuron: Fix build #87206

Merged
merged 1 commit into from May 10, 2020

Conversation

@maralorn
Copy link
Contributor

maralorn commented May 7, 2020

WARNING: Do not merge, right now this uses a tarball from my personal server, not upstream, as a proof of concept.

Motivation for this change

I am trying to get the nice neuron app to build.

It is building, but I had to employ weird workarounds.

  1. The hackage package is not containing everything needed to actually compile, so pulling the source again from github. This is probably something we should fix on upstream in future releases.
  2. The program includes a nix-generate shell script at compiletime. I don‘t know how to do this nicely in nixpkgs without relying on IFD. Would be very open to suggestions.
  3. Right now the git version is not set during the build process. That is not a problem, but generated html outputs (UNKNOWN) were normally a git commit id would be.

I will contact upstream regarding all three points.

Things to be done
  • Add extra-sources on upstream
  • Change bash-script to not use IFD on upstream
  • New upstream release
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.
@maralorn maralorn changed the title hackagePackages.neuron: Fix build [haskell-updates] hackagePackages.neuron: Fix build May 7, 2020
@maralorn maralorn mentioned this pull request May 7, 2020
@maralorn
Copy link
Contributor Author

maralorn commented May 7, 2020

@cdepillabout Since you where so helpful the other day. Do you have any idea what we can do about issue 2? I understand the urge of the package authors to include the nix store paths in there bash-script, but maintaining that script is configuration-common.nix is not a nice solution. Not sure what the best solution here is.
Kinda sad, that the authors embracing nix so much actually makes problem with including the package into nixpkgs.^^

@maralorn maralorn force-pushed the maralorn:fix-neuron branch from d293352 to c554518 May 7, 2020
@cdepillabout
Copy link
Member

cdepillabout commented May 8, 2020

The hackage package is not containing everything needed to actually compile, so pulling the source again from github.

In general, I think packages are considered "broken" if they don't contain all the files that are required to build when uploaded to Hackage.

The easiest thing to do here is to open an issue on the upstream repo (or even better, send a PR) that adds all required files to the extra-source-files section in the cabal file.

I think @srid is a big Nix user and Haskeller, so I imagine he will be sympathetic to fixing this.

After this is fixed, let's figure out what to do about 2 and 3. If @srid doesn't respond in a week or so, let's move forward with this PR and figure out how to fix it here in nixpkgs. In either case, please feel free to ping me here.

@maralorn maralorn force-pushed the maralorn:fix-neuron branch from c554518 to 5070d33 May 8, 2020
@peti peti force-pushed the NixOS:haskell-updates branch from d7b34d6 to f35d2a8 May 8, 2020
@maralorn maralorn force-pushed the maralorn:fix-neuron branch from 5070d33 to eb485ae May 8, 2020
@maralorn
Copy link
Contributor Author

maralorn commented May 8, 2020

This PR is basically working now. Of course before we merge it we need a new neuron and rib release with my PR srid/neuron#164 merged.

Then I can remove all the manual overrides I have done here.

};
# neuron expects the neuron-search script to be in PATH at built-time.
executableHaskellDepends = drv.executableHaskellDepends or [ ] ++ [
(pkgs.runCommand "neuron-search" {

This comment has been minimized.

Copy link
@maralorn

maralorn May 8, 2020

Author Contributor

@cdepillabout Do you think this makeWrapper solution is fine? Or is there a better way to do this?

This comment has been minimized.

Copy link
@cdepillabout

cdepillabout May 9, 2020

Member

I think this is basically on the right track.

I'd propose a few small nitpicks:

  • Technically this shouldn't be executableHaskellDepends, but something like executableToolDepends or maybe executableSystemDepends (depending on whether it should go into nativeBuildInputs or just buildInputs).
  • There are helper functions to build PATHs. Grep for makeBinPath in the top-level lib/ directory in nixpkgs.
  • I'd pull out the whole runCommand part into a let-statement somewhere and give it a comment.

I guess it might be possible to do the whole wrapping step for neuron-search in like the postUnpack phase of the neuron build itself, instead of creating a separate derivation for it.

This comment has been minimized.

Copy link
@maralorn

maralorn May 9, 2020

Author Contributor
  1. Okay makes sense. I think executableSystemDepends is the semantically the best.
  2. Yeah I wondered about that. Nice, will have a look.
  3. How far out? The runCommand needs the source. That‘s why it‘s in the derivation. If I don‘t find a solution for 4 I will just bump it as far as the scope permits.
  4. (Re: postUnpack) Yes, that would be much better. But I just couldn‘t get that to build. It‘s no problem to write a wrapper to put the script into $out/bin but neuron then needs to have that dir in the $PATH at compile time. Can you tell my how to do that?

This comment has been minimized.

Copy link
@maralorn

maralorn May 9, 2020

Author Contributor

TIL: Setting PATH in preBuild does not work, but it works in preConfigure or postUnpack.

This comment has been minimized.

Copy link
@maralorn

maralorn May 9, 2020

Author Contributor

I decided to use preConfigure instead of postUnpack, because there PWD is already the source and $out/bin is already created.
I think I have addressed all of this.

This comment has been minimized.

Copy link
@maralorn

maralorn May 10, 2020

Author Contributor

I take back the $out/bin part. Had to create that myself.

@maralorn maralorn force-pushed the maralorn:fix-neuron branch from eb485ae to 590e1fd May 8, 2020
@peti peti force-pushed the NixOS:haskell-updates branch 3 times, most recently from 33a27ef to 2a60d72 May 8, 2020
@srid
Copy link

srid commented May 8, 2020

rib 0.10 published to Hackage: https://hackage.haskell.org/package/rib-0.10.0.0

@maralorn
Copy link
Contributor Author

maralorn commented May 8, 2020

Okay, I think there is no big hurry here. So I‘ll wait until it‘s in hackage-packages.nix and rebase.

@srid
Copy link

srid commented May 9, 2020

neuron 0.4 published to Hackage: https://hackage.haskell.org/package/neuron-0.4.0.0

@maralorn maralorn force-pushed the maralorn:fix-neuron branch from 590e1fd to 0a95132 May 9, 2020
@maralorn
Copy link
Contributor Author

maralorn commented May 9, 2020

Now just waiting for neuron 0.4 to hit haskell-packages.nix and then we are good to go.

@maralorn maralorn force-pushed the maralorn:fix-neuron branch from 0a95132 to 74c7899 May 10, 2020
@maralorn maralorn force-pushed the maralorn:fix-neuron branch from 74c7899 to 9b2bc7f May 10, 2020
@maralorn
Copy link
Contributor Author

maralorn commented May 10, 2020

Okay, thanks @srid so much for your very quick cooperation. From my perspective this is ready to be merged.

@maralorn maralorn marked this pull request as ready for review May 10, 2020
@cdepillabout
Copy link
Member

cdepillabout commented May 10, 2020

@maralorn This looks good to me! Thanks for working through this.

@cdepillabout cdepillabout merged commit 7b69f18 into NixOS:haskell-updates May 10, 2020
13 checks passed
13 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="9b2bc7f"; rev="9b2bc7f9a58aa80b8584beafa34e4065908d0eee"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="9b2bc7f"; rev="9b2bc7f9a58aa80b8584beafa34e4065908d0eee"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="9b2bc7f"; rev="9b2bc7f9a58aa80b8584beafa34e4065908d0eee"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="9b2bc7f"; rev="9b2bc7f9a58aa80b8584beafa34e4065908d0eee"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="9b2bc7f"; rev="9b2bc7f9a58aa80b8584beafa34e4065908d0eee"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="9b2bc7f"; rev="9b2bc7f9a58aa80b8584beafa34e4065908d0eee"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="9b2bc7f"; rev="9b2bc7f9a58aa80b8584beafa34e4065908d0eee"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@maralorn maralorn deleted the maralorn:fix-neuron branch May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.