[bug#55703] Update minetest

Hello!

Le dimanche 29 mai 2022 à 15:48 +0200, Liliana Marie Prikler a écrit :
> Am Sonntag, dem 29.05.2022 um 14:47 +0200 schrieb Vivien Kraus:
> > Subject: [PATCH v1 1/8] gnu: minetest: Update to 5.5.1.
> > 
> > * gnu/local.mk (dist_patch_DATA): Remove minetest-add-
> > MINETEST_MOD_PATH.patch.
> > * gnu/packages/patches/minetest-add-MINETEST_MOD_PATH.patch: Delete
> > it.
> > * gnu/packages/minetest.scm (irrlichtmt): New variable.
> > (minetest): Update to 5.5.1.
> > [patches]: Remove patch.
> > [configure-flags]: find irrlichtmt and zstd.
> > [inputs]: Replace irrlicht with irrlichtmt, add zstd.
> > (minetest-data): Update hash.
> I'd name "irrlichtmt" to "irrlicht-for-minetest" and perhaps split
> this
> patch into two.  Even if they need to be bumped "at once" later, I
> don't think this holds for the initial introduction.

I renamed the fork, and I split the commit as: introduce the fork, and
then (update minetest + use irrlicht-for-minetest). If I split the
minetest update commit further, it will create a broken commit. I was
told on #guix that I should not create such commits. Quoting nckx:

    vivien: No, each commit should result in a sane state whenever
possible.

> > * gnu/packages/minetest.scm (minetest-basic-materials): Update to
> > 2022-03-28 (commet 9d55f991…).
> > [snippet]: Make sound_api_core a dependency, not a submodule.
> Again doing two things at once.  I think it'd be wiser to first do
> the
> updates, then add minetest-sound-core, then add the snippets.  WDYT?

minetest-sound-core is introduced as a submodule for the
basic_materials update. So the code for it is not present at all. If I
first update basic_materials, then the tests will fail because it can’t
find sound_core. Do you mean that I should first try to respect the
will of the author and add it as a submodule (I don’t know how to do
that) and then take it out as a separate package?

> ¹ I did not check for hash mismatches or ContenDB version
> equivalence.

I just looked up the date and found a the commit in github that was
there (usually contentdb reports the next day as the github commit). Is
there something more to do?

> ² As pointed out by Maxime elsewhere in the mailing list, Minetest
> packages usually have flaky tags in their forges, so someone needs to
> look closer at whether this is going to break in the future.

Yes. Mesecons was using version numbers and then the latest tag is a
date. Given that it’s April 1st, maybe there’s something funny occuring
here, but the changes around that day looked reasonable to my untrained
eye. However, one thing I didn’t notice at first was the drop of the +
in the license. Sorry.

Vivien

Hi,

Am Sonntag, dem 29.05.2022 um 17:43 +0200 schrieb Vivien Kraus:
> Hello!
> 
> Le dimanche 29 mai 2022 à 15:48 +0200, Liliana Marie Prikler a écrit :
> > Am Sonntag, dem 29.05.2022 um 14:47 +0200 schrieb Vivien Kraus:
> > > Subject: [PATCH v1 1/8] gnu: minetest: Update to 5.5.1.
> > > 
> > > * gnu/local.mk (dist_patch_DATA): Remove minetest-add-
> > > MINETEST_MOD_PATH.patch.
> > > * gnu/packages/patches/minetest-add-MINETEST_MOD_PATH.patch: Delete
> > > it.
> > > * gnu/packages/minetest.scm (irrlichtmt): New variable.
> > > (minetest): Update to 5.5.1.
> > > [patches]: Remove patch.
> > > [configure-flags]: find irrlichtmt and zstd.
> > > [inputs]: Replace irrlicht with irrlichtmt, add zstd.
> > > (minetest-data): Update hash.
> > I'd name "irrlichtmt" to "irrlicht-for-minetest" and perhaps split
> > this patch into two.  Even if they need to be bumped "at once" later,
> > I don't think this holds for the initial introduction.
> 
> I renamed the fork, and I split the commit as: introduce the fork, and
> then (update minetest + use irrlicht-for-minetest). If I split the
> minetest update commit further, it will create a broken commit. I was
> told on #guix that I should not create such commits. Quoting nckx:
> 
>     vivien: No, each commit should result in a sane state whenever
> possible.
That's exactly the split I intended, thanks.

> +    (source
> +     (origin
> +       (inherit (package-source irrlicht))
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/minetest/irrlicht")
> +             (commit version)))
> +       (sha256
> +        (base32
> +         "1jxk1x0f60n8lrz8a6x62aj2pqg0qnbajsld3lqncvwsfbi0xjx1"))
> +       (patches '())
> +       (snippet '(begin #t))))
Instead of inheriting and blanking patches and snippet, you could
simply not inherit.

> +    (arguments
> +     `(#:tests? #f))
As with Irrlicht itself, you should write out "; no check target"

> > > * gnu/packages/minetest.scm (minetest-basic-materials): Update to
> > > 2022-03-28 (commet 9d55f991…).
> > > [snippet]: Make sound_api_core a dependency, not a submodule.
> > Again doing two things at once.  I think it'd be wiser to first do
> > the updates, then add minetest-sound-core, then add the snippets. 
> > WDYT?
> 
> minetest-sound-core is introduced as a submodule for the
> basic_materials update.  So the code for it is not present at all.  If
> I first update basic_materials, then the tests will fail because it
> can’t find sound_core. Do you mean that I should first try to respect
> the will of the author and add it as a submodule (I don’t know how to
> do that) and then take it out as a separate package?
Hmm, perhaps I was confused by the commit message.  If changes to the
package are necessary for the bump, it makes sense to do them in the
same commit as you did, but the wording would imply that it was
previously a submodule, which perhaps through strange git magic is no
longer available in the source (because it has been externalized).

As far as I can see however, minetest-sound-core is a slightly
different kind of package though.  Like gnulib or GNOME's libgd, the
authors want it to be included as source code in the target mod.  I
don't know how this makes a difference in Minetest mods, but there is a
chance it might.  At the very least, it does not appear as though
minetest-sound-core is itself a mod (or is it?).

> > ¹ I did not check for hash mismatches or ContenDB version
> > equivalence.
> 
> I just looked up the date and found a the commit in github that was
> there (usually contentdb reports the next day as the github commit).
> Is there something more to do?
Ideally, you would check the JSON for the release, e.g.
https://content.minetest.net/api/packages/VanessaE/homedecor_modpack/releases/12307/

@Maxime, looking at the result it'd be possible to fetch sources from
contentdb.  Should we implement contentdb-fetch?

> > ² As pointed out by Maxime elsewhere in the mailing list,
> > Minetest packages usually have flaky tags in their forges, so
> > someone needs to look closer at whether this is going to break in
> > the future.
> 
> Yes. Mesecons was using version numbers and then the latest tag is a
> date. Given that it’s April 1st, maybe there’s something funny
> occuring here, but the changes around that day looked reasonable to
> my untrained eye. However, one thing I didn’t notice at first was the
> drop of the + in the license. Sorry.
I'm not sure whether that's the right version to bump to, sorry. 
According to ContentDB the latest release is
27c3c515b49af91c1dbc427f31a820722854eb24, but that's untagged in git; I
suggest either making a git-version and adding an appropriate comment
or contacting upstream for clarification.

Cheers

Liliana Marie Prikler schreef op zo 29-05-2022 om 18:39 [+0200]:
> @Maxime, looking at the result it'd be possible to fetch sources from
> contentdb.  Should we implement contentdb-fetch?

I don't think a separate fetch method is required, url-fetch owuld
suffice.  From <https://content.minetest.net/help/api/>:

  You can download a package by building one of the two URLs:

  https://content.minetest.net/packages/${author}/${name}/download/`
  https://content.minetest.net/packages/${author}/${name}/releases/${release}/download/`

However, this makes --with-commit=... & --with-branch=... not work
so there are some downsides.  I also don't know if these zips are
stable, would need to be investigated.  Maybe worth it, dunno.

Greetings,
Maxime.

Liliana Marie Prikler schreef op zo 29-05-2022 om 18:39 [+0200]:
> As far as I can see however, minetest-sound-core is a slightly
> different kind of package though.  Like gnulib or GNOME's libgd, the
> authors want it to be included as source code in the target mod.

IMO those need to be unbundled as well because of standard reasons for
unbundling.  Though for gnulib there is the complication that some
gnulib-using software makes some important modifications to their
gnulib (e.g. guile modifies lib/localcharset.c).

> I don't know how this makes a difference in Minetest mods, but there is a
> chance it might.  At the very least, it does not appear as though
> minetest-sound-core is itself a mod (or is it?).

mod.conf is missing, so it's not a mod by itself, from Minetest's
perspective (*).

Some differences though:

There exist (non-bundled) API mods, e.g. 
<https://content.minetest.net/packages/TestificateMods/climate_api/>.
Minetest also has a standard and well-functioning package manager.

As such, it seems to me that it could conceivably be turned into an API
mod.  Maybe if we explain nicely at
<https://github.com/mt-mods/basic_materials/issues/4>, upstream would
agree?  Not sure though.

(*) However, Vivien's substitutions have effectively turned it into a
mod!

Greetings,
Maxime.

Vivien Kraus schreef op zo 29-05-2022 om 17:43 [+0200]:
> Yes. Mesecons was using version numbers and then the latest tag is a
> date. Given that it’s April 1st, maybe there’s something funny
> occuring
> here, but the changes around that day looked reasonable to my
> untrained
> eye. However, one thing I didn’t notice at first was the drop of the
> +
> in the license. Sorry.

My guess is that upstream did some important fixes and decided to tag
it, upstream previously enabled the automatic system for automatically
making ContentDB releases for the latest commit every N days, upstream
then made another commit on the master branch on the same day, then
ContentDB notices the new commit and makes a releases based on the new
commit instead of the tagged commit.

Given that there is only a single date tag after 2017.03.05 but many
ContentDB releases, I don't think the date tags can be used.

More generally, I think it would be best to just use the ContentDB
releases (their version which might be a date, and the commit ContentDB
uses). 

Greetings,
Maxime

Vivien Kraus schreef op zo 29-05-2022 om 17:43 [+0200]:
> +       (patches '())
> +       (snippet '(begin #t))))

Why are the patches and snippet removed?  Those snippets remove bundled
binaries and bundled source code of libraries.  Libraries such as
bzip2, lzma, zlib, which presumably(?) weren't modified by Minetest.

Greetings,
Maxime.

Hello Maxime,

Le dimanche 29 mai 2022 à 19:48 +0200, Maxime Devos a écrit :
> Vivien Kraus schreef op zo 29-05-2022 om 17:43 [+0200]:
> > +       (patches '())
> > +       (snippet '(begin #t))))
> 
> Why are the patches and snippet removed?  Those snippets remove
> bundled
> binaries and bundled source code of libraries.  Libraries such as
> bzip2, lzma, zlib, which presumably(?) weren't modified by Minetest.

This is because the original irrlicht downloads a tarball, but we must
work with a git origin for irrlicht-for-minetest. Since the bundled
source code is only provided in the tarball, we don’t need to remove
it.

Vivien