Skip to content

treewide: migrate python3Packages with toPythonApplication to by-name#454559

Closed
qweered wants to merge 2 commits intoNixOS:masterfrom
qweered:migrate-python-all-packages
Closed

treewide: migrate python3Packages with toPythonApplication to by-name#454559
qweered wants to merge 2 commits intoNixOS:masterfrom
qweered:migrate-python-all-packages

Conversation

@qweered
Copy link
Contributor

@qweered qweered commented Oct 22, 2025

I have used this script:

#!/usr/bin/env nix-shell
#!nix-shell -i python3 -p python3

import argparse
import os
import re
from pathlib import Path


RE_PATTERN = re.compile(
    r"^(?P<indent>\s*)(?P<name>[A-Za-z0-9+_.\-]+)\s*=\s*"
    r"(?:"
    r"with\s+(?P<ns_with>python(?:[0-9]+)?(?:Packages|\.pkgs));\s*toPythonApplication\s+(?P<pkg_with>[A-Za-z0-9+_.\-]+)"
    r"|"
    r"(?P<ns_direct>python(?:[0-9]+)?(?:Packages|\.pkgs))\.toPythonApplication\s+(?P=ns_direct)\.(?P<pkg_direct>[A-Za-z0-9+_.\-]+)"
    r");\s*(?:#.*)?$"
)


def by_name_prefix(name: str) -> str:
    if len(name) >= 2:
        return name[:2]
    return (name + "_")[:2]


def build_package_nix_content(python_pkg_name: str, python_namespace: str) -> str:
    return f"""{{ lib, {python_namespace}, ... }}@args:

let
  inherit ({python_namespace}.{python_pkg_name}) override;
in
{python_namespace}.toPythonApplication (
  override (
    removeAttrs args [
      "lib"
      "{python_namespace}"
    ]
    // lib.intersectAttrs (lib.functionArgs override) args
  )
)
"""


def find_matches(lines):
    for idx, line in enumerate(lines):
        m = RE_PATTERN.match(line)
        if not m:
            continue
        yield idx, m


def ensure_package_file(
    repo_root: Path,
    attr_name: str,
    python_pkg_name: str,
    python_namespace: str,
    apply: bool,
    overwrite_existing: bool,
) -> Path:
    prefix = by_name_prefix(attr_name)
    dest_dir = repo_root / "pkgs" / "by-name" / prefix / attr_name
    dest_dir.mkdir(parents=True, exist_ok=True)
    dest_file = dest_dir / "package.nix"
    if apply:
        desired = build_package_nix_content(python_pkg_name, python_namespace)
        if dest_file.exists():
            if overwrite_existing:
                dest_file.write_text(desired)
        else:
            dest_file.write_text(desired)
    return dest_file


def process(
    repo_root: Path,
    apply: bool,
    only: set[str] | None,
    allow_name_mismatch: bool,
    overwrite_existing: bool,
) -> int:
    all_packages = repo_root / "pkgs" / "top-level" / "all-packages.nix"
    if not all_packages.exists():
        raise SystemExit(f"File not found: {all_packages}")

    original = all_packages.read_text().splitlines(keepends=True)

    changes = []
    delete_indices: set[int] = set()
    for idx, match in find_matches(original):
        name = match.group("name")
        if match.group("ns_with"):
            ns = match.group("ns_with")
            pkg = match.group("pkg_with")
        else:
            ns = match.group("ns_direct")
            pkg = match.group("pkg_direct")

        if ns.endswith(".pkgs"):
            ns = ns.replace(".pkgs", "Packages")
        if only and name not in only:
            continue
        if not allow_name_mismatch and name != pkg:
            continue
        dest_file = ensure_package_file(
            repo_root, name, pkg, ns, apply, overwrite_existing
        )
        delete_indices.add(idx)
        changes.append((idx + 1, name, pkg, dest_file))  # 1-based line numbers for display

    if apply and changes:
        # Remove the identified lines entirely (no blank placeholders)
        modified = [line for i, line in enumerate(original) if i not in delete_indices]
        all_packages.write_text("".join(modified))

    # Report
    if not changes:
        print("No eligible python3Packages.toPythonApplication entries found.")
    else:
        action = "Would migrate & delete" if not apply else "Migrated & deleted"
        for line_no, name, pkg, dest in changes:
            suffix = "" if name == pkg else f" (python package: {pkg})"
            print(f"{action}: {name}{suffix} (line {line_no}) -> {dest}")
    return 0


def main():
    parser = argparse.ArgumentParser(
        description=(
            "Migrate lines of the form 'name = with pythonNNPackages; toPythonApplication name;'\n"
            "from pkgs/top-level/all-packages.nix into pkgs/by-name/<pr>/<name>/package.nix and\n"
            "delete the corresponding line in all-packages.nix (by-name is auto-included).\n"
            "Default is a dry run."
        )
    )
    parser.add_argument(
        "--apply",
        action="store_true",
        help="Write changes to disk (create package.nix files and edit all-packages.nix).",
    )
    parser.add_argument(
        "--only",
        nargs="*",
        help="Limit migration to the given attribute names (space-separated).",
    )
    parser.add_argument(
        "--allow-name-mismatch",
        action="store_true",
        help=(
            "Migrate entries even when attribute name differs from the python package name. "
            "The by-name path uses the attribute name; the package.nix content references the python package."
        ),
    )
    parser.add_argument(
        "--overwrite-existing",
        action="store_true",
        help=(
            "If a destination package.nix already exists, overwrite it with the canonical content."
        ),
    )
    args = parser.parse_args()

    repo_root = Path(__file__).resolve().parent
    only = set(args.only) if args.only else None
    raise SystemExit(
        process(
            repo_root,
            args.apply,
            only,
            args.allow_name_mismatch,
            args.overwrite_existing,
        )
    )


if __name__ == "__main__":
    main()

Add a 👍 reaction to pull requests you find important.

@qweered qweered marked this pull request as draft October 22, 2025 14:01
@qweered qweered force-pushed the migrate-python-all-packages branch 4 times, most recently from 632a453 to 47eb907 Compare October 22, 2025 14:44
@Prince213
Copy link
Member

Feels like you missed a few characters in the title.

@qweered qweered changed the title treewide: migrate python3Packages.toPytonAppication usages to by-name treewide: migrate python3Packages.toPytonAppication packages to by-name Oct 22, 2025
@qweered qweered force-pushed the migrate-python-all-packages branch from 47eb907 to 15196b0 Compare October 22, 2025 14:54
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 6.topic: python Python is a high-level, general-purpose programming language. labels Oct 22, 2025
@qweered
Copy link
Contributor Author

qweered commented Oct 22, 2025

Done
Coulid shrink them to one-liners like

{ python3Packages }: with python3Packages; toPythonApplication ble-serial

but i don't think it worth it

@qweered qweered force-pushed the migrate-python-all-packages branch from 15196b0 to bd310a6 Compare October 22, 2025 15:00
@qweered qweered marked this pull request as ready for review October 22, 2025 15:00
@Prince213
Copy link
Member

Still not right, should be toPythonApplication instead of toPytonAppication.

@qweered qweered changed the title treewide: migrate python3Packages.toPytonAppication packages to by-name treewide: migrate python3Packages with toPytonAppication to by-name Oct 22, 2025
@qweered qweered changed the title treewide: migrate python3Packages with toPytonAppication to by-name treewide: migrate python3Packages with toPythonApplication to by-name Oct 22, 2025
@qweered qweered force-pushed the migrate-python-all-packages branch from bd310a6 to f38c618 Compare October 22, 2025 15:03
@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Oct 22, 2025
@qweered
Copy link
Contributor Author

qweered commented Oct 22, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 454559
Commit: f38c618332bc472e3db58a39b8fa58a175d037eb


x86_64-linux

❌ 8 packages failed to build:
  • python312Packages.mmcv
  • python312Packages.mmcv.dist
  • python312Packages.mmengine
  • python312Packages.mmengine.dist
  • python313Packages.mmcv
  • python313Packages.mmcv.dist
  • python313Packages.mmengine
  • python313Packages.mmengine.dist
✅ 14 packages built:
  • dvc (python313Packages.dvc)
  • dvc-with-remotes
  • dvc-with-remotes.dist
  • dvc.dist (python313Packages.dvc.dist)
  • python312Packages.dvc
  • python312Packages.dvc-hdfs
  • python312Packages.dvc-hdfs.dist
  • python312Packages.dvc.dist
  • python312Packages.dvclive
  • python312Packages.dvclive.dist
  • python313Packages.dvc-hdfs
  • python313Packages.dvc-hdfs.dist
  • python313Packages.dvclive
  • python313Packages.dvclive.dist

@qweered
Copy link
Contributor Author

qweered commented Oct 22, 2025

mmcv and mmengiine are failing on master too. lgtm ;)

@qweered
Copy link
Contributor Author

qweered commented Oct 24, 2025

Done Coulid shrink them to one-liners like

{ python3Packages }: with python3Packages; toPythonApplication ble-serial

but i don't think it worth it

Decided to do that, -2 lines for 400 packages seems big enough and thats how they were done in all-packages.nix

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 24, 2025
@qweered qweered force-pushed the migrate-python-all-packages branch 2 times, most recently from 10ed6c4 to d2dea14 Compare October 24, 2025 05:33
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 24, 2025
@qweered
Copy link
Contributor Author

qweered commented Dec 3, 2025

What if we use what @emilazy sugggests for immideate improvement and migration from top-level?

{ lib, python3Packages, ... }@args:

let
  inherit (python3Packages.somePkg) override;
in

python3Packages.toPythonApplication (
  override (
    removeAttrs args [
      "lib"
      "python3Packages"
    ]
    // lib.intersectAttrs (lib.functionArgs override) args
  )
)

Then we can implement proper solution

@dotlambda
Copy link
Member

dotlambda commented Dec 3, 2025

I don't see why intersectAttrs should be used. That would silence errors about unexpected arguments.

EDIT: Sorry, duplicate of #454559 (comment)

@qweered qweered force-pushed the migrate-python-all-packages branch from feb86dd to eb8266d Compare December 3, 2025 22:24
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 3, 2025
@qweered qweered force-pushed the migrate-python-all-packages branch from eb8266d to 57a05d2 Compare December 3, 2025 22:37
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 6.topic: python Python is a high-level, general-purpose programming language. labels Dec 3, 2025
@qweered qweered force-pushed the migrate-python-all-packages branch from 57a05d2 to 247071c Compare December 3, 2025 22:43
@qweered qweered marked this pull request as ready for review December 3, 2025 22:43
Copilot AI review requested due to automatic review settings December 3, 2025 22:43

This comment was marked as spam.

@qweered qweered force-pushed the migrate-python-all-packages branch from 247071c to a685b06 Compare December 3, 2025 22:47
@qweered qweered force-pushed the migrate-python-all-packages branch from a685b06 to 065def5 Compare December 3, 2025 22:51
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 5, 2025
@wolfgangwalther
Copy link
Contributor

I'm afraid it just doesn't make sense to change these at this stage. We first need to solve the .override problem for by-name in general.

@qweered
Copy link
Contributor Author

qweered commented Dec 12, 2025

Putting this on hold until better solution

@qweered
Copy link
Contributor Author

qweered commented Dec 24, 2025

I propose different solution in #473624 for now

@qweered qweered closed this Dec 24, 2025
@qweered qweered deleted the migrate-python-all-packages branch December 24, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants