minetestWithPackages,minetestPackages: init (minetest bundled with mods and games, package set generated from ContentDB) #194749

fgaz · 2022-10-06T12:36:24Z

Description of changes

Based on #189724 cc @hllizi
Closes #189724
Fixes #82680

This iteration addresses my comments on the previous PR and adds a minetestPackages.contentDB package set generated from https://content.minetest.net

Here's a screenshot obtained by building nixosTests.minetest-with-packages, which runs the package minetestWithPackages (mtps: [ mtps.contentDB.Wuzzy.mineclone2 ])

Things done

.github/CODEOWNERS

fgaz · 2022-10-06T12:38:51Z

pkgs/top-level/all-packages.nix

+  minetestWithPackages = callPackage ../games/minetest/minetest-with-packages.nix { };
+


What is the convention for *withPackages attribute paths? Should it stay at top level? inside minetestPackages? inside minetest?

fgaz · 2022-10-06T13:01:56Z

@GrahamcOfBorg build nixosTests.minetest-with-packages

zowoq · 2022-10-07T00:05:07Z

.editorconfig

@@ -92,3 +92,6 @@ trim_trailing_whitespace = unset

 [pkgs/tools/misc/timidity/timidity.cfg]
 trim_trailing_whitespace = unset
+
+[pkgs/games/minetest/packages/contentdb/default.nix]


The script that generates this file should be fixed so that it doesn't include any whitespace.

Why? Other generated files get the exception, the trailing whitespace does no harm since the file is always regenerated in the same way, and removing it would make the generator script less readable.

The others (only eggs.nix files and hackage-packages.nix) were pre-existing when we added this check and they won't be exemption when we start formatting .nix files.

make the generator script less readable.

Could just format the file which would remove the whitespace.

Could just format the file which would remove the whitespace

That's definitely cleaner, but I'd still like to know why that is useful for a generated file.
In NixOS/rfcs#101 it was decided not to format generated files, and there is still no consensus on the style/formatter to use, so why should we add complexity now, at the risk of having to change the formatter later?

I'd still like to know why that is useful for a generated file.

What benefit is there in excluding files? Isn't it easier if the entire tree is consistent? Currently there are only 5 nix files excluded because of whitespace, I can easily reduce that to just one (haskell package set). If haskell is fixed do you still think this file should to be excluded?

it was decided not to format generated files

Where does it day that?

so why should we add complexity now

Doesn't seem complicated.

at the risk of having to change the formatter later

What does it matter if it needs to be changed?

What benefit is there in excluding files? Isn't it easier if the entire tree is consistent?

I think it isn't really easier for anyone, since nobody is going to manually touch that file.

But...

If haskell is fixed do you still think this file should to be excluded?

Probably not

Where does it day that?

Looks like I can't find it anymore... I only find conflicting info about that now

Doesn't seem complicated.

Fair enough, and this is really a minor thing so I don't know why I was so stubborn...

Fixed. Now the extra whitespaces are filtered out.

zowoq · 2022-10-07T00:07:30Z

The generated file is 1MB! Honestly think this would be better in an external repo.

fgaz · 2022-10-07T14:12:26Z

The generated file is 1MB! Honestly think this would be better in an external repo.

Well, it's not like this is the first or the biggest generated file in nixpkgs:

$ du -h pkgs/development/haskell-modules/hackage-packages.nix
12M     pkgs/development/haskell-modules/hackage-packages.nix
$ find . -type f -name '*.lock' -exec du -ch {} + | grep total$
11M     total

And it compresses well. Then, after this first commit, it's just going to be small diffs.

Still, I could take some steps to further minimize file size:

Sort the packages before writing them out. This hopefully makes compression of multiple update commits even easier, other than improving diffs.
Do not commit the cache file, keep it somewhere else. But...
- Other package sets do have their caches committed. For example we have .yamls for haskellPackages.
- It's nice to be able to regenerate the .nix file just from data present in nixpkgs (that's why I added the -offline option)

Do not have a separate cache file at all, rederive everything from the .nix file.

Can be done with some eval shenanigans:

nix eval --impure --json --expr 'import pkgs/games/minetest/packages/contentdb/default.nix { lib = (import ./. {}).lib; fetchurl = x: x; buildMinetestPackage = x: x; }'

...but that seems a little too complex

I will do the first point, that just makes sense.
I'd rather not touch the cache, but I'll do the second or preferably third point if necessary.

zowoq · 2022-10-07T14:27:38Z

Well, it's not like this is the first or the biggest generated file in nixpkgs:

Doesn't mean we should keep adding them.

Then, after this first commit, it's just going to be small diffs.

Doesn't change that it's still 1MB extra bloat in shallow checkouts.

Why can't it live in an external repo?

fgaz · 2022-10-08T08:34:08Z

Doesn't change that it's still 1MB extra bloat in shallow checkouts.

That's less than 0.5% of nixpkgs, for more than a thousand of FOSS games and game mods. That seems like a good tradeoff to me

Why can't it live in an external repo?

That question applies to every single package or package set though, doesn't it?

And it has been discussed to death:

Autogenerated data in Nixpkgs #15480
[RFC 0109] Nixpkgs Generated Code Policy rfcs#109
nixpkgs is increasing with each Haskell LTS release for 400+ Kb #14897
https://discourse.nixos.org/t/wild-idea-how-about-splitting-nixpkgs-and-nixos/11487?u=fgaz
https://discourse.nixos.org/t/nixpkgss-current-development-workflow-is-not-sustainable/18741?u=fgaz
https://discourse.nixos.org/t/we-need-more-defined-guidelines-for-package-inclusion/3592?u=fgaz
- Answers to the three questions there are yes, yes, and yes
Off topic in pkgs is a mess #107539 starting from pkgs is a mess #107539 (comment)
Probably more

Regarding generated sets, the closest to a conclusion that I got from all those discussions is: Let's wait until flakes are stable to discuss this again, meanwhile generated sets are fine.

Regarding general package inclusion, this package set is updated upstream, will be updated here because it's automated, and is used by many

fgaz · 2022-10-16T16:17:12Z

I explained why I think merging this is reasonable and provided a history of relevant discussions.

Can I at least get some constructive criticism instead of downvotes and laughs?

hllizi · 2022-10-17T08:58:05Z

I explained why I think merging this is reasonable and provided a history of relevant discussions.

Can I at least get some constructive criticism instead of downvotes and laughs?

Seems like I might have downvoted that, too, but that wasn't intentional.

zowoq · 2022-10-17T11:09:34Z

Sorry, meant to post a response.

That's less than 0.5% of nixpkgs

It's already so bloated we should just keep adding more? Keeping a generated 1MB file out of the repo seems like a very easy win with basically no downside, it's not like adding another input is a hassle.

for more than a thousand of FOSS games and game mods

Sound like a perfect example of something that can live in it's own repo.

That seems like a good tradeoff to me

People with bad internet would likely disagree.

… override is plausible.

…variable MINETEST_MOD_PATH.

* add support for games * rename from minetestWithMods * add contentdb package set generator, move existing mods to extra-packages * remove from extra-packages abandoned mods and mods that are in contentdb * general improvements

fgaz · 2022-10-20T11:34:00Z

It's already so bloated we should just keep adding more? Keeping a generated 1MB file out of the repo seems like a very easy win with basically no downside, it's not like adding another input is a hassle.

Adding a new input and keeping it updated is a hassle without the still experimental flakes.
And having a package in nixpkgs has a ton of other advantages, such as discoverability, inclusion in repology, and archival of sources in software heritage.

Sound like a perfect example of something that can live in it's own repo.

Why? As mentioned in #194749 (comment) we have a consensus about including generated sets in nixpkgs. What is different about this set from haskell, emacs, vim, node, r... packages?

People with bad internet would likely disagree.

Trust me, I know how bad modern software can get with a bad internet connection! I had a terrible one until not so long ago. That's why I tried to optimize this as much as possible (it's now down to <1 MB uncompressed, 180K gzipped). In nixpkgs there are multiple packages that are individually more than that size.

There is still something I can do, though I'm not sure if that would be accepted: instead of only having a .nix file as mentioned in #194749 (comment), there could be just a 600K (150K gzipped) .json file. Then the nix code would just be something like

map mkMinetestPackage (importJSON ./packages.json)

fgaz · 2022-10-20T11:42:08Z

pkgs/games/minetest/packages/extra-packages.nix

+    meta = with lib; {
+      description = "Zombies based on TenPlus1's Mob API";
+      license = licenses.mit;
+      maintainers = with maintainers; [ hllizi ];


@hllizi you are not in the nixpkgs maintainer list yet. Are you fine with me adding you or removing the zombies mod?

fgaz · 2022-10-20T11:45:18Z

pkgs/top-level/packages-config.nix

@@ -48,5 +48,15 @@

    # This is an alias which we disallow by default; explicitly allow it
    emacs28Packages = emacs28.pkgs;
+
+    # FIXME: if minetestPackages uses callPackage anywhere, this will not work. Why?


I have no clue why this doesn't work with callPackage, I could use some help here

zowoq · 2022-10-20T12:19:39Z

Adding a new input ... without the still experimental flakes.

I used the word "inputs" but I didn't mean to refer to flakes specifically ... nix-channel, niv, npins, whatever.

keeping it updated is a hassle

Keeping it updated like you have to update nixpkgs anyway? How is that a hassle?

archival of sources in software heritage.

Not sure what this is? If you mean https://github.com/nix-community/nixpkgs-swh it hasn't been working for quite a while.

What is different about this set from haskell, emacs, vim, node, r... packages?

They are already in nixpkgs, this isn't. Haskell and node are used tree-wide by other packages. The others likely be could be external. This package set seems to be very self-contained, hence much easier to be kept out.

Trust me, I know how bad modern software can get with a bad internet connection! I had a terrible one

Would have expected a bit more consideration for the people who won't use this and are getting stuck with the bloat anyway.

until not so long ago.

...

In nixpkgs there are multiple packages that are individually more than that size.

Again, they already in and this isn't. It doesn't mean we should keep adding more, especially when keeping it out doesn't have much downside.

There is still something I can do, though I'm not sure if that would be accepted ...

Plenty of packages and sets do this so I don't see why it would be a problem here. 600k is still a lot of bloat so it doesn't change my objection.

SuperSandro2000 · 2022-10-27T12:10:36Z

The generated file is just twice some node-packages.nix file. Not a fan of these either since they just download, unzip and symlink things.

nyabinary · 2024-12-08T02:49:11Z

@fgaz Are you still planning to work on this?

fgaz · 2024-12-13T17:01:32Z

@nyabinary I plan to push a refinement I had locally on this branch (for archival), then I plan to close this and open a smaller pr that just adds the extension points (with a better design). I'll drop the package set for now. In parallel, I'm working on adding contentdb support to npins

fgaz requested review from alyssais and edolstra as code owners October 6, 2022 12:36

github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: policy discussion labels Oct 6, 2022

fgaz commented Oct 6, 2022

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

fgaz commented Oct 6, 2022

View reviewed changes

fgaz requested review from Mic92 and zowoq as code owners October 6, 2022 12:45

ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Oct 6, 2022

fgaz mentioned this pull request Oct 6, 2022

MineClone 2 #82680

Open

fgaz requested review from PyroLagus and fpletz October 6, 2022 13:18

fgaz added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Oct 6, 2022

zowoq reviewed Oct 7, 2022

View reviewed changes

hllizi added 7 commits October 20, 2022 11:53

minetest-mods: introduced mod-bundling packaged

124f7e5

minetestWithMods: introduced factory for modded minetest.

11f24e7

minetestWithMods: use override pattern

b485c8b

minetestWithMods: only leave as overridable arguments those for which…

1148b3c

… override is plausible.

minetestWithMods: remove abandoned mod.

50afec4

minetestWithMods: trailing whitespace removed.

7629d3d

minetestWithMods: trailing whitespace removed.

6f74f48

hllizi and others added 5 commits October 20, 2022 11:53

minetestWithMods: removed discarded set fields from function call.

5394bf3

minetestWithMods: use --prefix instead of --set with wrapProgram for …

b18e93a

…variable MINETEST_MOD_PATH.

minetestWithMods: use lib.flip

c0b80bb

minetestWithPackages: rename, many changes

b5fe77f

* add support for games * rename from minetestWithMods * add contentdb package set generator, move existing mods to extra-packages * remove from extra-packages abandoned mods and mods that are in contentdb * general improvements

lib.licenses: add afl11, agpl2Only, bsd2FreeBSD

71bf28f

fgaz force-pushed the minetest/mods branch from e56ad98 to 506de24 Compare October 20, 2022 10:16

fgaz added 3 commits October 20, 2022 12:35

minetestPackages.contentDB: populate

1cfbd70

nixosTests.minetest-with-packages: init

00d158e

minetestPackages: expose in packages.json

ec036a7

fgaz force-pushed the minetest/mods branch from 506de24 to ec036a7 Compare October 20, 2022 10:35

fgaz commented Oct 20, 2022

View reviewed changes

wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024

fgaz added the 6.topic: games label May 30, 2024

wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 8, 2024

wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

minetestWithPackages,minetestPackages: init (minetest bundled with mods and games, package set generated from ContentDB) #194749

minetestWithPackages,minetestPackages: init (minetest bundled with mods and games, package set generated from ContentDB) #194749

fgaz commented Oct 6, 2022 •

edited

Loading

fgaz Oct 6, 2022 •

edited

Loading

fgaz commented Oct 6, 2022

zowoq Oct 7, 2022

fgaz Oct 7, 2022

zowoq Oct 7, 2022

fgaz Oct 8, 2022

zowoq Oct 17, 2022

fgaz Oct 20, 2022

zowoq commented Oct 7, 2022

fgaz commented Oct 7, 2022 •

edited

Loading

zowoq commented Oct 7, 2022

fgaz commented Oct 8, 2022 •

edited

Loading

fgaz commented Oct 16, 2022

hllizi commented Oct 17, 2022

zowoq commented Oct 17, 2022 •

edited

Loading

fgaz commented Oct 20, 2022

fgaz Oct 20, 2022

fgaz Oct 20, 2022

zowoq commented Oct 20, 2022

SuperSandro2000 commented Oct 27, 2022

nyabinary commented Dec 8, 2024

fgaz commented Dec 13, 2024

		minetestWithPackages = callPackage ../games/minetest/minetest-with-packages.nix { };

minetestWithPackages,minetestPackages: init (minetest bundled with mods and games, package set generated from ContentDB) #194749

Are you sure you want to change the base?

minetestWithPackages,minetestPackages: init (minetest bundled with mods and games, package set generated from ContentDB) #194749

Conversation

fgaz commented Oct 6, 2022 • edited Loading

Description of changes

Things done

fgaz Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

fgaz commented Oct 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zowoq commented Oct 7, 2022

fgaz commented Oct 7, 2022 • edited Loading

zowoq commented Oct 7, 2022

fgaz commented Oct 8, 2022 • edited Loading

fgaz commented Oct 16, 2022

hllizi commented Oct 17, 2022

zowoq commented Oct 17, 2022 • edited Loading

fgaz commented Oct 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zowoq commented Oct 20, 2022

SuperSandro2000 commented Oct 27, 2022

nyabinary commented Dec 8, 2024

fgaz commented Dec 13, 2024

fgaz commented Oct 6, 2022 •

edited

Loading

fgaz Oct 6, 2022 •

edited

Loading

fgaz commented Oct 7, 2022 •

edited

Loading

fgaz commented Oct 8, 2022 •

edited

Loading

zowoq commented Oct 17, 2022 •

edited

Loading