Skip to content

python3Packages: remove most --replace usages #420365

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

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

TomaSajt
Copy link
Contributor

@TomaSajt TomaSajt commented Jun 26, 2025

Tracking: #356002

This was made semi-manually.

I tried building all packages I changed while I was working on the PR, and I am fairly certain they'll build fine.

The methodology was:

  • Replace --replace with --replace-fail in around ~10 packages.
  • Try building them
    • If the package is marked as broken or disabled on all currently supported python3 versions, I just skip it
      • I also skip if the package is not marked as broken, but still failed
    • If there is no rebuild triggered, it is probably behind an isDarwin conditional.
      • (In those cases I made sure it worked by forcing the conditional and checking manually)
  • In case the substitution fails, check if it makes sense to remove it
    • In most cases, it makes sense. I check those manually to make sure it's fine to remove
    • In a the case of blivet, I opted to use --replace-quiet instead
    • For pyu2f I had to separate out a substituteInPlace into two parts
    • In some cases non-trivial changes are needed, I just skipped those
  • After the 10 packages (or less if they were skipped), are deemed to build fine with the changes, git stash them, so that they don't cause rebuilds in the next iteration of 10 packages

Here's a script that only checks out the changes
to the file that the package is defined in and then builds it.
It takes a few hours to run and will generate a failed.txt file (which will hopefully be empty).

Do note that some changes are behind optional flags, so this check is not 100% thorough.

#!/usr/bin/env bash

set -eou pipefail

# put the branch name here
# or just the git revision
branch="python-replace"

changed_files=$(git diff --name-only "$branch"~ "$branch")

git co "$branch"~
git reset "$branch"~ --hard

echo "The following packages failed to build:" > ./failed.txt

for file in $changed_files; do

  stripped=${file#pkgs/development/python-modules/}

  if [[ "$stripped" == "capstone/4.nix" ]]; then
    attrname="capstone_4"
  elif [[ "$stripped" == "numpy/1.nix" ]]; then
    attrname="numpy_1"
  elif [[ "$stripped" == "django/4.nix" ]]; then
    attrname="django_4"
  elif [[ "$stripped" == "torch/source/default.nix" ]]; then
    attrname="torch"
  elif [[ "$stripped" == "pytidylib/default.nix" ]]; then
    attrname="tidylib"
  elif [[ "$stripped" == "sudachidict/default.nix" ]]; then
    attrname="sudachidict-core"
  else
    attrname=$(dirname "$stripped")
  fi

  case "$attrname" in
    "deepwave"|"mip")
      # I didn't want to wait out the long tests...
      continue
      ;;
    "bacpypes"|"bwapy"|"pytest-cram")
      # disabled for higher versions
      python_pkgs="python311Packages"
      ;;
    "blockchain"|"certipy-ad"|"dataclasses-serialization"|"gradient-utils"|\
    "gradient"|"nampa"|"nipype"|"numpy_1"|"palace"|"pyrevolve"|"update-dotdee")
      # disabled for higher versions
      python_pkgs="python312Packages"
      ;;
    *)
      python_pkgs="python313Packages"
      ;;
  esac

  to_build="$python_pkgs"."$attrname"

  # Pull in the changes for one specific file
  git checkout "$branch" -- "$file"

  echo "Building python package: $attrname"
  if ! nix-build -A "$to_build"; then
    echo "Build failed for $attrname"
    echo "$attrname" >> ./failed.txt
  fi

  git reset "$branch"~ --hard
done

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 6.topic: python Python is a high-level, general-purpose programming language. labels Jun 26, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab and removed 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. labels Jun 28, 2025
@TomaSajt TomaSajt force-pushed the python-replace branch 6 times, most recently from 9ae2be4 to 62b4630 Compare June 28, 2025 13:46
@TomaSajt TomaSajt changed the title treewide: python3Packages: use --replace-fail instead of --replace and remove unneeded substitutions python3Packages: remove most --replace usages Jun 28, 2025
@TomaSajt TomaSajt changed the base branch from master to staging June 28, 2025 13:48
@nixpkgs-ci nixpkgs-ci bot closed this Jun 28, 2025
@nixpkgs-ci nixpkgs-ci bot reopened this Jun 28, 2025
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

I went through the diff and found nothing unexpected.
Assuming everything still builds fine, we should be able to merge this PR.

Thanks for doing this cleaning job!

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jun 28, 2025
@GaetanLepage GaetanLepage requested a review from drupol June 28, 2025 15:36
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

Nice cleanup, thanks for doing it!

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Jun 28, 2025
@GaetanLepage GaetanLepage requested a review from JohnRTitor June 28, 2025 20:35
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 6, 2025
@JohnRTitor
Copy link
Member

JohnRTitor commented Jul 6, 2025

To be honest this is a bit hard to review, can someone confirm if all of the "changed" packages build fine? Nix-Community builders can be used for this purpose.

Another solution would be to split this into multiple chunks.

@TomaSajt
Copy link
Contributor Author

TomaSajt commented Jul 6, 2025

I'll whip up a script with which you can test out the changes one by one, so as to avoid the many rebuilds.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 6, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 7, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 8, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 15, 2025
@TomaSajt TomaSajt marked this pull request as draft July 24, 2025 23:44
@NixOS NixOS deleted a comment from nix-owners bot Jul 24, 2025
@TomaSajt TomaSajt marked this pull request as ready for review July 25, 2025 02:24
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 25, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 12, 2025
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 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants