Skip to content

Add shellFor multi-package development environments #121

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

Merged
merged 6 commits into from
May 28, 2019
Merged

Conversation

rvl
Copy link
Contributor

@rvl rvl commented May 12, 2019

Adds a shellFor function like in the nixpkgs haskell infra. It lets you work on multi-package projects

Relates to #108.

@rvl rvl added the wip Work In Progress label May 12, 2019
@rvl rvl changed the base branch from master to rvl/without-ghc-environment May 12, 2019 12:10
Copy link
Collaborator

@angerman angerman left a comment

Choose a reason for hiding this comment

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

I'll do a more thourough review later. A lot of moved code.

let
pkgSet = mkStackSnapshotPkgSet {
resolver = "lts-12.21";
pkg-def-extras = [(hackage: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we hsould add a comment why these custom packages are necessary. Eventually someone will be hopelessly lost as to what the hell is going on.

Best to link the relevant stackage issue: commercialhaskell/stackage#4466

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, commented. That issue suggests that we could fix the yaml in stackage.
One thing I'm confused about is that this is a GHC 8.4.4 LTS not 8.6.4.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is indeed a curious case. Does it work without the extras argument? This change seemed to be necessary for 8.6.x only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without the extras argument, it fails with:

Configuring library for transformers-0.5.5.0..
Setup: Encountered missing dependencies:
base (>=2 && <6) && <0

For the ghc-8.4.4 which is used by the build, ghc-pkg list | grep transformers reports 0.5.5.0, and it's the same version in lts-12.21. It looks like there is a hackage cabal revision that disqualifies this package in the cabal solver.

Copy link
Collaborator

Choose a reason for hiding this comment

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

what a mess...

Since https://gitlab.haskell.org/ghc/ghc/issues/16199#note_174847 (and the script was added to CI) I hope that going forward 8.6+, ghc doesn't lie about the shipped packages anymore.

@rvl
Copy link
Contributor Author

rvl commented May 13, 2019

A lot of moved code.

Yeah I had to split out the code for creating the configFiles derivation into a separate function so that it could be used by shellFor. Also to accommodate the shellFor function, I needed to move the package builder into a separate file.

@rvl rvl force-pushed the rvl/without-ghc-environment branch from 5388894 to 72174cb Compare May 13, 2019 00:30
@rvl rvl force-pushed the rvl/shell-for branch 2 times, most recently from e84e72d to 45b0db2 Compare May 14, 2019 00:45
@rvl rvl changed the base branch from rvl/without-ghc-environment to master May 14, 2019 01:09
@michaelpj
Copy link
Collaborator

It would also be very nice to have a one-stop function to get the shell for a whole cabal.project, within which cabal new-build etc. would work. I think in this case I'd have to manually list all the packages myself?

@angerman
Copy link
Collaborator

It would also be very nice to have a one-stop function to get the shell for a whole cabal.project, within which cabal new-build etc. would work. I think in this case I'd have to manually list all the packages myself?

With the cabalProjectToNix ifd, this might be possible? @rvl am I missing some important ingredient that would prevent this from workign if we had the cabalProjectToNix and the supporting infrastructure like providing the hackage index?

@michaelpj
Copy link
Collaborator

I mean, I'm assuming I've already generated my package set with plan2nix, I don't need the IFD necessarily. Perhaps we don't know that the package set came from a cabal.project after we've generated it?

@michaelpj
Copy link
Collaborator

Maybe plan2nix should also record some information about which packages are "home" packages?

@angerman
Copy link
Collaborator

Maybe plan2nix should also record some information about which packages are "home" packages?

Wouldn't that be anything that's addressed relative?

@michaelpj
Copy link
Collaborator

Yes, I guess so? Although I think we also need to include source-package-repository packages, since they're effectively local due to haskell/cabal#5586.

Copy link
Collaborator

@angerman angerman left a comment

Choose a reason for hiding this comment

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

LGTM.

@deepfire
Copy link
Contributor

It looks like https://github.com/input-output-hk/haskell.nix/pull/121/files#diff-ddd10f94c4d895dfc4bc7a0f5f968f14R61 is sometimes tripped up by getting a package, where a component is expected.

Trying to isolate..

@deepfire
Copy link
Contributor

It could be because the stub component in https://github.com/input-output-hk/haskell.nix/pull/121/files#diff-f08a0eb1bcc8b02bf00cda55ff7e231cR16 has packages in its depends list.

@deepfire
Copy link
Contributor

The rabbit hole was quite a bit deeper -- https://github.com/input-output-hk/haskell.nix/tree/shell-for-plus is the running result of the investigation..

@deepfire
Copy link
Contributor

Ok, got it working: IntersectMBO/ouroboros-network#528

@deepfire
Copy link
Contributor

One problem is that non-library dependencies are not quite handled as well, and they need some type-specific love. Like executables can now be added into the shell by listing them as follows:
https://github.com/input-output-hk/ouroboros-network/pull/528/files#diff-5204cbe23b218cae9de0b1aedd4edcd2R16

Tests and benchmarks likely need different treatment, due to dependencies they incur.

, preHaddock
, postHaddock

, shellHook
Copy link
Contributor

Choose a reason for hiding this comment

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

Deleting shellHook from the comp-builder.nix input will break my (already merged) changes for configurable shellHooks per-package: #117

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code has been moved not deleted. You should find shellHook in hspkg-builder.nix.

@rvl rvl force-pushed the rvl/shell-for branch from 6179e9c to a073e47 Compare May 20, 2019 11:56
@rvl
Copy link
Contributor Author

rvl commented May 20, 2019

Just working on the last bit (i.e. only including dependencies of packages, not the packages themselves). Had a few problems with broken tests... #136 #137 #138

@angerman
Copy link
Collaborator

Just working on the last bit (i.e. only including dependencies of packages, not the packages themselves). Had a few problems with broken tests... #136 #137 #138

Absolutely sorry about this. Esp. #136. Sorry for wasting your time!

jbgi added a commit to input-output-hk/cardano-shell that referenced this pull request May 21, 2019
 To allow stack to pick git for user environnement.
 (waiting for input-output-hk/haskell.nix#121 for a better fix.
CodiePP pushed a commit to input-output-hk/cardano-shell that referenced this pull request May 21, 2019
* Update build to use nix-tools.

* updated commands for 'cardano-shell'

Signed-off-by: Alexander Diemand <[email protected]>

* Regeneate nix.

* set project name in nix commands

Signed-off-by: Alexander Diemand <[email protected]>

* Fix nix-shells.

* Update iohk-nix

* Clean-up unused nix files.

* Bump process and transformers for compat with ghc 8.6

* regenerated nix files; updated iohk-nix

Signed-off-by: Alexander Diemand <[email protected]>

* pinned logging to new version

Signed-off-by: Alexander Diemand <[email protected]>

* Update pkgs.nix

* Update pkgs.nix

* Update pkgs.nix

* allow: cabal new-build

Signed-off-by: Alexander Diemand <[email protected]>

* Update pkgs.nix

* bump iohk-nix (and thus haskell.nix)

* regenerate

* Fix katip on Windows

* Set `doExactConfig` on `turtle` on windows as well.

* trying cross-compilation

Signed-off-by: Alexander Diemand <[email protected]>

* rebased; stack2nix applied

Signed-off-by: Alexander Diemand <[email protected]>

* adapted; but still does not build with 'stack build --nix'

Signed-off-by: Alexander Diemand <[email protected]>

* Update iohk-nix. Fix nix-shells.

* Disable nix-shell purity in stack.yaml

 To allow stack to pick git for user environnement.
 (waiting for input-output-hk/haskell.nix#121 for a better fix.
@rvl rvl force-pushed the rvl/shell-for branch 2 times, most recently from c94ff16 to e8d4859 Compare May 25, 2019 07:21
@rvl rvl changed the title wip: Add shellFor multi-package development environments Add shellFor multi-package development environments May 25, 2019
@rvl rvl marked this pull request as ready for review May 25, 2019 07:22
@rvl rvl force-pushed the rvl/shell-for branch from e8d4859 to 7cb4984 Compare May 25, 2019 20:48
@rvl
Copy link
Contributor Author

rvl commented May 25, 2019

This PR is ready for review. It also works for cardano-wallet, which is a multi-package cabal.project (that project also has a stack.yaml and the dependencies are generated with stack-to-nix).

Three things not done yet (the next PRs):

  • Add instructions to the user manual.
  • Generate the hackage index.
  • Michael's idea for a single function which uses the package list from cabal.project.

@rvl rvl removed the wip Work In Progress label May 25, 2019
@rvl rvl force-pushed the rvl/shell-for branch from 7cb4984 to cde9b82 Compare May 25, 2019 23:51
@angerman
Copy link
Collaborator

LGTM. @deepfire do you want to take a look at this as well?

@rvl
Copy link
Contributor Author

rvl commented May 27, 2019

@deepfire I needed to remove the commits from #130 because they made the tests fail. I did fix it so that buildInputs can be used, but I don't understand what the other change was for. Here is your branch rebased: rvl/shell-for...shell-for-plus

rvl added 6 commits May 28, 2019 09:34
I needed to rearrange a few things to write this function.
Now the shell provides the dependencies of the given Haskell packages,
and not the packages themselves.

System libraries and Haskell executables are added to the shell.
@rvl rvl force-pushed the rvl/shell-for branch from cde9b82 to e6b07b4 Compare May 27, 2019 23:35
@rvl rvl merged commit e6b07b4 into master May 28, 2019
@rvl rvl added the enhancement New feature or request label May 28, 2019
@rvl rvl deleted the rvl/shell-for branch May 28, 2019 01:07
HirotoShioi pushed a commit to input-output-hk/cardano-shell that referenced this pull request Jun 4, 2019
 To allow stack to pick git for user environnement.
 (waiting for input-output-hk/haskell.nix#121 for a better fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants