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

hickory-dns: rename from trust-dns #316466

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

uninsane
Copy link
Contributor

@uninsane uninsane commented Jun 1, 2024

Description of changes

upstream has renamed the project and associated binaries.

this PR is the same change as proposed earlier in #262268, just rebased/updated, so i've kept @oneingan as the author and just listed myself as co-author.

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

@@ -1017,7 +1017,7 @@ Make sure to also check the many updates in the [Nixpkgs library](#sec-release-2

- [trust-dns](https://trust-dns.org/), a Rust based DNS server built to be safe
and secure from the ground up. Available as
[services.trust-dns](#opt-services.trust-dns.enable).
[services.trust-dns](options.html#opt-services.trust-dns.enable).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the release notes don't seem to handle renamed options: the nixos manual was failing to build unless i change the format to this.

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should also mention the rename.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change is required for the manual to build. otherwise:

$ nix-build nixos/release.nix -A manual.x86_64-linux
...
       > RuntimeError: failed to render manual manual.md
       > error:
       >        failed to render manual manual.md
       >
       > caused by:
       >         rendering release-notes/release-notes.md
       >
       > caused by:
       >  rendering release-notes/rl-2311.section.md
       >
       > caused by:
       > bad local reference, id #opt-services.trust-dns.enable not known
       For full logs, run 'nix log /nix/store/bhmzf4h92cghvjf980vn4ypdjh1k69r9-nixos-manual-html.drv'.

this seems to be because mkRenamedOptionModule hides the old option name from the docs. if it's absolutely forbidden to edit the old release notes, then the other way to fix this is to use mkAliasOptionModule instead, which will keep both services.hickory-dns.* and services.trust-dns.* visible in the docs.

it's a weird error. would love some advice if you've seen this before.

Copy link
Contributor

@adamcstephens adamcstephens Jul 15, 2024

Choose a reason for hiding this comment

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

Why are you trying to change the old release notes? I don't know the answer, but would assume that untouched files would not be checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm not trying to change the old release notes: i'd love to not have to. but even a 24.11 nixos configuration with documentation.enable = true will build all the release notes back through 13.10 -- against the 24.11 nixpkgs (because they're referenced in the toplevel release-notes.md). it seems weird to me, but unless i've totally misconfigured my environment, that's the way it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a diff on the 23.11 release notes. Maybe try removing this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the changes to 23.11 and pushed to this PR. ofborg confirms a failure to build the manual: https://github.com/NixOS/nixpkgs/actions/runs/9947857257/job/27481347451?pr=316466

pkgs/applications/networking/browsers/vivaldi/default.nix Outdated Show resolved Hide resolved
@@ -1017,7 +1017,7 @@ Make sure to also check the many updates in the [Nixpkgs library](#sec-release-2

- [trust-dns](https://trust-dns.org/), a Rust based DNS server built to be safe
and secure from the ground up. Available as
[services.trust-dns](#opt-services.trust-dns.enable).
[services.trust-dns](options.html#opt-services.trust-dns.enable).

This comment was marked as outdated.

@uninsane uninsane changed the title trust-dns: rebrand as hickory-dns hickory-dns: rename from trust-dns Jun 30, 2024
@uninsane uninsane force-pushed the rebrand-trust-dns branch 2 times, most recently from baeac78 to f20e1fa Compare June 30, 2024 19:15
@uninsane uninsane force-pushed the rebrand-trust-dns branch 2 times, most recently from cb775b0 to 52a7b95 Compare July 15, 2024 23:01
@uninsane
Copy link
Contributor Author

@ofborg build manual.x86_64-linux

@aktaboot
Copy link
Contributor

any idea on what is the source cause of the manual's failure to build ? thanks

@adamcstephens
Copy link
Contributor

It looks like the manual is passing now, but another rebase is necessary

@uninsane
Copy link
Contributor Author

uninsane commented Aug 7, 2024

it's still passing for me only by changing rl-2311.section.md:

-  [services.trust-dns](#opt-services.trust-dns.enable).
+  [services.trust-dns](options.html#opt-services.trust-dns.enable).

though if you saw it build without that possibly it's an environment thing?

rebased against master, all the same.

@uninsane
Copy link
Contributor Author

uninsane commented Aug 8, 2024

@NickCao looks like you hit a similar option-renaming issue recently: any chance you could advise/sign off on the part of this PR which touches old release notes? seems most of us are unsure what to do about it 😅

(error message here)

@NickCao
Copy link
Member

NickCao commented Aug 8, 2024

@NickCao looks like you hit a similar option-renaming issue recently: any chance you could advise/sign off on the part of this PR which touches old release notes? seems most of us are unsure what to do about it 😅

(error message here)

In my case I simply dropped the link: changing [foo](link) to foo

@uninsane
Copy link
Contributor Author

i've updated the manual part of the patch to

- [services.trust-dns](#opt-services.trust-dns.enable).
+ `services.trust-dns`.

seems like the safest way to fix the build.

@adamcstephens adamcstephens merged commit 0536436 into NixOS:master Aug 11, 2024
26 of 28 checks passed
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.

4 participants