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

Build unstable manuals #270

Closed
wants to merge 2 commits into from
Closed

Build unstable manuals #270

wants to merge 2 commits into from

Conversation

oxij
Copy link
Member

@oxij oxij commented Mar 27, 2019

@@ -100,7 +100,8 @@
[% ELSIF menu == 'nixpkgs' %]
<ul class="nav pull-left">
<li><a href="[%root%]nixpkgs/download.html">Download</a></li>
<li><a href="[%root%]nixpkgs/manual">Manual</a></li>
<li><a href="[%root%]nixpkgs/manual">Manual ([%latestNixOSSeries%])</a></li>
<li><a href="[%root%]nixpkgs/manual/unstable">Manual (unstable)</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

The Manual HTML generation is done in bootstrapify-docbook.sh, and it doesn't have the nixos-release.tt in its pre-process chain, which will cause it to lack the current release for the stable manual link

image

The following diff includes the nix-release.tt file too, but probably isn't required.

diff --git a/bootstrapify-docbook.sh b/bootstrapify-docbook.sh
index f53788f..9475441 100755
--- a/bootstrapify-docbook.sh
+++ b/bootstrapify-docbook.sh
@@ -20,6 +20,7 @@ mkdir -p "$outDirTmp"
         xsltproc --nonet bootstrapify-docbook.xsl "$inDir/$fn" > "$outDirTmp/$fn.in"
         root=$(realpath --relative-to="$(pwd)" "$outDirTmp/$fn" | sed -e 's|[^/]||g' -e 's|/|../|g')
         tpage --pre_chomp --post_chomp \
+            --pre_process=nix-release.tt --pre_process=nixos-release.tt \
             --define root="${root}" \
             --pre_process=common.tt \
             > "$outDirTmp/$fn" <<EOF

@@ -11,6 +11,8 @@
the list of configuration options by running <tt>man
configuration.nix</tt>. You can also <a href="options.html">search
the list of options online</a>.</p>

<p>There is also <a href="[%nixosManual%]/unstable/">a version for the <strong>nixos-unstable</strong> channel</a>.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Most of the file is soft-wrapped at around 75 chars. One simple newline would be welcome (imo).

diff --git a/nixos/support.tt b/nixos/support.tt
index b6b6763..5ccb27d 100644
--- a/nixos/support.tt
+++ b/nixos/support.tt
@@ -12,7 +12,8 @@
     configuration.nix</tt>. You can also <a href="options.html">search
     the list of options online</a>.</p>
 
-    <p>There is also <a href="[%nixosManual%]/unstable/">a version for the <strong>nixos-unstable</strong> channel</a>.</p>
+    <p>There is also <a href="[%nixosManual%]/unstable/">a version for the
+	<strong>nixos-unstable</strong> channel</a>.</p>
   </div>
 
   <div class="span4" id="discourse">

./bootstrapify-docbook.sh $(NIXPKGS_MANUAL_IN)/unstable/share/doc/nixpkgs $(NIXPKGS_MANUAL_OUT)/unstable 'Nixpkgs manual' nixpkgs https://github.com/NixOS/nixpkgs/tree/master/doc
ln -sfn manual.html $(NIXPKGS_MANUAL_OUT)/unstable/index.html
# alias for stable
ln -sfn stable/index.html $(NIXPKGS_MANUAL_OUT)/index.html
Copy link
Member

Choose a reason for hiding this comment

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

This will orphan a https://nixos.org/nixpkgs/manual/manual.html page, that seems to be un-indexed, but still will exist.

“But there's an rm!” or so I thought initially, but it's a no-op print of rm -rf; @echo is an unintuitive construct in Makefiles. (And I'm not targeting you, this seems borrowed from another target.)

What about fixing the rm -f? I'm not sure it's a good idea, this will, periodically (every twenty minutes or so) cause the manuals to briefly vanish from the server while they're being generated. Tried it locally and it's trivial to see it; this will end up confusing users, I think.

What should be done? Not sure. About the orphaned manual.html file? Maybe manually remove it. Should we mv manual.html index.html instead of symlinking to prevent unintended files to be present? Could we generate into a temp dir and as-atomically-as-possible pivot the manual to it instead of rm ing for the shortest amount of time the manuals?

Same comments about rm applies to the nixos manual, except there isn't a manual.html file there.

@samueldr
Copy link
Member

In addition to the review comments, I'm not sure the nixos.org/*/manual/[index.html] -> nixos.org/*/manual/stable/[index.html] symlink is entirely good. I may be overthinking URLs, but I'm thinking this could potentially cause issues where this is seen as duplicated content on the site. It'd be better if this could be handled through a permanent HTTP redirection, since IMO having a /stable and /unstable discrete location is good to convey which manual exactly it is; it might slightly reduce confusion as to why stuf on master isn't in the /stable manual.

Though, the annoying bit is how HTTP redirections aren't handled through configuration in this repo; it would need to be handled in the nixos org configurations, and might get even more involved to implement if/when the website is ever moved directly on S3.

@garbas garbas added this to Confirmed in TODO Mar 29, 2020
@samueldr
Copy link
Member

With #505 done, I think this can be closed.

If anything is missing, feel free to say.

@samueldr samueldr closed this Aug 28, 2020
@garbas garbas moved this from Confirmed to Done in TODO Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
TODO
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants