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

Jump to bottom

feat: add overlays to flake.nix #218

Merged
merged 3 commits into from
Nov 1, 2023
Merged

feat: add overlays to flake.nix #218

merged 3 commits into from
Nov 1, 2023

Conversation

musjj
Copy link
Contributor

@musjj musjj commented Sep 24, 2023

Having overlays makes it easier to install on NixOS systems.

@sxyazi
Copy link
Owner

sxyazi commented Sep 24, 2023 •
edited
Loading

cc @XYenon @teto

Could you please review this PR, I've completely forgotten my limited Nix knowledge lol 🤣

Ah @musjj, almost forgot, thanks for contributing the PR. =)

teto
teto reviewed Sep 25, 2023
View reviewed changes
flake.nix Outdated Show resolved Hide resolved
teto
teto reviewed Sep 25, 2023
View reviewed changes
flake.nix Outdated
overlays = {
default = overlays.yazi;
yazi = final: prev: {
yazi = final.callPackage ./nix/yazi.nix { };
Copy link

Choose a reason for hiding this comment

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

safer to just inherit yazi here to have a single source of truth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't re-use the yazi inside eachDefaultSystem because it's already instantiated into a specific architecture. An overlay should not be dependent on a specific architecture and accept whatever nixpkgs that was passed to it.

@teto
Copy link

teto commented Sep 25, 2023

you can install yazi.packages.${system}.yazi, how does the overlay make it easier, aka you can just import the overlay without modifying the rest of your config ?

@musjj
Copy link
Contributor Author

musjj commented Sep 25, 2023

In flakes, you actually can't access the current system architecture AFAIK. So you have to use workarounds by passing custom args from flake.nix into your configuration, or just hardcoding it. Most other repo flakes I've seen provide overlays which seems like a cleaner and more convenient solution.

@sxyazi
Copy link
Owner

sxyazi commented Oct 16, 2023

I'm cleaning up PRs, looks like this PR is just one step away.

@teto, would you like to give it a review? If it's ready, feel free to ping me anytime to get it merged! 😉

XYenon
XYenon reviewed Oct 17, 2023
View reviewed changes
flake.nix Outdated Show resolved Hide resolved
teto
teto reviewed Oct 18, 2023
View reviewed changes
flake.nix Show resolved Hide resolved
@sxyazi sxyazi force-pushed the main branch 2 times, most recently from 9e2cfcd to 6a651f4 Compare October 23, 2023 23:36
@sxyazi sxyazi merged commit f9fbe72 into sxyazi:main Nov 1, 2023
@sxyazi
Copy link
Owner

sxyazi commented Nov 1, 2023

Thank you all, merged! 🎉

@branchvincent branchvincent mentioned this pull request Jan 15, 2024
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@teto teto teto left review comments

@XYenon XYenon XYenon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@musjj @sxyazi @teto @XYenon