Skip to content

minecraftServers: move to by-name#512168

Merged
Enzime merged 1 commit into
NixOS:masterfrom
zimward:push-txysqxtosprr
May 19, 2026
Merged

minecraftServers: move to by-name#512168
Enzime merged 1 commit into
NixOS:masterfrom
zimward:push-txysqxtosprr

Conversation

@zimward

@zimward zimward commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

named minecraftServers because the package set is named that

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.

@zimward zimward marked this pull request as draft April 21, 2026 19:36
@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 Apr 21, 2026
@zimward zimward force-pushed the push-txysqxtosprr branch 2 times, most recently from edf20b1 to dae3d9d Compare April 21, 2026 19:55
@nixpkgs-ci nixpkgs-ci Bot added the 6.topic: games Gaming on NixOS label Apr 21, 2026
@phanirithvij

Copy link
Copy Markdown
Member

I think nixpkgs-vet is wrong here. recurseForDerivations works fine with pkgs/by-name.

Comment thread pkgs/top-level/all-packages.nix Outdated
@zimward

zimward commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

I think nixpkgs-vet is wrong here. recurseForDerivations works fine with pkgs/by-name.

yeah there is a open issue for that on nixpkgs-vet

@nixpkgs-ci nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label May 12, 2026
@zimward zimward force-pushed the push-txysqxtosprr branch from dae3d9d to a3ed9a6 Compare May 12, 2026 19:08
@zimward zimward marked this pull request as ready for review May 12, 2026 19:09

@mdaniels5757 mdaniels5757 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Per pkgs/by-name/README.md, "[t]he return value [of each package.nix] must be a derivation". Even if it "works" (and I haven't tested myself) to have non-derivation return values, it breaks assumptions of nixpkgs-vet and the merge bot to have non-derivation package.nix return values, and doing so is also contrary to the design in NixOS/rfcs#140.

You need NixOS/rfcs#197, I think.

@zimward

zimward commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

the readme also documents exporting a set of derivations though which is what i do here

@phanirithvij

Copy link
Copy Markdown
Member

@zimward, you have to refactor this to do what the readme is suggesting for multi version packages.

Instead of exposing an atrrset of packages in package.nix, you need to turn that into a single derivation. And in all-packages.nix you have to do the trick mentioned in the section you linked. So recurseForDerivations removed and the full attrset needs to be refactored.

@phanirithvij

phanirithvij commented May 13, 2026

Copy link
Copy Markdown
Member

Actually this worked

  • moving the package.nix to a versions.nix.
    • no need to remove the lib.recurseIntoAttrs
  • moving the pkgs/by-name/minecraftServers to pkgs/by-name/minecraft-server
    • as minecraft-server is an existing package the new strictDeps and __structuredAttrs are not required to be added.
  • adding a package.nix with {callPackage}: (callPackage ./versions.nix { }).vanilla
    • remove the minecraft-server = minecraftServers.vanilla; in all-packages
    • set minecraftServers = callPackage ../by-name/mi/minecraft-server/versions.nix { };

@phanirithvij phanirithvij self-requested a review May 13, 2026 12:19
@phanirithvij

Copy link
Copy Markdown
Member

@mdaniels5757 please review. It is now passing vet.

@nixpkgs-ci nixpkgs-ci Bot removed the 12.approvals: 1 This PR was reviewed and approved by one person. label May 14, 2026
@nixpkgs-ci nixpkgs-ci Bot added the 12.approvals: 2 This PR was reviewed and approved by two persons. label May 14, 2026
@phanirithvij phanirithvij requested review from drupol and pbsds May 15, 2026 06:28
@Enzime Enzime added this pull request to the merge queue May 19, 2026
Merged via the queue into NixOS:master with commit 261e4d4 May 19, 2026
32 of 34 checks passed
@zimward zimward deleted the push-txysqxtosprr branch June 18, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: games Gaming on NixOS 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: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants