srid / haskell-flake

A `flake-parts` Nix module for Haskell development
https://community.flake.parts/haskell-flake
MIT License
128 stars 14 forks source link

Patches are applied twice if `buildFromSDist` is enabled #290

Open johnhampton opened 3 months ago

johnhampton commented 3 months ago

The haskell-flake is trying to apply patches twice. The issue seems to be in the new buildFromSDist implementation. Setting buildFromSDist to false resolves the problem. For a reproduction, visit https://github.com/johnhampton/haskell-flakes-double-patch.

{
  inputs = {
    nixpkgs.url = "github:nixos/nixpkgs/nixpkgs-unstable";
    flake-parts.url = "github:hercules-ci/flake-parts";
    haskell-flake.url = "github:srid/haskell-flake";
  };
  outputs = inputs@{ self, nixpkgs, flake-parts, ... }:
    flake-parts.lib.mkFlake { inherit inputs; } {
      systems = nixpkgs.lib.systems.flakeExposed;

      imports = [ inputs.haskell-flake.flakeModule ];
      perSystem = { self', pkgs, config, ... }:
        {
          haskellProjects.default = {
            settings = {
              logict = {
                # buildFromSdist = false;
                patches = [ ./nix/patches/logict.patch ];
              };
            };
          };
        };
    };
}
error: builder for '/nix/store/2mbwfyxq1jvqiz3cq52dn8r9za3fia06-logict-0.8.1.0.drv' failed with exit code 1;
       last 10 log lines:
       > source root is logict-0.8.1.0
       > setting SOURCE_DATE_EPOCH to timestamp 1711982133 of file logict-0.8.1.0/test/Test.hs
       > warning: file logict-0.8.1.0/test/Test.hs may be generated; SOURCE_DATE_EPOCH may be non-deterministic
       > Running phase: patchPhase
       > applying patch /nix/store/r335m619yhwcqr0flp4ad9s6kihqqz3w-logict.patch
       > patching file logict.cabal
       > Reversed (or previously applied) patch detected!  Assume -R? [n]
       > Apply anyway? [n]
       > Skipping patch.
       > 1 out of 1 hunk ignored -- saving rejects to file logict.cabal.rej
       For full logs, run 'nix log /nix/store/2mbwfyxq1jvqiz3cq52dn8r9za3fia06-logict-0.8.1.0.drv'.
error: 1 dependencies of derivation '/nix/store/bp67yaxnr421c3whglrdgvy4a7dy03hx-haskell-flakes-double-patch-0.1.0.0.drv' failed to build
srid commented 3 months ago

Oh man, buildFromSdist is causing problems once again 😔 cc @roberth

(I'm travelling; will be a while to look into this further)

srid commented 3 months ago

I haven't looked into this in detail, but I suspect that a short-term fix would be to eval appendPatches after buildFromSdist (just as we do with removeReferencesTo):

https://github.com/srid/haskell-flake/blob/785956b3d77cf5e9ed304cd6514ad2f45750188e/nix/modules/project/settings/default.nix#L75-L81

roberth commented 3 months ago

291 lgtm; see comment there.

Maybe also make sure that buildFromSdist is not used when the source is already a tarball and therefore presumably an sdist?

srid commented 3 months ago

298 should also fix this for your particular repro, but I believe we still need #291 if someone were to apply patches on a local package (is that an use-case we should support?).

johnhampton commented 3 months ago

298 appears to resolve my issue. I'm unsure about when to use buildFromSdist. I don't foresee patching local packages, but would I ever want to enable buildFromSdist for a remote package with packages.*.source pointing to a GitHub repository?

If fixing buildFromSdist isn't possible, it would be helpful to show an error or warning when both buildFromSdist and patches (or any other invalid option) are enabled together.

roberth commented 2 months ago

291 if someone were to apply patches on a local package (is that an use-case we should support?).

Might be useful as an escape hatch if the local build needs to be different from the packaged build for some (bad) reason.