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

nixos-render-docs-redirects: init #357383

Merged
merged 1 commit into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion doc/doc-support/package.nix
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
mkShellNoCC,
documentation-highlighter,
nixos-render-docs,
nixos-render-docs-redirects,
writeShellScriptBin,
nixpkgs ? { },
}:

Expand Down Expand Up @@ -105,8 +107,14 @@ stdenvNoCC.mkDerivation (
buildArgs = "./.";
open = "/share/doc/nixpkgs/manual.html";
};
nixos-render-docs-redirects' = writeShellScriptBin "redirects" "${lib.getExe nixos-render-docs-redirects} --file ${toString ../redirects.json} $@";
in
mkShellNoCC { packages = [ devmode' ]; };
mkShellNoCC {
packages = [
devmode'
nixos-render-docs-redirects'
];
};

tests.manpage-urls = callPackage ../tests/manpage-urls.nix { };
};
Expand Down
6 changes: 5 additions & 1 deletion nixos/doc/manual/shell.nix
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ let
buildArgs = "../../release.nix -A manualHTML.${builtins.currentSystem}";
open = "/${outputPath}/${indexPath}";
};
nixos-render-docs-redirects = pkgs.writeShellScriptBin "redirects" "${pkgs.lib.getExe pkgs.nixos-render-docs-redirects} --file ${toString ./redirects.json} $@";
in
pkgs.mkShellNoCC {
packages = [ devmode ];
packages = [
devmode
nixos-render-docs-redirects
];
}
22 changes: 22 additions & 0 deletions pkgs/by-name/ni/nixos-render-docs-redirects/package.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{ lib, python3 }:

