Skip to content

Conversation

@fricklerhandwerk
Copy link
Collaborator

this removes more manuals steps from adapting the site to new releases.

@nixos/documentation-team please bikeshed whether to generate pages at all, as this introduces some mental overhead for working with the site.

depends on #830

@fricklerhandwerk fricklerhandwerk requested a review from a team as a code owner January 15, 2024 13:46
@fricklerhandwerk fricklerhandwerk force-pushed the nix-manual-dynamic-index branch from cfc2652 to 8a71272 Compare January 15, 2024 13:49
@DanielSidhion
Copy link
Member

I looked at the changes and they seem ok, I don't know what you were referring to when you said "whether to generate pages at all" (what's the alternative? Not have any links to the nix manuals from nix.dev?).

However, I've been thinking. Is it possible to grab the nix version directly from each channel during CI? By that I mean, can we do a few "niv add " in CI and then for each channel, run nix --version and grab the version from there, so we (ideally) don't need to do any manual intervention at all?

@fricklerhandwerk fricklerhandwerk added the site Improvements to the site infrastructure or content presentation label Jan 27, 2024
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

I looked at the changes and they seem ok, I don't know what you were referring to when you said "whether to generate pages at all" (what's the alternative? Not have any links to the nix manuals from nix.dev?).

Emphasis is on generate. The alternative would be to not merge this PR, which then requires manually updating the Nix versions in the .md file whenever stable/unstable changes.

I think it's good to have this, automation is key!

Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Oh just noticed a problem with this: If we update Nix versions, we'd be invalidating old version-specific links like https://nix.dev/manual/nix/2.18/.... So we should never do minor version updates for niv entries, but rather add new ones.

@fricklerhandwerk
Copy link
Collaborator Author

fricklerhandwerk commented Feb 12, 2024

We could also link to stable uniformly, or even expand such links to the actual version at build time. The rabbit hole of content generation is as deep as any Turing tar pit.

@fricklerhandwerk fricklerhandwerk force-pushed the nix-manual-dynamic-index branch from b6c585f to 926f8fa Compare March 2, 2024 23:52
@infinisil infinisil mentioned this pull request Mar 6, 2024
CONTRIBUTING.md Outdated
Comment on lines 39 to 43
- On each new Nix release, update the `nix-latest` to the corresponding release branch:

1. Add the latest version in [`default.nix`](./default.nix).
For example, to add Nix 2.19:

```bash
niv add nixos/nix -n nix_2-19 -b 2.19-maintenance
```

2. Reference the latest version in [`source/reference/nix-manual.md`](./source/reference/nix-manual.md).
```bash
niv update nix-latest -b 2.20-maintenance
```
Copy link
Member

Choose a reason for hiding this comment

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

My above previous concern is still valid: This would break version-specific links, that's a no-go.

Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid #863 (comment) wasn't a good suggestion after all

@fricklerhandwerk fricklerhandwerk force-pushed the nix-manual-dynamic-index branch 3 times, most recently from f165e98 to 098b8dd Compare March 16, 2024 00:35
- refactor `default.nix` for hopefully better readability
- de-noise the overview page text
before that, the script was semantically wrong as it pinned to the tip
of the branch.
this will accumulate releases over time. it's not great, but this is the
price of persistent URLs. otherwise, when people link to supposedly stable
URLs, and we garbage-collect old manuals, those links will rot away.

it's also slightly simpler algorithmically, and therefore easier to
reason about.

this can be incrementally improved in the future, by automatically
adding redirects to e.g. the closest following release for which the
manual is still served. for example, if we currently serve

    23.11
    23.05
    22.11
    22.05

but in the future decide to cut that list to the first two, the script
would drop the excess pins and add redirects like these:

    /manual/nixpkgs/22.05/* /manual/nixpkgs/23.05/:splat 301
    /manual/nixpkgs/22.11/* /manual/nixpkgs/23.05/:splat 301

the condition for that would be that we are reasonably sure that the
manuals will manage their own internal redirects as pages and anchors
move around between releases.
this change also places all historical Nix manuals under a stable URL
@fricklerhandwerk
Copy link
Collaborator Author

It works now and is even somewhat readable:

$ ls $(nix-build -A build --no-out-link)/manual/nix
2.10  2.11  2.12  2.13  2.14  2.15  2.16  2.17  2.18  2.19  2.20  2.21  2.4  2.5  2.6  2.7  2.8  2.9

Extra goodie: The Nix manual is again (optionally) available as single page (ping @ehmry)!

image

@fricklerhandwerk fricklerhandwerk force-pushed the nix-manual-dynamic-index branch from 98bae7c to e837576 Compare April 3, 2024 12:01
- Use separate JSON files to track sources for Nixpkgs and Nix releases
  This greatly simplifies the code, because we can directly encode
  versions as keys without ambiguity
- Avoid an instance of IFD for the redirect generation
- Use pkgs.substitute instead of builtins.replaceStrings
- Turn off the single-page feature for now. It was added by Valentin in
  a previous commit, but I think we should discuss this a bit more
- Simplify a lot of the code and add comments
- Fix the mutable redirects, they were broken by a parent commit
- Remove pieces of Nix code that weren't used, like the import of the
  Nixpkgs manual. This can be added in the future when necessary
- Make sure that Nix versions are cached by building from the store path
@infinisil infinisil force-pushed the nix-manual-dynamic-index branch from 4daecbe to f479463 Compare April 4, 2024 00:05
@infinisil
Copy link
Member

I'm afraid the code felt very cobbled together overall.. I kind of refactored the entire code now 😅. Please check the commit message for more details, but this PR looks good to me now. I tested it locally with netlify dev -d result, including the redirects. I'm also pretty sure that all Nix versions are cached now, I don't have to build any anymore.

Extra goodie: The Nix manual is again (optionally) available as single page (ping @ehmry)!

I reverted that for now, though I left it in the code:

nix.dev/default.nix

Lines 54 to 56 in 4daecbe

# TODO: This needs to be looked into more carefully before enabling:
# This is a hacky way to get a single-page manual from mdBook's print feature:
# sed -z 's|\s*window\.addEventListener(\x27load\x27, function() {\s*window\.setTimeout(window.print, 100);\s*});||g' ${nix.doc}/share/doc/nix/manual/print.html > $out/manual/nix/${version}/sp.html

This feels way too hacky for me, it should be discussed a bit more. And there's no reason this has to be part of this PR, let's not make it bigger than it already is!

@fricklerhandwerk fricklerhandwerk enabled auto-merge (squash) April 5, 2024 15:21
@fricklerhandwerk
Copy link
Collaborator Author

@infinisil thanks a lot for the review pass, it's quite solid now. I'll open a new PR for single-page rendering.

@infinisil infinisil disabled auto-merge April 5, 2024 15:36
@infinisil infinisil enabled auto-merge April 5, 2024 15:36
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Whoops fat fingers, though it looks like auto-merge doesn't do much when somebody still requested changes, I guess I'll have to approve :)

@infinisil infinisil merged commit c3e0585 into NixOS:master Apr 5, 2024
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/tweag-nix-dev-update-56/43035/1

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

Labels

site Improvements to the site infrastructure or content presentation

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants