haskellPackages.rest-rewrite: fix tests

[tbidne]

Description of changes

Resolves #215073.

There are two problems:

1. Test dependencies

For starters, the error described in the issue is due to the tests requiring z3 at runtime:

rest: z3: createProcess: posix_spawnp: does not exist (No such file or directory)

The tests also require graphviz.

Fix

This is fixed with:

rest-rewrite = overrideCabal (drv: {
  testToolDepends = (drv.testToolDepends or [ ]) ++ [ pkgs.graphviz pkgs.z3 ];
  ...
}) super.rest-rewrite;

It appears to me that it might be nicer to fix the derivation in hackage-packages.nix i.e. something like:

diff --git a/pkgs/development/haskell-modules/hackage-packages.nix b/pkgs/development/haskell-modules/hackage-packages.nix
index e1993ae80dd..8505ba1de6f 100644
--- a/pkgs/development/haskell-modules/hackage-packages.nix
+++ b/pkgs/development/haskell-modules/hackage-packages.nix
@@ -249671,8 +249671,8 @@ self: {
      }) {};
 
   "rest-rewrite" = callPackage
-    ({ mkDerivation, base, containers, hashable, monad-loops, mtl
-     , parsec, process, QuickCheck, text, time, unordered-containers
+    ({ mkDerivation, base, containers, hashable, graphviz, monad-loops, mtl
+     , parsec, process, QuickCheck, text, time, unordered-containers, z3
      }:
      mkDerivation {
        pname = "rest-rewrite";
@@ -249687,11 +249687,11 @@ self: {
          unordered-containers
        ];
        doHaddock = false;
+       testSystemDepends = [ graphviz z3 ];
        description = "Rewriting library with online termination checking";
        license = lib.licenses.bsd3;
        hydraPlatforms = lib.platforms.none;
-       broken = true;
-     }) {};
+     }) {inherit (pkgs) graphviz z3;};
 
   "rest-snap" = callPackage
     ({ mkDerivation, base, base-compat, bytestring, case-insensitive

But I am not sure how to do that / if it is any better.

2. Missing directory

Once those dependencies are present, you will encounter a different error:

Drawing graph
rest: graphs/fig4.dot: withFile: does not exist (No such file or directory)

This is due to the lines here:

mkGraph :: String -> DiGraph -> IO ()
mkGraph name graph = do
  let dotfile = printf "graphs/%s.dot" name
  let pngfile = printf "graphs/%s.png" name
  writeFile dotfile (graphString graph)
  result <- readProcessWithExitCode "dot" ["-Tpng", dotfile, "-o", pngfile] ""
  print result

In particular, the writeFile call requires the graphs directory to exist, but because it is not copied over during out-of-tree builds (it contains no source files), it does not exist, hence fails on nix.

Fix

This is fixed with:

rest-rewrite = overrideCabal (drv: {
  ...
  # Test suite 'rest' relies on the relative dir graphs/ existing. This
  # is a problem for "out of tree" builds, as the directory is not copied
  # over.
  postPatch = ''
    mkdir graphs
  '';
}) super.rest-rewrite;

Once again there are possible alternative solutions e.g. adding the following to the cabal file:

extra-source-files:
  graphs/.DONOTDELETE

I attempted this via a patch but wasn't able to get it to work 🤷.

Misc

Sadly liquidhaskell -- which was transitively broken by rest-rewrite -- is still broken for another reasons, hence its move to broken.yaml. liquid-fixpoint now works, however.

Also, the transitive/hackage-packages.nix files were updated via regenerate-hackage-packages.nix.

Please let me know what changes I should make. Thanks!

Prerequisites

The following is a list of of tasks that must be completed before this PR is merged.

• rest-rewrite
  • PR merged.
  • New hackage release.
  • Updated in nixpkgs.
• cabal2nix
  • PR merged.
  • Updated in nixpkgs.
• This PR modified to remove rest-rewrite from broken package list only. The other changes will not be needed once the other actions are completed.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[cdepillabout]

Alright, I merged in NixOS/cabal2nix#605, updated cabal2nix in nixpkgs, and regenerated the expression for rest-rewrite in 1b36ac8, which is in #240387. However, when I try to build it, the tests are now failing:

$ NIXPKGS_ALLOW_BROKEN=1 nix-build -A haskellPackages.rest-rewrite
this derivation will be built:
  /nix/store/srwh5y2skdv9ix3vj6k5c41kgyvj4cqa-rest-rewrite-0.4.1.drv
building '/nix/store/srwh5y2skdv9ix3vj6k5c41kgyvj4cqa-rest-rewrite-0.4.1.drv'...
setupCompilerEnvironmentPhase
...
[10 of 11] Compiling WQO              ( test/WQO.hs, dist/build/test-rest/test-rest-tmp/WQO.o )
[11 of 11] Compiling Main             ( test/Test.hs, dist/build/test-rest/test-rest-tmp/Main.o )
[12 of 12] Linking dist/build/test-rest/test-rest
running tests
Running 2 test suites...
Test suite rest: RUNNING...
s₁ ∩ s₀ → ∅
Z ∩ (X ∪ Y) → (Z ∩ X) ∪ (Z ∩ Y)
X ∩ X → X
X ∪ (Y ∪ Z) → (X ∪ Y) ∪ Z
(X ∪ Y) ∪ Z → X ∪ (Y ∪ Z)
(X ∪ Y) ∩ Z → (X ∩ Z) ∪ (Y ∩ Z)
X ∩ ∅ → ∅
Z ∪ (X ∩ Y) → (Z ∪ X) ∩ (Z ∪ Y)
X ∪ X → X
(X ∩ Y) ∪ Z → (X ∪ Z) ∩ (Y ∪ Z)
X ∪ ∅ → X
REST run completed, in 0.01201661s
Drawing graph
rest: graphs/fig4.dot: withFile: does not exist (No such file or directory)
Test suite rest: FAIL
Test suite logged to: dist/test/rest-rewrite-0.4.1-rest.log
Test suite test-rest: RUNNING...
...

Maybe a test file that isn't included in the sdist?

@tbidne Would you be able to take a look at this?

[tbidne]

@cdepillabout thanks for that!

Indeed, that test error is actually what I fixed in zgrannan/rest#38. However there is no hackage release for it yet, hence the failure. I will ask for a release and ping you here once it's up.

[cdepillabout]

@tbidne Ah yeah, sorry I missed that!

I actually tried applying your PR as a patch, and it didn't seem to actually fix the test error for me. I'm not sure why, but maybe it is just the difference between the sdist build and the environment nix runs in.

However, I did take your change from this PR and apply it in 3ba086a. So rest-rewrite is now working.

If this is a package you frequently use, you may want to think about adding yourself as the maintainer, so you can see if it ends up breaking in the future.

Thanks!

[tbidne]

Ah interesting @cdepillabout. I noticed the same thing as you actually (patch did not work, though I also am not sure why).

I'll proceed with the rest-rewrite hackage release, and circle back with another PR here if I can improve the nix situation / remove the need for that mkdir hack. I'll also take you up on the maintainers suggestion then.

Thanks again!