always clean the source, no matter what

[DavHau]

This change might remove the need for materialization.
I can now edit unrelated files in my project without the haskell-project-pla-to-nix-pkgs derivation being re-built all the time.

I haven't understood all the complexity of clean-source-with.nix yet, therefore I'm not sure if this change can possibly introduce breakages elsewhere.
I'm happy to make changes according to your suggestions.

[michaelpj]

I think this was all introduced in #209 and mostly hasn't been touched much since. So the reasoning may be lost to time unless Hamish remembers...

[DavHau]

If I modify canCleanSource to always return true, it leads to a mass rebuild of what is probably the whole haskell.nix stack itself, but it seem to work fine.
So if you guys can verify that this doesn't break anything, then just removing canCleanSource might be the better solution.

[hamishmack]

bors try

[iohk-bors]

try

Build failed:

• buildkite/haskell-dot-nix

[hamishmack]

This is failing with:

error: string '/nix/store/jgfzhnj5pyim2yf8yv4q0jxwnlwjnvy5-haskell-language-server-1.6.1.1-src' cannot refer to other paths
  |  
  | at /var/lib/buildkite-agent-iohk/builds/packet-ipxe-5-ci5-2/input-output-hk/haskell-dot-nix/lib/clean-source-with.nix:109:52:
  |  
  | 108\|       filter = filter';
  | 109\|       outPath = (builtins.path { filter = filter'; path = origSrc; name = name'; }) + origSubDir;
  | 110\|       _isLibCleanSourceWithEx = true;

Keep in mind that canCleanSource is based on the Nixpkgs version here.

[DavHau]

It is probably possible to modify the source filtering to not run into this problem.
@hamishmack Could you provide me with a derivation that reproduces the problem, so I can work out a solution?

[hamishmack]

The thing I was worried about is that neither Haskell.nix cleanSourceWIth nor the Nixpkgs one actually do anything meaningful if the source is in the store. Both rely on the builtins.path filter function. Without early cut-off there is little point in filtering any other way.

For instance if I run:

nix-build -E 'let pkgs = import <nixpkgs> {}; in pkgs.lib.cleanSourceWith { src = pkgs.fetchgit { url = "https://github.com/input-output-hk/haskell.nix"; rev = "f3f2c08051d64ed540448604009b72bd1f41ad12"; sha256="sha256-T7J2vdIHll1LAz22XvMS7cfYQXY8/kKuGeGb9DuJiWM="; }; filter = type: path: false; }'

I might expect to get an empty result, but instead it just returns the src argument unfiltered.

[DavHau]

The thing I was worried about is that neither Haskell.nix cleanSourceWIth nor the Nixpkgs one actually do anything meaningful if the source is in the store.

I think there is nothing to worry about. This just seems to be weird behavior of nix-build. The final result of the filtering is exposed via the .outPath attribute. If you put your expression into a string substitution or use it as a build input for something else, then you will end up with an empty directory as expected.

Without early cut-off there is little point in filtering any other way.

You can filter in any arbitrary imaginable way. It doesn't matter. As long as you do filter. What is important is that you pass the result through builtins.path while assigning a name. Luckily cleanSourceWith does already exactly that. But we could make use of any other filtering method and then pipe the result through builtins.path. That should work as well.

I'm happy to experiment with this, but I need a example to reproduce the error. So far I could not.

[hamishmack]

Error is in buildkite for the bors try commit.

https://buildkite.com/input-output-hk/haskell-dot-nix/builds/5866#300038b5-8130-475f-935d-b3a84fdf13b8

I think it is running something like:

nix-build build.nix -A maintainer-scripts.check-hydra -o check-hydra.sh
./check-hydra.sh 3

[DavHau]

The issue with builtins.path triggering a cannot refer to other paths error, only happens in restricted eval mode and only with an outdated version of nix which is currently used by the hydra of haskell.nix.

The specific issue you're running into has been fixed in nix here:
NixOS/nix#5163

The current upstream hydra has this fix included, though the hydra-unstable from nixpkgs used in haskell.nix seems to be outdated. Currently at 2021-08-11.

I now updated hydra now via flake input + adding an overlay.

Please have a look again if your tests succeed.

[DavHau]

bors try

[iohk-bors]

🔒 Permission denied

Existing reviewers: click here to make DavHau a reviewer

[hamishmack]

bors try

[iohk-bors]

try

Build succeeded:

• buildkite/haskell-dot-nix
• ci/hydra:Cardano:haskell-nix:required

[michaelpj]

This is missing a couple of things:

• A comment explaining the hydra overlay, which I guarantee will confuse people in the future.
• An audit of the other uses of canCleanSource - we should get rid of them also, presumably, and delete the function.

[DavHau]

This is missing a couple of things:

* A comment explaining the hydra overlay, which I guarantee will confuse people in the future.

* An audit of the other uses of `canCleanSource` - we should get rid of them also, presumably, and delete the function.

I can have a look at this and make another PR.

[michaelpj]

Thank you :) I'm being fussy, but Nix code is hard to maintain without a certian amount of fussiness 😱

[ocharles]

I believe this might now be causing us to run into this error:

error: string '/nix/store/41m72als4hpykqy6ihfqg3875b496ljh-hoogle-5.0.18.3-src' cannot refer to other paths, at /nix/store/q9mmnkrhkqwhr1yahrhw5lk153dx8nkc-haskell.nix-src/lib/clean-source-with.nix:109:52

Edit: I can confirm reverting just this commit fixes the problem 😞

[michaelpj]

Indeed, I also wasn't clear on the import of this hydra comments.

Does this mean that haskell.nix is now broken for anyone with that combination of nix/hydra versions?

[DavHau]

Indeed, I also wasn't clear on the import of this hydra comments.

Does this mean that haskell.nix is now broken for anyone with that combination of nix/hydra versions?

Yes, any hydra build against nix prior to 2.4 will fail during evaluation, as it won't contain the fixes in nix required for builtins.path.
I'm sorry for the inconvenience caused by this. My intention was not to have this merged already.

What we could do is to modify canCleanSource to check for the nix version being greater than 2.4 and only then do the source cleaning.
Though the downside of a nix version check would be that if some users hydra has a different version than the locally used nix, it will possibly produce other derivations than expected by the user, leading to unexpected cache misses etc..

Therefore maybe source cleaning should be a setting of haskell.nix which can only be turned off manually by the user via some flag. That makes sure that users/machines with different nix versions never build different derivations.

[angerman]

Do we know the exact nix version range required? Can we check for it and abort with a warning that nix >= x.y.z is required for Haskell.nix?

[DavHau]

Do we know the exact nix version range required?

Yes, the oldest nix stable release which doesn't have the error is 2.4

Can we check for it and abort with a warning that nix >= x.y.z is required for Haskell.nix?

Done in #1409