Patches are applied twice if `buildFromSDist` is enabled

[johnhampton]

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]

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

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

[srid]

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):

haskell-flake/nix/modules/project/settings/default.nix

Lines 75 to 81 in 785956b

	# HACK: buildFromSdist must apply *last*
	# cf. https://github.com/srid/haskell-flake/pull/252
	# In future, we can refactor this as part of https://github.com/srid/haskell-flake/issues/285
	# NOTE: removeReferencesTo must apply *before* buildFromSdist, because the
	# later appears it fuck up the former otherwise.
	impl = lib.attrsets.removeAttrs cfg.impl [ "buildFromSdist" "removeReferencesTo" ];
	fns = lib.attrValues impl ++ [ cfg.impl.buildFromSdist cfg.impl.removeReferencesTo ];

[roberth]

#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]

#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]

#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]

#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.