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

Nix cannot execute Shake's test suite #267

Closed
peti opened this issue Jun 23, 2015 · 32 comments
Closed

Nix cannot execute Shake's test suite #267

peti opened this issue Jun 23, 2015 · 32 comments

Comments

@peti
Copy link

peti commented Jun 23, 2015

Shake's test suite depends on ghc being aware of extra packages, which we achieve by setting $GHC_PACKAGE_PATH. New versions of the test suite, however, run some kind of cabal configure, which fails when that environment variable is set: http://hydra.cryp.to/build/942465/nixlog/2/raw.

peti added a commit to peti/nixpkgs that referenced this issue Jun 24, 2015
@peti
Copy link
Author

peti commented Jun 24, 2015

All builds at http://hydra.cryp.to/project/shake fail as well. I've disabled the CI job for the time being.

@ndmitchell
Copy link
Owner

The error is fallout from #252, which means I now reconfigure Shake into a different directory so I can generate documentation.

So the relevant bit from the log is:

Command: runhaskell Setup.hs configure --builddir=output/docs/dist --user
Exit code: 1
Stderr:
Setup.hs: Use of GHC's environment variable GHC_PACKAGE_PATH is incompatible
with Cabal. Use the flag --package-db to specify a package database (it can be
used multiple times).

That's pretty horrid. I guess I need to grab the GHC_PACKAGE_PATH and convert that to a --package-db flag? Do you know what the GHC_PACKAGE_PATH points at, and if there is a package db in a predictable location from that? I've not really used either mechanism. It's a shame Cabal can't do that for me.

@ndmitchell
Copy link
Owner

Seems like yesodweb/yesod@a7cccf2#diff-7cd4bde7642547b86872a48f8139829eR138 should be able to be applied here as well to translate the variable.

@ndmitchell
Copy link
Owner

I've fixed this as part of getting Stack to work, can you try again now @peti?

@peti
Copy link
Author

peti commented Nov 24, 2015

The current master branch has an undeclared dependency on the cabal build tool:

## BUILD docs --abbrev=output=$OUT -j3
# cabal (for $OUT/docs/dist/setup-config)
Error when running Shake build system:
* output/docs/Success.txt
* output/docs/Files.lst
* output/docs/dist/doc/html/shake/index.html
* output/docs/dist/setup-config
user error (Development.Shake.cmd, system command failed
Command: cabal configure --builddir=output/docs/dist --user --package-db=/tmp/nix-build-shake-0.15.5.drv-0/package.conf.d --package-db=.
cabal: createProcess: runInteractiveProcess: exec: does not exist (No such file or directory))

TESTS FAILED
Test suite shake-test: FAIL

@ndmitchell
Copy link
Owner

Yes, the test depends on the cabal tool, can I add that as a run dependency some way?

@peti
Copy link
Author

peti commented Nov 24, 2015

Strangely enough, Cabal does not know about that build tool out of the box, so you need to register it in your Setup.hs file:

module Main ( main ) where

import Distribution.Simple
import Distribution.Simple.Program

main :: IO ()
main = defaultMainWithHooks simpleUserHooks
       { hookedPrograms = [ simpleProgram "cabal" ]
       }

Then the shake.cabal file can have an attribute

build-tools: cabal

in its test suite stanza.

@peti
Copy link
Author

peti commented Nov 24, 2015

If I add cabal to the list of available build tools manually, then the test suite fails as follows:

## BUILD docs --abbrev=output=$OUT -j3
# cabal (for $OUT/docs/dist/setup-config)
Config file path source is default config file.
Config file /tmp/nix-build-shake-0.15.5.drv-0/.cabal/config not found.
Writing default configuration to
/tmp/nix-build-shake-0.15.5.drv-0/.cabal/config
Warning: The package list for 'hackage.haskell.org' does not exist. Run 'cabal
update' to download it.
Resolving dependencies...
Configuring shake-0.15.5...
# cabal (for $OUT/docs/dist/doc/html/shake/index.html)
Running Haddock for shake-0.15.5...
Preprocessing library shake-0.15.5...
haddock: internal error: ./package.cache: openBinaryFile: does not exist (No such file or directory)
Error when running Shake build system:
* output/docs/Success.txt
* output/docs/Files.lst
* output/docs/dist/doc/html/shake/index.html
user error (Development.Shake.cmd, system command failed
Command: cabal haddock --builddir=output/docs/dist
Exit code: 1
Stderr was empty)

TESTS FAILED

@ndmitchell
Copy link
Owner

What do you have as the GHC_PACKAGE_PATH when running Nix packages? Is it accurate? Does it contain a cache of packages? Will it have all the packages required to build Shake?

Given cabal is a runtime dependency, I really don't want to add it to the compile time dependency list, and writing my own Setup.hs fills me with horror - it makes everything related to Shake much harder. Any way to hint at it being a runtime dependency via other mechanisms? Nix specific metadata somewhere?

@peti
Copy link
Author

peti commented Nov 24, 2015

GHC_PACKAGE_PATH is accurate and refers to a directory that contains the transitive closure of all dependencies listed in the Cabal file.

I'm not sure what the distinction between run-time and compile-time means with regard to the test suite. The test suite is run as part of the build, so arguably everything that's a run-time dependency of the test suite is a compile-time dependency of your package. I realize that the complications involved in getting those dependencies declared accurately are unpleasant and that you'd prefer to avoid them, but I don't see a way to accomplish that. If you execute cabal as part of your build, then cabal is a build-time dependency. That's just how it is.

Personally, I would not want my test suite to depend on cabal in the first place, but I guess that's a matter of taste.

Maybe you could try to detect the presence of cabal at run-time and fail the test suite gracefully, i.e. in a non-fatal way if it's not there?

@ndmitchell
Copy link
Owner

Building the test and running it are definitely two different things, although I can see how you might run it as part of the overall build step.

I would significantly prefer my test suite to not depend on Cabal, but every other approach has failed on other platforms for other reasons - I just want to generate haddock documentation and then check it is type checks. I might have another go at avoiding Cabal and using runhaskell on the Setup.hs - that should be NIX compliant, right? I think my previous flaw might

My concern with a custom Setup.hs is that it breaks plenty of other things and the API changes every 5 minutes, so the benefit of tests is Nix is probably outweighed by the cost of a custom Setup. I have used custom setups in the past, and vowed never to again. 4 times bitten, 5th time shy.

If all else fails, I'll just exclude this test based on the presence/absence of cabal, but then I always worry that my standard tester will accidentally put cabal in the wrong place and I'll omit tests I wanted to run. But that's not the end of the world.

TLDR: I'll try and avoid Cabal first.

@peti
Copy link
Author

peti commented Nov 24, 2015

Building the test and running it are definitely two different things, although I can see how you might run it as part of the overall build step.

Well, you are right, of course. It's just that Cabal does not distinguish build-time and run-time dependencies in that way; so these concepts do not exist in our "language" (for describing builds). Curiously enough, Nix does not distinguish these concepts (explicitly) either. We end up scanning the compiled binaries for paths that look like /nix/store/<sha-hash>-... and call those paths that we discover "run-time dependencies", but AFAIK you cannot easily declare those things in the build expression. Anyway, I digress ...

I just want to generate haddock documentation and then check it is type checks.

Using runhaskell Setup.hs seems like the safest bet to me -- that's what we do in Nix, too. One might also argue that you could leave this stuff out of the regression test suite entirely and instead provide some kind of test script that's run on Travis-CI (and by everyone else who cares) but not as part of cabal check.

My concern with a custom Setup.hs is that it breaks plenty of other things and the API changes every 5 minutes.

The custom-build API is subject to changes, no doubt, but that particular part of the API has been stable for a long time. I wouldn't worry about the stability of your build because of use of the hookedPrograms extension. Many other foundational libraries use these hooks (like zip-archive, which needs zip to run its test suite), and these don't seem to have any trouble. The major downside with build-tools is that cabal-install will not install them automatically (but stack will).

If all else fails, I'll just exclude this test based on the presence/absence of cabal, but then I always worry that my standard tester will accidentally put cabal in the wrong place and I'll omit tests I wanted to run.

You could check the presence of an environment variable, say SHAKE_STRICT_TESTS, and define that one in your Travis-CI job to force test failures if cabal isn't there. That way, the tests would just work(tm) for everyone, but you wouldn't run the risk of getting false positives because the travis guys changed their default $PATH.

@ndmitchell
Copy link
Owner

I've switched to runhaskell Setup, let's see how that does.

@peti
Copy link
Author

peti commented Nov 24, 2015

Now it says:

## BUILD docs --abbrev=output=$OUT -j3
# runhaskell (for $OUT/docs/dist/setup-config)

Setup:1:1: lexical error at character '\DEL'
Error when running Shake build system:
* output/docs/Success.txt
* output/docs/Files.lst
* output/docs/dist/doc/html/shake/index.html
* output/docs/dist/setup-config
user error (Development.Shake.cmd, system command failed
Command: runhaskell Setup configure --builddir=output/docs/dist --user --package-db=/tmp/nix-build-shake-0.15.5.drv-0/package.conf.d --package-db=.
Exit code: 1
Stderr:
Setup:1:1: lexical error at character '\DEL'
)

TESTS FAILED

@ndmitchell
Copy link
Owner

Hmm, it looks like it's trying to runhaskell on a literal file called Setup. Any chance Nix created that, and thus GHC's "find Setup.hs or Setup.lhs" logic got bypassed?

@ndmitchell
Copy link
Owner

OK, I've made it explicitly ask for Setup.hs, since there's no chance I'll ever use Setup.lhs anyway.

@peti
Copy link
Author

peti commented Nov 24, 2015

Now we're back to the same error we had with cabal before:

# runhaskell (for $OUT/docs/dist/doc/html/shake/index.html)
Running Haddock for shake-0.15.5...
Preprocessing library shake-0.15.5...
haddock: internal error: ./package.cache: openBinaryFile: does not exist (No such file or directory)
Error when running Shake build system:
* output/docs/Success.txt
* output/docs/Files.lst
* output/docs/dist/doc/html/shake/index.html
user error (Development.Shake.cmd, system command failed
Command: runhaskell Setup.hs haddock --builddir=output/docs/dist
Exit code: 1
Stderr was empty)

TESTS FAILED

Not sure what is going on with ./package.cache. Can your code that interprets GHC_PACKAGE_PATH deal with an empty entry? We configure the variable as follows:

export GHC_PACKAGE_PATH="$packageConfDir:"

ndmitchell added a commit that referenced this issue Nov 24, 2015
@ndmitchell
Copy link
Owner

I take the path and pass each to cabal in order. I guess cabal prefers the last flag, and GHC prefers the first path entry. I've now reversed it, which might give you more luck.

@ndmitchell
Copy link
Owner

(It would be nice if Cabal just obeyed the path like everyone else did :()

@peti
Copy link
Author

peti commented Nov 24, 2015

535857e gives the same error as before: no change.

What bothers me is the path ./package.cache. I'm not sure what the current working directory is at that point, but if it's the top-level source code directory, then it's no wonder there's no package.cache there -- because there isn't.

@ndmitchell
Copy link
Owner

Given a GHC_PACKAGE_PATH of $packageConfDir: isn't that the same as saying there is a package DB at $packageConfDir and .? Why are you adding the trailing :? I assume that is where the ./package.cache is coming from.

@peti
Copy link
Author

peti commented Nov 24, 2015

According to https://downloads.haskell.org/~ghc/latest/docs/html/users_guide/packages.html#ghc-package-path, the meaning is:

If GHC_PACKAGE_PATH ends in a separator, then the default package database stack (i.e. the user and global package databases, in that order) is appended.

ndmitchell added a commit that referenced this issue Nov 24, 2015
@ndmitchell
Copy link
Owner

I did not know what. Now I just filter out "", I hope that's sufficient...

@peti
Copy link
Author

peti commented Nov 24, 2015

## BUILD docs --abbrev=output=$OUT -j3
# runhaskell (for $OUT/docs/dist/setup-config)
Configuring shake-0.15.5...
# runhaskell (for $OUT/docs/dist/doc/html/shake/index.html)
Running Haddock for shake-0.15.5...
Preprocessing library shake-0.15.5...
haddock: internal error: ./package.cache: openBinaryFile: does not exist (No such file or directory)
Error when running Shake build system:
* output/docs/Success.txt
* output/docs/Files.lst
* output/docs/dist/doc/html/shake/index.html
user error (Development.Shake.cmd, system command failed
Command: runhaskell Setup.hs haddock --builddir=output/docs/dist
Exit code: 1
Stderr was empty)

TESTS FAILED

@ndmitchell
Copy link
Owner

splitSearchPath "foo:" gives ["foo","."], whereas curiously the Windows version gives ["foo"]. Very weird. I now filter out both empty and ..

@peti
Copy link
Author

peti commented Nov 24, 2015

46b7533 succeeds its test suite in Nix. Wow, that was easy!

splitSearchPath "foo:" gives ["foo","."], whereas curiously the Windows version gives ["foo"].

I find it hard to imagine that this could possibly be intended behavior. And if it is, then it's still a really bad idea. Have you considered reporting this phenomenon as a bug?

@ndmitchell
Copy link
Owner

Finally! Thanks for all your help - I owe you a beer.

I've raised haskell/filepath#51 (to myself, as it happens) to investigate. I suspect this may meet both platforms guidelines, but if so, it needs explicitly documenting as a difference.

@peti
Copy link
Author

peti commented Nov 25, 2015

Is there going to be a new release of shake soon? I wonder because I'd like to re-enable the test suite in Nixpkgs, but I can only do so after the fixed version is available on Hackage.

@ndmitchell
Copy link
Owner

Shortly, yes, although the fsatrace stuff is currently in flux. I guess my options are to release saying that is unsupported, or wait a while til that clams down. @jacereda - any thoughts/preferences?

@jacereda
Copy link
Contributor

I'll be working on fsatrace and I think I can have something releaseable on Monday. But if you're in a hurry go ahead.

@ndmitchell
Copy link
Owner

Not in that much of a hurry - my Friday and weekend are already spoken for so early next week sounds good.

@peti
Copy link
Author

peti commented Nov 26, 2015

Just take your time guys, there's no need to hurry. When the new version comes out, please just ping be briefly so that I can update the build expression in Nixpkgs to run the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants