-
-
Notifications
You must be signed in to change notification settings - Fork 477
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
Add networking.hosts and .hostFiles from nixos #939
base: master
Are you sure you want to change the base?
Conversation
Possible duplicate of #807? (my PR is older and doesn't have tests though, so maybe this should be preferred) |
Sorry @willemml I missed your PR. I didn’t intend to duplicate work. |
This PR seems fine but is blocked as a review is required. How can we help to get it merged? |
Oh this would be awesome :) |
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.
Can you rebase this PR?
851b145
to
274963a
Compare
@Enzime rebasing is done. I tried to do the hash thing but I think I got it wrong. I did |
It looks right to me:
|
However the file in |
@emilazy good catch! Indeed: Without the newline: $ sha256sum doc/known-files/c7dd0e2ed261ce76d76f852596c5b54026b9a894fa481381ffd399b556c0e2da
1b6fb3e08d41ae31b6bfe1f66fecf3ef53e302047993f6235570bc9391f291bb doc/known-files/c7dd0e2ed261ce76d76f852596c5b54026b9a894fa481381ffd399b556c0e2da With: $ sha256sum doc/known-files/c7dd0e2ed261ce76d76f852596c5b54026b9a894fa481381ffd399b556c0e2da
c7dd0e2ed261ce76d76f852596c5b54026b9a894fa481381ffd399b556c0e2da doc/known-files/c7dd0e2ed261ce76d76f852596c5b54026b9a894fa481381ffd399b556c0e2da |
Looks like the tests are broken now |
@Enzime indeed 😞 I have two questions related to the two failures I see. How can I fix the I looked and looked at the test output and can’t see the error printed out. How can I run the tests locally if I use flakes? I tried a few command line incantations but I can’t seem to make it right. |
The ability to run the tests through a flake interface will be added in #1140, until that's merged you can run your test locally with I've accessed the GitHub runner through tmate (I suspect only maintainers have access but I'm not sure, it's the last step in the
I suspect that every GitHub Mac will have a different nix-darwin/.github/workflows/test.yml Lines 143 to 146 in 98e7dba
|
Also if you could rebase this PR that would be great, let us know if you're still interested in working on this PR |
Probably simplest to just move the config files out of the way rather than hashing them. |
In the case of |
I'd love this functionality, any ETA on finalizing this PR and merging it, please? |
There hasn't been @ibizaman in 3 weeks so if you're interested in picking up the work, feel free to take the changes in this PR and rebase them on top of |
I'm afraid I'm not at the skill level of debugging and developing nix components at this point, just a happy user. |
Sorry about the hiatus. I’ll pick this up this week. But of course I don’t mind if someone beats me to it 😛 |
2f5b93f
to
fbc45cf
Compare
Well it seems I did a bad job rebasing 😅 more rebasing incoming. |
fbc45cf
to
00274c8
Compare
00274c8
to
a65c078
Compare
a65c078
to
3161f9b
Compare
I'm not sure where the issue comes from. I tried to reproduce locally but I don't have more log output. Any idea? |
tests/networking-hosts.nix
Outdated
cat $file | ||
echo "'$(grep '127.0.0.1' $file | head -n1)'" |
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.
cat $file | |
echo "'$(grep '127.0.0.1' $file | head -n1)'" |
Should be unnecessary with set -v
, not sure if it'll output the value that file
gets set to or if you need to use set -x
instead
tests/networking-hosts.nix
Outdated
echo "'$(grep '127.0.0.1' $file | head -n1)'" | ||
exit 1 | ||
fi | ||
if [[ ! $(grep '127.0.0.1' $file | tail -n1) =~ my.super.host$ ]]; then |
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.
if [[ ! $(grep '127.0.0.1' $file | tail -n1) =~ my.super.host$ ]]; then | |
grep '127.0.0.1' $file | tail -n1 | grep 'my.super.host$' |
set -e
should be enabled so if you can construct these as a single chain of commands then if statements shouldn't necessary
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 verified and indeed if I meddle with the second grep, like replacing host
with hst
, the test fails.
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.
Can you squash all the commits into one commit with a commit message like:
networking: add `hosts` option for managing `/etc/hosts`
if ! grep '127.0.0.1' $file | head -n1 | grep localhost$; then | ||
exit 1 | ||
fi |
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.
if ! grep '127.0.0.1' $file | head -n1 | grep localhost$; then | |
exit 1 | |
fi | |
grep '127.0.0.1' $file | head -n1 | grep localhost$ |
The if !; then exit
isn't necessary
|
||
environment.etc.hosts = { | ||
knownSha256Hashes = [ | ||
"c7dd0e2ed261ce76d76f852596c5b54026b9a894fa481381ffd399b556c0e2da" |
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.
Can you add a comment explaining where you got this file from, like the macOS version you're running
etcHostsHash=$(shasum -a 256 /etc/hosts | cut -d ' ' -f 1) | ||
/usr/bin/sed -i.bak \ | ||
"s/# programs.fish.enable = true;/nix.settings.access-tokens = [ \"github.com=\${{ secrets.GITHUB_TOKEN }}\" ]; environment.etc.\"nix\/nix.conf\".knownSha256Hashes = [ \"$nixConfHash\" ]; environment.etc.hosts.knownSha256Hashes = [ \"$etcHostsHash\" ];/" \ |
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.
Looks like these commands should be indented more
The comment in the default
does make me a little apprehensive about symlinking this into place instead of copying it. |
Almost there! |
This code was taken nearly verbatim from
https://github.com/NixOS/nixpkgs/blob/nixos-unstable/nixos/modules/config/networking.nix
The few changes were related to making the default
/etc/hosts
match the Apple's stock one. This implied no127.0.0.2
and forcing the IPV6::1
entry.