Conversation
|
Can you add a minimal test for the new functionality?
We would never want this to silently regress...
|
|
@flokli you mean for systemd-homed? first I have to understad it :-). I'm currently trying to rebuild my pc, and then figure out how homed works. |
|
Should I add |
|
Suggestion: add a |
|
@kloenk is this ready for review? We might want to degrade this to a draft PR until it has proven to work, and has a test ensuring it stays that way ;-) |
|
Oh, yeah. It's a draft. |
|
I think the way to continue with this should be:
|
I installed that systemd on my main machine xD. |
|
I'd really prefer if we'd first address the PAM issues mentioned by Archlinux, and include a test, before shipping this in master. |
|
PAM configuration are a NixOS Module concern no? This PR does not do
anything related to NixOS modules so far; and I think the point of Doron
was that if the systemd package has homed support; then it's easier for
others to contribute a working NixOS module (including the correct PAM
configuration).
…On Mon, Nov 2, 2020 at 7:03 PM Florian Klink ***@***.***> wrote:
I'd *really* prefer if we'd first address the PAM issues mentioned by
Archlinux, and include a test, before shipping this in master.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#98299 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEZNI3MFA7L7MLRPNP4AC3SN3X5LANCNFSM4RTIF2WA>
.
|
|
I agree on including this as a package. The module can be contributed later, and those who feel brave enough could try the new tech on their machines or VMs and help contributing the aforementioned module. I would totally install this on a VM for testing! |
|
Can we add a That'd mean we won't ship untested features enabled by default, but people can get a systemd with homed, without rebuilding the world, by slightly overriding |
|
Pity draft PRs don't get their conflict status updated. This certainly has cosmetic conflicts with #101886. |
|
Also, do we have an issue on our end for the PAM debacle? |
|
Will try to rebase and recheck tomorrow |
|
Rebased, not yet tested if it builds and works |
|
I successfully built both |
flokli
left a comment
There was a problem hiding this comment.
This LGTM. This should unblock some more testing and toying with it without too many world rebuilds :-) Happy to flip the default once there's a module system integration and a VM test.
Ericson2314
left a comment
There was a problem hiding this comment.
OK applied my nit-pick and then cleanued up history to separate the bootstrapping changes from the changes to systemd proper. Looks all good to me now!
|
⛵🚀
|
doronbehar
left a comment
There was a problem hiding this comment.
Looks good to me besides that comment typo.
This is disabled by default to indicate that is hasn't been adiquately tested with NixOS yet.
Motivation for this change
Fixes #91243
Contains #98998 (merged)
Things done
Please also see #72802 (comment)
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)CC: @Mic92 (changed your systemd patch)