python3.pkgs.buildPythonApplication {
pname = "nixos-render-docs-redirects";
fricklerhandwerk marked this conversation as resolved.
Show resolved Hide resolved
version = "0.0";
pyproject = true;

src = ./src;

build-system = with python3.pkgs; [ setuptools ];

nativeCheckInputs = with python3.pkgs; [
pytestCheckHook
];

meta = {
description = "Redirects manipulation for nixos manuals";
license = lib.licenses.mit;
maintainers = with lib.maintainers; [ getpsyched ];
mainProgram = "redirects";
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import argparse
import json
import sys
from pathlib import Path


def add_content(redirects: dict[str, list[str]], identifier: str, path: str) -> dict[str, list[str]]:
if identifier in redirects:
raise IdentifierExists(identifier)

# Insert the new identifier in alphabetical order
new_redirects = list(redirects.items())
insertion_index = 0
for i, (key, _) in enumerate(new_redirects):
if identifier > key:
insertion_index = i + 1
else:
break
fricklerhandwerk marked this conversation as resolved.
Show resolved Hide resolved
new_redirects.insert(insertion_index, (identifier, [f"{path}#{identifier}"]))
return dict(new_redirects)


def move_content(redirects: dict[str, list[str]], identifier: str, path: str) -> dict[str, list[str]]:
if identifier not in redirects:
raise IdentifierNotFound(identifier)
redirects[identifier].insert(0, f"{path}#{identifier}")
return redirects


def rename_identifier(
redirects: dict[str, list[str]],
old_identifier: str,
new_identifier: str
) -> dict[str, list[str]]:
if old_identifier not in redirects:
raise IdentifierNotFound(old_identifier)
if new_identifier in redirects:
raise IdentifierExists(new_identifier)

# To minimise the diff, we recreate the redirects mapping allowing
# the new key to be updated in-place, preserving the index.
new_redirects = {}
current_path = ""
for key, value in redirects.items():
if key == old_identifier:
new_redirects[new_identifier] = value
current_path = value[0].split('#')[0]
continue
new_redirects[key] = value
new_redirects[new_identifier].insert(0, f"{current_path}#{new_identifier}")
return new_redirects


def remove_and_redirect(
redirects: dict[str, list[str]],
old_identifier: str,
new_identifier: str
) -> dict[str, list[str]]:
if old_identifier not in redirects:
raise IdentifierNotFound(old_identifier)
if new_identifier not in redirects:
raise IdentifierNotFound(new_identifier)
redirects[new_identifier].extend(redirects.pop(old_identifier))
return redirects


def main():
parser = argparse.ArgumentParser(description="redirects manipulation for nixos manuals")
commands = parser.add_subparsers(dest="command", required=True)
parser.add_argument("-f", "--file", type=Path, required=True)

add_content_cmd = commands.add_parser("add-content")
add_content_cmd.add_argument("identifier", type=str)
add_content_cmd.add_argument("path", type=str)

move_content_cmd = commands.add_parser("move-content")
move_content_cmd.add_argument("identifier", type=str)
move_content_cmd.add_argument("path", type=str)

rename_id_cmd = commands.add_parser("rename-identifier")
rename_id_cmd.add_argument("old_identifier", type=str)
rename_id_cmd.add_argument("new_identifier", type=str)

remove_redirect_cmd = commands.add_parser("remove-and-redirect")
remove_redirect_cmd.add_argument("identifier", type=str)
remove_redirect_cmd.add_argument("target_identifier", type=str)

args = parser.parse_args()

with open(args.file) as file:
redirects = json.load(file)

try:
if args.command == "add-content":
redirects = add_content(redirects, args.identifier, args.path)
print(f"Added new identifier: {args.identifier}")

elif args.command == "move-content":
redirects = move_content(redirects, args.identifier, args.path)
print(f"Moved '{args.identifier}' to the new path: {args.path}")

elif args.command == "rename-identifier":
redirects = rename_identifier(redirects, args.old_identifier, args.new_identifier)
print(f"Renamed identifier from {args.old_identifier} to {args.new_identifier}")

elif args.command == "remove-and-redirect":
redirects = remove_and_redirect(redirects, args.identifier, args.target_identifier)
print(f"Redirect from '{args.identifier}' to '{args.target_identifier}' added.")
except Exception as error:
print(error, file=sys.stderr)
else:
with open(args.file, "w") as file:
json.dump(redirects, file, indent=2)
file.write("\n")


class IdentifierExists(Exception):
def __init__(self, identifier: str):
self.identifier = identifier

def __str__(self):
return f"The identifier '{self.identifier}' already exists."


class IdentifierNotFound(Exception):
def __init__(self, identifier: str):
self.identifier = identifier

def __str__(self):
return f"The identifier '{self.identifier}' does not exist in the redirect mapping."
16 changes: 16 additions & 0 deletions pkgs/by-name/ni/nixos-render-docs-redirects/src/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[project]
name = "nixos-render-docs-redirects"
version = "0.0"
description = "redirects manipulation for nixos manuals"
classifiers = [
"Programming Language :: Python :: 3",
"License :: OSI Approved :: MIT License",
"Operating System :: OS Independent",
]

[project.scripts]
redirects = "nixos_render_docs_redirects:main"

[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import unittest
from nixos_render_docs_redirects import (
add_content,
move_content,
rename_identifier,
remove_and_redirect,
IdentifierExists,
IdentifierNotFound,
)


class RedirectsTestCase(unittest.TestCase):
def test_add_content(self):
initial_redirects = {
"bar": ["path/to/bar.html#bar"],
"foo": ["path/to/foo.html#foo"],
}
final_redirects = {
"bar": ["path/to/bar.html#bar"],
"baz": ["path/to/baz.html#baz"],
"foo": ["path/to/foo.html#foo"],
}

result = add_content(initial_redirects, "baz", "path/to/baz.html")
self.assertEqual(list(result.items()), list(final_redirects.items()))

with self.assertRaises(IdentifierExists):
add_content(result, "foo", "another/path.html")


def test_move_content(self):
initial_redirects = {
"foo": ["path/to/foo.html#foo"],
"bar": ["path/to/bar.html#bar"],
}
final_redirects = {
"foo": ["new/path.html#foo", "path/to/foo.html#foo"],
"bar": ["path/to/bar.html#bar"],
}

result = move_content(initial_redirects, "foo", "new/path.html")
self.assertEqual(list(result.items()), list(final_redirects.items()))

with self.assertRaises(IdentifierNotFound):
move_content(result, "baz", "path.html")


def test_rename_identifier(self):
initial_redirects = {
"foo": ["path/to/foo.html#foo"],
"bar": ["path/to/bar.html#bar"],
"baz": ["path/to/baz.html#baz"],
}
final_redirects = {
"foo": ["path/to/foo.html#foo"],
"boo": ["path/to/bar.html#boo", "path/to/bar.html#bar"],
"baz": ["path/to/baz.html#baz"],
}

result = rename_identifier(initial_redirects, "bar", "boo")
self.assertEqual(list(result.items()), list(final_redirects.items()))

with self.assertRaises(IdentifierNotFound):
rename_identifier(result, "bar", "boo")
with self.assertRaises(IdentifierExists):
rename_identifier(result, "boo", "boo")


def test_remove_and_redirect(self):
initial_redirects = {
"foo": ["new/path.html#foo", "path/to/foo.html#foo"],
"bar": ["path/to/bar.html#bar"],
"baz": ["path/to/baz.html#baz"],
}
final_redirects = {
"bar": ["path/to/bar.html#bar", "new/path.html#foo", "path/to/foo.html#foo"],
"baz": ["path/to/baz.html#baz"],
}

result = remove_and_redirect(initial_redirects, "foo", "bar")
self.assertEqual(list(result.items()), list(final_redirects.items()))

with self.assertRaises(IdentifierNotFound):
remove_and_redirect(result, "foo", "bar")
with self.assertRaises(IdentifierNotFound):
remove_and_redirect(initial_redirects, "foo", "baz")
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,49 @@ def __str__(self):

If you moved content, add its new location as the first element of the redirects mapping.
Please update doc/redirects.json or nixos/doc/manual/redirects.json!
""") # TODO: automatically detect if you just missed adding a new location, and make a tool to do that for you
""")
if self.identifiers_without_redirects:
error_messages.append(f"""
Identifiers present in the source must have a mapping in the redirects file.
- {"\n - ".join(self.identifiers_without_redirects)}

This can happen when an identifier was added or renamed.
Please update doc/redirects.json or nixos/doc/manual/redirects.json!
""") # TODO: add tooling in the development shell to do that automatically and point to that command

Added new content?
redirects add-content ❬identifier❭ ❬path❭

Moved existing content to a different output path?
redirects move-content ❬identifier❭ ❬path❭

Renamed existing identifiers?
redirects rename-identifier ❬old-identifier❭ ❬new-identifier❭

Removed content? Redirect to alternatives or relevant release notes.
redirects remove-and-redirect ❬identifier❭ ❬target-identifier❭

Note that you need to run `nix-shell doc` or `nix-shell nixos/doc/manual` to be able to run this command.
fricklerhandwerk marked this conversation as resolved.
Show resolved Hide resolved
""")
if self.orphan_identifiers:
error_messages.append(f"""
Keys of the redirects mapping must correspond to some identifier in the source.
- {"\n - ".join(self.orphan_identifiers)}

This can happen when an identifier was removed or renamed.
Please update doc/redirects.json or nixos/doc/manual/redirects.json!
""") # TODO: add tooling in the development shell to do that automatically and point to that command

Added new content?
redirects add-content ❬identifier❭ ❬path❭

Moved existing content to a different output path?
redirects move-content ❬identifier❭ ❬path❭

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk Nov 27, 2024

Choose a reason for hiding this comment

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

Found an unspecified requirement: nrd will complain if you rename an identifier but don't also rename references. It would be very helpful if this helper tool also did something akin to

sed -i 's/<old>/<new>/g' **/*

but that needs a bit of smarts, because we have to capture old identifiers correctly (e.g. naively inserting foo and foo2 after already having renamed the "tag" location of foo but not the "ref" locations, will produce foo22 at the "tag").

So this is not blocking for this PR. You would get the identifier errors by nrd already, so this change doesn't make it strictly worse. It may mess with expectations though, because one may think such a command will just do the Right Thing for you already.

Renamed existing identifiers?
redirects rename-identifier ❬old-identifier❭ ❬new-identifier❭

Removed content? (good for redirecting deprecations to new content or release notes)
redirects remove-and-redirect ❬identifier❭ ❬target-identifier❭

Note that you need to run `nix-shell doc` or `nix-shell nixos/doc/manual` to be able to run this command.
""")

error_messages.append("NOTE: If your Manual build passes locally and you see this message in CI, you probably need a rebase.")
return "\n".join(error_messages)
Expand Down