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

treewide: replace substituteAll with replaceVars (part 3) #373659

Merged
merged 1 commit into from
Jan 19, 2025

Conversation

wolfgangwalther
Copy link
Contributor

@wolfgangwalther wolfgangwalther commented Jan 14, 2025

On the road again, driving #237216 forward. Continuation of #365440. Same approach: Auto-generated changes, removed all of those that would require adjustments.

The number of rebuilds is higher here, so targeting staging. But since this PR only contains changes in by-name/, it's easy to generate a build command to target the actually changed packages:

git diff-tree --no-commit-id --name-only HEAD -r | cut -d'/' -f 4 | sed -e 's/^/-A /g' | NIXPKGS_ALLOW_UNFREE=1 xargs nix-build

I ran this on a checkout of the PR, thus building all touched files. We probably need to double-check that none of the changes are behind a conditional which wouldn't be reached that way.

Edit: I tested this on the merge-base of master and staging, but there was a conflict here for one package. Thus rebased on latest staging - unfortunately, this causes about 3k builds for me now. So probably better tested by applying the changes to master first...

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: cinnamon Desktop environment label Jan 14, 2025
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 14, 2025
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 14, 2025
@nix-owners nix-owners bot requested review from mkg20001 and mweinelt January 14, 2025 07:42
Copy link
Contributor Author

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

I was able to rebase on a merge-base between staging and master again, after the latest staging-next iteration merged, with much fewer builds. Hopefully this will not have a merge conflict again, let's see.

Some packages need manual testing, because they are not caught by the test command in the PR description:

  • cmake has a darwin condition - confirmed the patch indeed builds fine there
  • iniparser needs a doCheck=true override - confirmed it works.
  • patchPpdFilesHook needs to test the passthrough tests - confirmed they pass.

@wolfgangwalther wolfgangwalther merged commit 5bbeb6d into NixOS:staging Jan 19, 2025
24 of 27 checks passed
@wolfgangwalther wolfgangwalther deleted the refactor-tractor-3 branch January 19, 2025 15:12
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

I could have sworn I reviewed this. ✅

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

Successfully merging this pull request may close these issues.

2 participants