-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Prepare inclusion in nixos-search #2971
Conversation
f420bf3
to
8d75e43
Compare
I am happy to see this initiative but why mixing it with format changes ? can these go in another MR ? Is the following expected ?
|
Nope, that's a bug in nixos-search. |
arf nmd issue:
|
Ah, I guess I didn't run into that because that path was substituted by the nix-community cache. So we should probably add nmd and nmt to the flake inputs instead of having an IFD (which I did initially but then I thought it wasn't needed). |
176833a
to
bfb27a7
Compare
Should be fixed now |
Add a lockfile to work around NixOS/nix#6541 (and because it's a good idea anyway). Also use flake-utils, and restrict ourselves to the five platforms supported by nixpkgs. Otherwise, the IFD for nmd fails on weird platforms. This fixes `nix flake check`. Remove the redundant `apps` output, see nix-community#2442 (comment)
Make sure the option is included in the NixOS/nix-darwin manual (but the HM submodule options aren't). Also add a static description to the HM submodule type so that we don't need to evaluate the submodules just to build the option manual. This makes nixos-search able to index the home-manager flake. Also clean up some TODOs.
This avoids having to use `pkgs.fetchFromGitLab` in an IFD, which causes issues when indexing packages with nixos-search because `pkgs` is instantiated with every platform.
bfb27a7
to
5b98520
Compare
Thanks for the quick merge! (EDIT: wait, it's been 18 days? where does time go...) @rycee quick heads-up since I don't know how familiar you are with flakes: inputs like nmd and nmt can now be updated with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about Flakes so I'll defer to @teto's judgement about merging. I just added a few minor comments that I didn't see discussed, probably because they are too blindingly obvious for people who aren't ignorant about Flakes 🙂
flake = (import | ||
(let lock = builtins.fromJSON (builtins.readFile ./flake.lock); | ||
in fetchTarball { | ||
url = | ||
"https://github.com/edolstra/flake-compat/archive/${lock.nodes.flake-compat.locked.rev}.tar.gz"; | ||
sha256 = lock.nodes.flake-compat.locked.narHash; | ||
}) { src = ./.; }).defaultNix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels pretty ugly to me. Why not simply change the pkgs.fetchFromGitLab
to fetchTarball
when fetching nmd and nmt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only a compatibility layer that should progressively become useless as we transition to flakes and the new nix CLI.
Why not simply change the
pkgs.fetchFromGitLab
tofetchTarball
when fetching nmd and nmt?
We could, but if we're going to use flakes, we might as well use them fully, i.e. for all inputs. I think they're a nicer way to manage dependencies, and they make updating and overriding easier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that eventually it will be worth switching over to flakes but I'm skeptical about delving too deep while the feature is marked as experimental. At least unless there is a really compelling reason. But in this case there is a straight forward way to avoid the pkgs reference without touching flakes and needing compatibility shims.
I also would like to move the nmt and nmd projects over to sourcehut at some point and it doesn't seem like the flake-compat project supports the sourcehut URLs. I'll probably maintain a mirror at the gitlab location so this is not a blocker, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also not a blocker because we can just use URLs like git+https://git.sr.ht/~rycee/nmt
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm skeptical about delving too deep while the feature is marked as experimental.
It is only marked as experimental to not have to create backwards compatibility while it still might change.
"nixpkgs": { | ||
"locked": { | ||
"lastModified": 1654230545, | ||
"narHash": "sha256-8Vlwf0x8ow6pPOK2a04bT+pxIeRnM1+O0Xv9/CuDzRs=", | ||
"owner": "nixos", | ||
"repo": "nixpkgs", | ||
"rev": "236cc2971ac72acd90f0ae3a797f9f83098b17ec", | ||
"type": "github" | ||
}, | ||
"original": { | ||
"owner": "nixos", | ||
"ref": "nixos-unstable", | ||
"repo": "nixpkgs", | ||
"type": "github" | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want it to be explicit to the user which Nixpkgs they are using in their configuration. I.e., HM should never be responsible for pinning Nixpkgs on behalf of users. So if we are pinning Nixpkgs here wouldn't we also need to change
pkgs ? builtins.getAttr system nixpkgs.outputs.legacyPackages
to simply
pkgs
in the homeManagerConfiguration
function?
Ideally, perhaps disallowing
home-manager.url = "github:nix-community/home-manager";
without an accompanying
home-manager.inputs.nixpkgs.follows = "nixpkgs";
would be most reassuring but I don't know if that is possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if we are pinning Nixpkgs here wouldn't we also need to change
Agreed, I'll do that in a follow-up PR.
Ideally, perhaps disallowing
That's not possible, and I think it would be overkill: we only need to ensure that the nixpkgs
input is not passed down to the HM modules, but we may (and should) use the locked nixpkgs
for flake packages.
Prevents Home Manager from pinning nixpkgs on behalf of users, see nix-community#2971 (comment)
Prevents Home Manager from pinning nixpkgs on behalf of users, see nix-community#2971 (comment)
Prevents Home Manager from pinning nixpkgs on behalf of users, see nix-community#2971 (comment)
Prevents Home Manager from pinning Nixpkgs on behalf of users, see nix-community#2971 (comment)
so what is now the way to pass nixpkgs? It is not clear to me at all. I tried passing The docs for flakes haven't been updated yet. |
See #3037 for the docs update |
* Add flake.lock and clean up flake.nix Add a lockfile to work around NixOS/nix#6541 (and because it's a good idea anyway). Also use flake-utils, and restrict ourselves to the five platforms supported by nixpkgs. Otherwise, the IFD for nmd fails on weird platforms. This fixes `nix flake check`. Remove the redundant `apps` output, see nix-community#2442 (comment) * nixos,nix-darwin: factor out into a common module * nixos,nix-darwin: make `home-managers.users` shallowly visible Make sure the option is included in the NixOS/nix-darwin manual (but the HM submodule options aren't). Also add a static description to the HM submodule type so that we don't need to evaluate the submodules just to build the option manual. This makes nixos-search able to index the home-manager flake. Also clean up some TODOs. * flake: add nmd and nmt This avoids having to use `pkgs.fetchFromGitLab` in an IFD, which causes issues when indexing packages with nixos-search because `pkgs` is instantiated with every platform.
Prevents Home Manager from pinning Nixpkgs on behalf of users, see nix-community#2971 (comment)
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/the-nix-module-in-home-manager-and-nixos/20999/11 |
* Add flake.lock and clean up flake.nix Add a lockfile to work around NixOS/nix#6541 (and because it's a good idea anyway). Also use flake-utils, and restrict ourselves to the five platforms supported by nixpkgs. Otherwise, the IFD for nmd fails on weird platforms. This fixes `nix flake check`. Remove the redundant `apps` output, see nix-community#2442 (comment) * nixos,nix-darwin: factor out into a common module * nixos,nix-darwin: make `home-managers.users` shallowly visible Make sure the option is included in the NixOS/nix-darwin manual (but the HM submodule options aren't). Also add a static description to the HM submodule type so that we don't need to evaluate the submodules just to build the option manual. This makes nixos-search able to index the home-manager flake. Also clean up some TODOs. * flake: add nmd and nmt This avoids having to use `pkgs.fetchFromGitLab` in an IFD, which causes issues when indexing packages with nixos-search because `pkgs` is instantiated with every platform.
Prevents Home Manager from pinning Nixpkgs on behalf of users, see nix-community#2971 (comment)
Description
Lays the groundwork for NixOS/nixos-search#323 by making
succeed. Note that this PR does NOT solve the problem of how to expose Home Manager modules/options in the flake, it only ensures that we can index packages and NixOS options.
Best reviewed one commit at a time.
This PR partly supersedes #1934, although it does not add an auto-update workflow.
Tested the NixOS module and the manual. nix-darwin testers welcome.
Checklist
Change is backwards compatible.
Code formatted with
./format
.Code tested through
nix-shell --pure tests -A run.all
.Test cases updated/added. See example.
Commit messages are formatted like
See CONTRIBUTING for more information and recent commit messages for examples.