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
Conversation
@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. |
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 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. |
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" { |
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.
@cdepillabout Do you think this makeWrapper solution is fine? Or is there a better way to do this?
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.
I think this is basically on the right track.
I'd propose a few small nitpicks:
- Technically this shouldn't be
executableHaskellDepends
, but something likeexecutableToolDepends
or maybeexecutableSystemDepends
(depending on whether it should go intonativeBuildInputs
or justbuildInputs
). - There are helper functions to build PATHs. Grep for
makeBinPath
in the top-levellib/
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.
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.
- Okay makes sense. I think executableSystemDepends is the semantically the best.
- Yeah I wondered about that. Nice, will have a look.
- 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.
- (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?
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.
TIL: Setting PATH in preBuild does not work, but it works in preConfigure or postUnpack.
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.
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.
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.
I take back the $out/bin part. Had to create that myself.
33a27ef
to
2a60d72
Compare
rib 0.10 published to Hackage: https://hackage.haskell.org/package/rib-0.10.0.0 |
Okay, I think there is no big hurry here. So I‘ll wait until it‘s in hackage-packages.nix and rebase. |
neuron 0.4 published to Hackage: https://hackage.haskell.org/package/neuron-0.4.0.0 |
Now just waiting for neuron 0.4 to hit haskell-packages.nix and then we are good to go. |
Okay, thanks @srid so much for your very quick cooperation. From my perspective this is ready to be merged. |
@maralorn This looks good to me! Thanks for working through this. |
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.
I will contact upstream regarding all three points.
Things to be done
Things done
sandbox
innix.conf
on non-NixOS linux)./result/bin/
)