Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove cleanSourceWith from features list #14

Merged
merged 1 commit into from Sep 2, 2019
Merged

Remove cleanSourceWith from features list #14

merged 1 commit into from Sep 2, 2019

Conversation

lukateras
Copy link
Contributor

@lukateras lukateras commented Sep 2, 2019

gitignoreSource does not compose with cleanSourceWith as of 6e75696.

See discussion in #11. Another way to resolve this is to make cleanSourceWith compose one way only, with gitignoreSource always having to be the final filter (based on @yorickvP's workaround):

{ lib ? import <nixpkgs/lib> }:
let
  find-files = import ./find-files.nix { inherit lib; };

  constantSource = path:
    if path ? _isLibCleanSourceWith
    then builtins.path {
      name = "source";
      inherit (path) filter;
      path = path.origSrc;
    }
    else builtins.path {
      name = "source";
      inherit path;
    };
in
{
  inherit (find-files) gitignoreFilter;

  gitignoreSource = path:
    constantSource (lib.cleanSourceWith {
      filter = find-files.gitignoreFilter path;
      src = path;
    });
}

The problem with this is that it's not intuitive and still results in API breakage, while there are other ways to compose filters that are more explicit and work with deterministic source names.

@roberth roberth mentioned this pull request Sep 2, 2019
4 tasks
@roberth roberth merged commit 6c4ab20 into hercules-ci:master Sep 2, 2019
@roberth
Copy link
Member

roberth commented Sep 2, 2019

Thanks! I've merged it to reflect the current situation.
I'm confident that this can be fixed properly, see #15

@lukateras
Copy link
Contributor Author

Sounds great! I assume the general idea is to make sure that cleanSourceWith produces deterministic store paths or works with builtins.path in its src. Looking forward to #15 :3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants