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.

Sign up for GitHub

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

systemFlake: make one unified outputsBuilder #58

Closed
wants to merge 2 commits into from
Closed

systemFlake: make one unified outputsBuilder #58

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
47dd1d8
systemFlake: make one unified outputsBuilder
Pacman99 May 17, 2021
931a57d
attempt at deprecating old builders
Pacman99 May 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 19 additions & 20 deletions src/systemFlake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,21 @@
extraArgs = sharedExtraArgs;
}

, packagesBuilder ? null
, defaultPackageBuilder ? null
, appsBuilder ? null
, defaultAppBuilder ? null
, devShellBuilder ? null
, checksBuilder ? null
, outputsBuilder ? channels: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming: I really like outputsBuilder. I don't think we should use exports. Any better ideas?

packages = packagesBuilder channels;
defaultPackage = defaultPackageBuilder channels;
apps = appsBuilder channels;
defaultApp = defaultAppBuilder channels;
devShell = devShellBuilder channels;
checks = checksBuilder channels;
}
Comment on lines +22 to +29
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you can imagine, this would have a couple consequences.

  • We create all the outputs whether users want them or not
  • I think flake check errors out from empty attrsets in defaultPackage and defaultApp

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use something like this instead?

        deprecatedBuilders = channels: {}
          // optionalAttrs (packagesBuilder != null) (packagesBuilder channels)
          // optionalAttrs (defaultPackageBuilder != null) (defaultPackageBuilder channels)
          // optionalAttrs (appsBuilder != null) (appsBuilder channels)
          // optionalAttrs (defaultAppBuilder != null) (defaultAppBuilder channels)
          // optionalAttrs (devShellBuilder != null) (devShellBuilder channels)
          // optionalAttrs (checksBuilder != null) (checksBuilder channels);


, packagesBuilder ? _: { }
, defaultPackageBuilder ? _: { }
, appsBuilder ? _: { }
, defaultAppBuilder ? _: { }
, devShellBuilder ? _: { }
, checksBuilder ? _: { }
, ...
}@args:

Expand Down Expand Up @@ -189,21 +198,11 @@ mergeAny otherArguments (

pkgs = mapAttrs importChannel channels;

mkOutput = output: builder:
mergeAny
# prevent override of nested outputs in otherArguments
(optionalAttrs (otherArguments ? ${output}.${system})
{ ${output} = otherArguments.${output}.${system}; })
(optionalAttrs (args ? ${builder})
{ ${output} = args.${builder} pkgs; });
outputs = mapAttrs
(outputName: output: otherArguments.${outputName}.${system} or { } // output)
(outputsBuilder pkgs);
Comment on lines +201 to +203
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried filtering for empty attrsets or null here, but that can cause infinite recursion if any output depends on another.

Copy link
Collaborator

@blaggacao blaggacao May 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can be a little more restrictive and require users to set default* in terms of self.packages.${system}.mypackage instead of provide the possibility of a full blown builder, that receives channels attribute. (Same for apps.)

What do you think?

The implication would be that "defaults" specifically depend on "non-defaults", which semantically kind of makes sense anyhow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is generally a good practice. I have no idea how to implement a restriction for that. If we try and check if something is referring to self we would probably run into infinite recursion.

in
{ inherit pkgs; }
// mkOutput "packages" "packagesBuilder"
// mkOutput "defaultPackage" "defaultPackageBuilder"
// mkOutput "apps" "appsBuilder"
// mkOutput "defaultApp" "defaultAppBuilder"
// mkOutput "devShell" "devShellBuilder"
// mkOutput "checks" "checksBuilder"
{ inherit pkgs; } // outputs
)
# produces attrset in the shape of
# { nixosConfigurations = {}; darwinConfigurations = {}; ... }
Expand Down