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

polysemy-plugin: Make plugin available to doctest #71164

Closed
sevanspowell opened this issue Oct 15, 2019 · 9 comments
Closed

polysemy-plugin: Make plugin available to doctest #71164

sevanspowell opened this issue Oct 15, 2019 · 9 comments

Comments

@sevanspowell
Copy link
Contributor

sevanspowell commented Oct 15, 2019

Describe the bug
polysemy-plugin is broken on the haskell-updates branch of nixpkgs. I was working on fixing this, but ran into a problem I don't know how to overcome.

The package builds fine and the tests run, but one of the doctests do not pass:

Test suite polysemy-plugin-test: RUNNING...
test/TypeErrors.hs:9: failure in expression `:set -fplugin=Polysemy.Plugin'
expected:
 but got: Could not find module ‘Polysemy.Plugin’
          Use -v to see a list of the files searched for.

AFAIK "polysemy-plugin" is a GHC plugin and this doctest tries to import the plugin and set it up before running the test, but it's not available to the doctest.

I noticed that when a test requires executables built by the package, we do something like this:

  arbtt = overrideCabal super.arbtt (drv: {
    preCheck = ''
      for i in $PWD/dist/build/*; do
        export PATH="$i:$PATH"
      done
    '';
  });

Is there an equivalent for exposing plugins built by the package to the test? I assumed that it would already be "in-scope" so to speak.

To Reproduce
Steps to reproduce the behavior:

  1. git clone https://github.com/sevanspowell/nixpkgs.git
  2. cd nixpkgs
  3. git fetch --all
  4. git checkout d83d6a
  5. nix-build --no-out-link -A haskellPackages.polysemy-plugin

Expected behavior
Builds and tests pass.

Metadata

 - system: `"x86_64-linux"`
 - host os: `Linux 4.14.79, NixOS, 18.09.1228.a4c4cbb613c (Jellyfish)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.1.3`
 - channels(sam): `""`
 - channels(root): `"nixos-18.09.1228.a4c4cbb613c"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

Maintainer information:

# a list of nixpkgs attributes affected by the problem
attribute: pkgs.haskellPackages.polysemy-plugin
# a list of nixos modules affected by the problem
module:

@peti

@sevanspowell
Copy link
Contributor Author

sevanspowell commented Oct 15, 2019

Notes:

Results of adding ":show" flags to doctest (test/TypeErrors.hs):

:show packages

active package flags: none

:show linker

----- Linker state -----
          Pkgs: [base-4.12.0.0, integer-gmp-1.0.2.0, ghc-prim-0.5.3, rts-1.0]
          Objs: []
          BCOs: []

:show imports

import TypeErrors -- added automatically
import Prelude -- implicit

I would expect that these would need to at least have polysemy-plugin.

Suggests to me that maybe doctests exe isn't loading the package-db.

@sevanspowell
Copy link
Contributor Author

Hmm, adding to the doctest args:

  , "-package-db /nix/store/fa3n97826xlvbck75wgpsm7xc6crv966-ghc-8.6.5-with-packages/lib/ghc-8.6.5/package.conf.d" <- my packagedb for this drv
  , "-package polysemy"

I can then see polysemy with :show packages.

So I need to source a package-db with polysemy-plugin in it. Not sure how to do that at this stage.

@sevanspowell
Copy link
Contributor Author

sevanspowell commented Oct 15, 2019

Ok, progress! I think I'm at least partially correct with the above.

If I source the package-db created as part of the build, I can get the doctest to pass.

I fed "-package-db $src/dist-newstyle/packagedb/ghc-8.6.5" to doctest via the args:

{-# LANGUAGE CPP #-}

module DoctestSpec where

import Test.Hspec
import Test.DocTest

spec :: Spec
spec = parallel $ describe "Error messages" $ it "should pass the doctest" $ doctest
  [ "--fast"
  , "-fobject-code"
  , "-package-db /home/sam/code/nix/polysemy-plugin-0.2.3.0/dist-newstyle/packagedb/ghc-8.6.5"
  , "-XDataKinds"
  , "-XDeriveFunctor"
  , "-XFlexibleContexts"
  , "-XGADTs"
  , "-XLambdaCase"
  , "-XPolyKinds"
  , "-XRankNTypes"
  , "-XScopedTypeVariables"
  , "-XStandaloneDeriving"
  , "-XTypeApplications"
  , "-XTypeFamilies"
  , "-XTypeOperators"
  , "-XUnicodeSyntax"

#if __GLASGOW_HASKELL__ < 806
  , "-XMonadFailDesugaring"
  , "-XTypeInType"
#endif

  , "test/TypeErrors.hs"
  ]

And the tests pass!

Note "-package-db ./dist-newstyle/packagedb/ghc-8.6.5" or "-package-db ./dist/package.conf.inplace" will also suffice (depending on cabal version).

So, now my question/issue changes to "What's the best way for me to source the built package-db in the doctests of this package?"

One way would be to add something to the "preCheck" hook that modifies the source code, but I imagine that would be frowned upon.

@sevanspowell
Copy link
Contributor Author

Additionally, does this issue need to be pushed upstream?

If so, is there a more general way we can do it? The package.conf is going to be in different locations for different users/different versions of cabal/ghc/etc.

@cdepillabout
Copy link
Member

cdepillabout commented Oct 18, 2019

@sevanspowell I think you diagnosed this problem correctly.

The doctests are being run in an environment that doesn't necessarily know about all of the other targets from the .cabal file.

This is actually a pretty big problem when running doctest tests. All the common Haskell build tools run tests in slightly different environments (with slightly different package databases available). This includes stack < 2.0, stack > 2.0, cabal test, cabal new-test, the default Haskell builder in nixpkgs, and haskell.nix.

This means that if you use plain doctests, you often have problems when building with different build tools. In nixpkgs, you can find a lot of Haskell packages where the tests are disabled purely because the doctests aren't working well.

The only "correct" solution is to use a wrapper around doctest that uses a setup hook to figure out which Haskell packages need to be exposed to the doctest executable:

http://hackage.haskell.org/package/cabal-doctest

Another possible solution is just to add the packages needed by doctest to the build-depends section of the test target that defines the doctests, and hope that cabal/stack decides to run doctest in a way where it can see the additional packages you added. (Although now that I look at the .cabal file for polysemy-plugin, I can see that it is already setup like this. The other weird thing is that I've never seen doctest used to test comments in the test files, instead of the actual source files for the library.)

This isn't really correct though, since the doctest test target in the cabal file normally doesn't actually need a build-depend on the packages that are being used in the doctests. It should instead have a runtime-depend. However, .cabal files don't have a way of specifying runtime dependencies for the test targets. This is why cabal-doctest is necessary.


Of course, the easiest solution is just to disable the tests for polysemy-plugin when building in nixpkgs.

Also, I should say that I don't have any experience using GHC plugins, so it is possible that GHC plugins require some other work-arounds when using doctests.

@cdepillabout
Copy link
Member

What's the best way for me to source the built package-db in the doctests of this package?

I'd say that the best way is to change polysemy-plugin to use something more reliable like cabal-doctest. If the authors don't want to do that (some people don't), then just disabling the tests in nixpkgs is the easiest thing to do.

@sevanspowell
Copy link
Contributor Author

Thanks @cdepillabout :)

@sevanspowell
Copy link
Contributor Author

There's a single doctest. So I might try submitting a PR to polysemy-plugin to use cabal doctest, see if that fixes the issue.

sevanspowell pushed a commit to sevanspowell/polysemy that referenced this issue Oct 25, 2019
- Haskell build tools run in slightly different environments (meaning different
  package databases are available).
- The nixpkgs build for polysemy-plugin is failing due to a missing package
  database, which causes the doctest to fail (more information here:
  NixOS/nixpkgs#71164).
- By using cabal-doctest we can expose the Haskell packages required to the
  doctests no matter the build tool we're using.
sevanspowell added a commit to sevanspowell/nixpkgs that referenced this issue Oct 28, 2019
- polysemy-plugin was broken due to failing doctests:
  NixOS#71164.
- I submitted a PR upstream to fix this:
  polysemy-research/polysemy#265.
- I've applied the patch of the PR here and moved the default
  "polysemy" attribute to "polysemy_1_2_0_0" because polysemy-plugin
  requires "polysemy >= 1.2.0.0".
- Move default "polysemy-zoo" attribute to "polysemy-zoo_0_6_0_1"
  because it is fixed by the polysemy-plugin changes and fixes issues
  with building "polysemy-RandomFu" and "knit-haskell".
- Removed packages no longer broken from
  "configuration-hackage2nix.yaml".
- Add cabal-doctest to setupDepends of polysemy-plugin.
isovector pushed a commit to polysemy-research/polysemy that referenced this issue Oct 28, 2019
* Use cabal-doctest

- Haskell build tools run in slightly different environments (meaning different
  package databases are available).
- The nixpkgs build for polysemy-plugin is failing due to a missing package
  database, which causes the doctest to fail (more information here:
  NixOS/nixpkgs#71164).
- By using cabal-doctest we can expose the Haskell packages required to the
  doctests no matter the build tool we're using.

* Use cabal-doctest in polysemy, build on GHC 8.8.1

- Use @googleson78 's changes to build polysemy on GHC 8.8.1, with slight
  modifications. The source distribution is now found in "dist-newstyle/sdist",
  so we've updated the command to point at that folder. Additionally, cabal
  v2-install doesn't support installing .tar.gz files in the same way v1-install
  did, so updated the command to use "cabal v1-install".
- Modified polysemy to use "cabal-doctest" and so overcome issues with the
  doctest tests (see issue #258, PR #265).
@sevanspowell
Copy link
Contributor Author

Thanks all!

Upstream issue fixed here: polysemy-research/polysemy#267
Fixed in nixpkgs here: #71967

peti pushed a commit that referenced this issue Nov 1, 2019
- polysemy-plugin was broken due to failing doctests:
  #71164.
- I submitted a PR upstream to fix this:
  polysemy-research/polysemy#265.
- I've applied the patch of the PR here and moved the default
  "polysemy" attribute to "polysemy_1_2_0_0" because polysemy-plugin
  requires "polysemy >= 1.2.0.0".
- Move default "polysemy-zoo" attribute to "polysemy-zoo_0_6_0_1"
  because it is fixed by the polysemy-plugin changes and fixes issues
  with building "polysemy-RandomFu" and "knit-haskell".
- Removed packages no longer broken from
  "configuration-hackage2nix.yaml".
- Add cabal-doctest to setupDepends of polysemy-plugin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants