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

nixos/guix: init #264331

Merged
merged 3 commits into from Dec 3, 2023
Merged

nixos/guix: init #264331

merged 3 commits into from Dec 3, 2023

Conversation

foo-dogsquared
Copy link
Member

Description of changes

Now that #246975 is merged, at least why not make a service module for it. While some parts of the integration test is rendered redundant once the package enables checking (e.g., doCheck = true), I guess it's better than nothing.

cc @cafkafk

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@foo-dogsquared
Copy link
Member Author

There are still some setups I have to figure out so it's in draft for now.

Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

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

Ohh nice work, can't wait to try this out!

Maybe since you've done most of the work on guix you'd want to add yourself to the maintainers on the guix package as well?

nixos/doc/manual/release-notes/rl-2311.section.md Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
@h7x4
Copy link
Member

h7x4 commented Oct 30, 2023

Note that there is also #264323. Maybe we could try to combine the efforts somehow? @viperML

nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
@foo-dogsquared
Copy link
Member Author

Maybe since you've done most of the work on guix you'd want to add yourself to the maintainers on the guix package as well?

Sure. That's fine by me.

Note that there is also #264323. Maybe we could try to combine the efforts somehow?

I'm open for ideas. (I find it amusing both PRs are drafted just within an hour)

@viperML
Copy link
Contributor

viperML commented Oct 30, 2023

Well my draft intended to open the discussion for the options, but it seems you already have the module ready.

@foo-dogsquared
Copy link
Member Author

Well my draft intended to open the discussion for the options, but it seems you already have the module ready.

I only upstreamed the module from my Guix overlay and haven't updated or gave the module options a second look before this PR so there's still room for discussion.

@foo-dogsquared
Copy link
Member Author

A few test setups later and it is done. Should be ready for review.

@foo-dogsquared foo-dogsquared marked this pull request as ready for review November 1, 2023 07:40
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/2857

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Please read through the descriptions and make sure they are written in a style suitable for a manual.
Also please check that the comments explain the reason something is done and don't just elaborately describe the line below with little extra information.

nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
nixos/modules/services/misc/guix/default.nix Outdated Show resolved Hide resolved
@viperML
Copy link
Contributor

viperML commented Nov 2, 2023

  • The nix-daemon.nix uses unitConfig.RequiresMountsFor = "/nix/store"; , should we do the same for /gnu/store ?

  • And more of a general guix usage question, how are channels handled? The package already comes with some channels bundled?

@foo-dogsquared
Copy link
Member Author

Please read through the descriptions and make sure they are written in a style suitable for a manual. Also please check that the comments explain the reason something is done and don't just elaborately describe the line below with little extra information.

Yeah, I neglected checking for the documentation on this one. My bad.

Will keep that in mind for future modules.

@foo-dogsquared
Copy link
Member Author

  • The nix-daemon.nix uses unitConfig.RequiresMountsFor = "/nix/store"; , should we do the same for /gnu/store ?

Good point. Will update the unit with those.

  • And more of a general guix usage question, how are channels handled? The package already comes with some channels bundled?

If you meant managing custom Guix channels, this module does not handle Guix channels. You should be able to handle them as described in the manual. As for the package, it doesn't bundle any channels.

This is in preparation for the Nix module where it will allow the user
to set custom store and state directory.
@ofborg ofborg bot requested a review from cafkafk December 1, 2023 15:49
@wegank wegank merged commit 4e81387 into NixOS:master Dec 3, 2023
23 checks passed
@foo-dogsquared foo-dogsquared deleted the add-nixos-guix-module branch December 4, 2023 02:27
@hpfr
Copy link

hpfr commented Dec 7, 2023

Since this is a new module (and some new options for the Guix package that appear to match the previous baked-in values), could this be backported to 23.11? I recently started using your overlay repo (thanks!) and just noticed this PR that barely missed the release (and #246975 did make it). I was planning to switch to stable NixOS now that I can try moving my home environment to Guix.

@wegank wegank added the backport release-23.11 Backport PR automatically label Dec 7, 2023
Copy link
Contributor

github-actions bot commented Dec 7, 2023

Backport failed for release-23.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-23.11
git worktree add -d .worktree/backport-264331-to-release-23.11 origin/release-23.11
cd .worktree/backport-264331-to-release-23.11
git switch --create backport-264331-to-release-23.11
git cherry-pick -x df46b418951994499f8b6c4cc12db65f807ea50f 092aaf841806e8a5fcdfea566bbaae2e25cea069 ad277ea47e17e3073ba61af07284d1dff8d1601e

@foo-dogsquared
Copy link
Member Author

This has been backported to 23.11 (#272632). Now, it's a matter of waiting for it to land in nixos-23.11 branch (or whatever stable branch you're using).


Also I didn't know you can backport module additions. Nice.

@wegank
Copy link
Member

wegank commented Dec 7, 2023

Yes, new modules are backwards-compatible changes.

Meanwhile, this makes the change in the 24.05 release note obsolete.

@jian-lin
Copy link
Contributor

Meanwhile, this makes the change in the 24.05 release note obsolete.

Could you elaborate the meaning of this and what action should be taken for

- [Guix](https://guix.gnu.org), a functional package manager inspired by Nix. Available as [services.guix](#opt-services.guix.enable).
.

@wegank
Copy link
Member

wegank commented Dec 12, 2023

I'd like to drop this line and, if possible, move it to the 23.11 release note.

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

Successfully merging this pull request may close these issues.

None yet

9 participants