Skip to content

various: migrate applications/audio to by-name/ part 2#454032

Merged
wolfgangwalther merged 32 commits intoNixOS:masterfrom
pancaek:by-name-audio2
Oct 22, 2025
Merged

various: migrate applications/audio to by-name/ part 2#454032
wolfgangwalther merged 32 commits intoNixOS:masterfrom
pancaek:by-name-audio2

Conversation

@pancaek
Copy link
Contributor

@pancaek pancaek commented Oct 21, 2025

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@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. labels Oct 21, 2025
@pancaek pancaek changed the title various: migrate applications/audio to by-name/ various: migrate applications/audio to by-name/ part 2 Oct 21, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 21, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 21, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 21, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 21, 2025
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 21, 2025
@pancaek pancaek force-pushed the by-name-audio2 branch 3 times, most recently from 142db71 to 860fc89 Compare October 21, 2025 05:10
Copy link
Contributor

Choose a reason for hiding this comment

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

The commit message in mbrola(-voices): move to by-name/ is wrong - you need to use {} instead of () for the right expansion syntax, so that ofborg can pick this up correctly.

(have not checked the other commit messages)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I missed that the same commit message needs a comma, too:

-mbrola{-voices}: move to by-name/
+mbrola{,-voices}: move to by-name/

@nixpkgs-ci nixpkgs-ci bot added 2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 22, 2025
@pancaek
Copy link
Contributor Author

pancaek commented Oct 22, 2025

(sorry about the rebases, merge conflict)

I think this is actually ready, I did want to check in about a couple of the packages I moved though. Was hoping to be able to migrate the whole thing here, but oh well.

  • raysession
  • pithos
  • greg
  • patchance
  • petrifoo

I was a little unsure if these violated this paragraph about by-name/

Note that callPackage definitions in all-packages.nix with custom arguments should not be removed. That is a backwards-incompatible change because it changes the .override interface. Such packages may still be moved to pkgs/by-name however, in order to avoid the slightly superficial choice of directory / category in which the default.nix file was placed, but please keep the definition in all-packages.nix using callPackage. See also changing implicit attribute defaults.

I can adjust those ones more if needed, but I had a hard time following that section

@wolfgangwalther
Copy link
Contributor

I was a little unsure if these violated this paragraph about by-name/

Note that callPackage definitions in all-packages.nix with custom arguments should not be removed. That is a backwards-incompatible change because it changes the .override interface. Such packages may still be moved to pkgs/by-name however, in order to avoid the slightly superficial choice of directory / category in which the default.nix file was placed, but please keep the definition in all-packages.nix using callPackage. See also changing implicit attribute defaults.

I can adjust those ones more if needed, but I had a hard time following that section

This advice will be outdated very soon, when we merge #454147. So it's fine to either not deal with these right now or to migrate them to a form where no callPackage remains in all-packages.nix.

Copy link
Contributor

@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.

Just the nit about the commit message.

@wolfgangwalther
Copy link
Contributor

(commit message is not really relevant, because there are no rebuilds anyway - so doesn't matter that ofborg can't pick it up!)


espeak-ng = callPackage ../applications/audio/espeak-ng { };
espeak = res.espeak-ng;
espeak = espeak-ng;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be in aliases

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sure about which of the two should be the alias.

I am 100% sure that this out of scope for this PR, though. This was not introduced here.

Feel free to open a PR to improve it!

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

Labels

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. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants