awakesecurity / proto3-wire

https://hackage.haskell.org/package/proto3-wire
Other
23 stars 30 forks source link

Allow bytestring-0.11 in test #88

Closed TristanCacqueray closed 2 years ago

TristanCacqueray commented 2 years ago

This change enables running proto3-wire test with ghc-9.2.

TristanCacqueray commented 2 years ago

The test compile, but it somehow still fails with:

tests:
<interactive>:1:1: warning: [-Wdeprecations]
    Module ‘Proto3.Wire.Builder’ is deprecated:
      This module is no longer used by the rest of the proto3-wire package.

Test suite tests: FAIL
Test suite logged to: dist/test/proto3-wire-1.4.0-tests.log
0 of 1 test suites (0 of 1 test cases) passed.
j6carey commented 2 years ago

The test compile, but it somehow still fails with: ...

The bit about deprecation is probably not relevant. How are you setting up your build environment with the new version of bytestring, and presumably GHC 9.2.x?

j6carey commented 2 years ago

I am guessing that bytestring-0.11 is key to triggering the problem, and our CI tests do not use it. In particular, this excellent change is likely the reason for the failure:

https://github.com/haskell/bytestring/pull/175

I will have to update the low-level hackery to cope with the change, once I can get a build environment.

TristanCacqueray commented 2 years ago

@j6carey oh well, trying to create a reproducer I see the tests are now passing, and the warning was a red herring :)

TristanCacqueray commented 2 years ago

For what its worth, here is a nix-shell that show this patch working with ghc-9.2.4:

let
  nixpkgs = import (builtins.fetchTarball {
    url =
      "https://github.com/NixOS/nixpkgs/archive/00d73d5385b63e868bd11282fb775f6fe4921fb5.tar.gz";
  });
  pkgs = nixpkgs { };
  hspkgs = pkgs.haskell.packages.ghc924.extend (hpFinal: hpPrev: {
    data-diverse = pkgs.haskell.lib.dontCheck
      (pkgs.haskell.lib.overrideCabal hpPrev.data-diverse { broken = false; });
    proto3-wire = hpPrev.proto3-wire.overrideAttrs (_: {
      patches = [
        (pkgs.fetchpatch {
          url =
            "https://github.com/TristanCacqueray/proto3-wire/commit/14eff5d9b76e0f30b0735087e8a08edba610ea7a.patch";
          sha256 = "sha256-JwGGkrA9796btdsClaMjotBXPORCEkN+oB7Pr9GIMLc=";
        })
      ];
    });
  });

in hspkgs.ghcWithPackages (p: [ p.proto3-wire ])
evanrelf commented 2 years ago

I am guessing that bytestring-0.11 is key to triggering the problem, and our CI tests do not use it.

Oh yeah duh, I'm sorry 🙃 You're totally right.

j6carey commented 2 years ago

@TristanCacqueray , which branch of nixpkgs has 00d73d5385b63e868bd11282fb775f6fe4921fb5 ? (I ask because I'm getting an error when I try to build data-diverse in the latest master-branch nixpkgs.)

Answering my own question: haskell-updates.

TristanCacqueray commented 2 years ago

@j6carey in https://github.com/NixOS/nixpkgs/commit/00d73d5385b63e868bd11282fb775f6fe4921fb5 you can see it is coming from a recent PR made to the haskell-updates branch

j6carey commented 2 years ago

@TristanCacqueray , I tried reproducing your results in a slightly different way: by making these changes to proto3-wire:

diff --git a/nix/nixpkgs.nix b/nix/nixpkgs.nix
index bb70009..cd5ecd0 100644
--- a/nix/nixpkgs.nix
+++ b/nix/nixpkgs.nix
@@ -2,8 +2,8 @@ args:

 let
   # nixpkgs release 21.05
-  rev = "7e9b0dff974c89e070da1ad85713ff3c20b0ca97";
-  sha256 = "1ckzhh24mgz6jd1xhfgx0i9mijk6xjqxwsshnvq789xsavrmsc36";
+  rev = "00d73d5385b63e868bd11282fb775f6fe4921fb5";
+  sha256 = "1hlhfgh3v6jvn125ck43i6sv6flwwv8vmk7cz5yrdcyh8lm86v6a";

   nixpkgs = builtins.fetchTarball {
     url = "https://github.com/NixOS/nixpkgs/archive/${rev}.tar.gz";
diff --git a/nix/pkgs.nix b/nix/pkgs.nix
index abe9410..9b5a56b 100644
--- a/nix/pkgs.nix
+++ b/nix/pkgs.nix
@@ -1,7 +1,7 @@
 import ./nixpkgs.nix {
   overlays = [
     (import ./haskell-packages.nix {
-      compiler = "ghc884";
+      compiler = "ghc924";
     })
   ];
 }

The result is this compilation error in a dependency:

test/Data/Diverse/ManySpec.hs:308:24: error:
    parse error on input ‘Bool’
    |
308 |             amend' @ '[Bool, Char] x (True ./ 'B' ./ nil) `shouldBe`
    |                        ^^^^

I'm still trying to figure out why...

j6carey commented 2 years ago

Looks like this is why: https://github.com/louispan/data-diverse/commit/c72139091b6a10fc9b2b022c18f5f2e26d8b774d

j6carey commented 2 years ago

I'm still having trouble getting the appropriate build environment; however, I see no advantage to restricting the version range in tests more than it is restricted in the library itself, so I think it is best to go ahead with the merge and unblock the general effort to support GHC 9.2 in nixpkgs.

TristanCacqueray commented 2 years ago

Thank you for the prompt feedback!

j6carey commented 2 years ago

@TristanCacqueray , you are welcome!