mbox series

[bug#52977,0/6] Update some minetest packages

Message ID 0eceb36ac47fee789ebaa551cc3b041e777bbce1.camel@telenet.be
Headers show
Series Update some minetest packages | expand

Message

M Jan. 3, 2022, 1:45 p.m. UTC
Hi,

This patch series updates some minetest packages.
They were made with 'guix refresh -u -t minetest', using the 'git-
fetch' updater from <https://issues.guix.gnu.org/50072#24>.

Checklist:
  * [x] version, commit and sha256/base32 updated
  * [x] it builds: ./pre-inst-env guix build minetest-homedecor-modpack
minetest-unified-inventory minetest-mobs-animal minetest-mobs minetest-
basic-materials minetest-technic --rounds=2
  * [x] linter is satisfied: ./pre-inst-env guix lint minetest-
homedecor-modpack minetest-unified-inventory minetest-mobs-animal
minetest-mobs minetest-basic-materials minetest-technic
  * [x] source code diff reasonable

    minetest-technic: some small documentation and code changes
    minetest-basic-materials: new subdirectory '.github'.
    minetest-mobs: some code changes, updated translations
    minetest-mobs-animal: Some code changes (chickens are different
now?)
    minetest-unified-inventory: Lots of code changes, some
documentation changes, updated translations and images
    minetest-homedecor-modpack: Some code changes.
  * [x] Minetest doesn't crash
    ./pre-inst-env guix shell --pure minetest-homedecor-modpack
minetest-unified-inventory minetest-mobs-animal minetest-mobs minetest-
basic-materials minetest-technic minetest -- minetest
    Create world with all mods enabled.
    + [x] Inventory mode appears to work
    + [x] spawning chickens appears to work
    + [x] rotating blocks from homedecor works

A TODO for the git-fetch updater:
  * [ ] following redirects (to avoid mixed case -> lower case changes
and .git suffixes in GitHub URLs)
  * [ ] Support (let ((commit ...) (revision ...)) [...]) so more
packages can be updated automatically

Greetings,
Maxime.

Comments

Ludovic Courtès Jan. 5, 2022, 10:30 p.m. UTC | #1
Hey Maxime,

Welcome back!  ;-)

Maxime Devos <maximedevos@telenet.be> skribis:

> This patch series updates some minetest packages.
> They were made with 'guix refresh -u -t minetest', using the 'git-
> fetch' updater from <https://issues.guix.gnu.org/50072#24>.

Oh nice.

> Checklist:
>   * [x] version, commit and sha256/base32 updated
>   * [x] it builds: ./pre-inst-env guix build minetest-homedecor-modpack
> minetest-unified-inventory minetest-mobs-animal minetest-mobs minetest-
> basic-materials minetest-technic --rounds=2
>   * [x] linter is satisfied: ./pre-inst-env guix lint minetest-
> homedecor-modpack minetest-unified-inventory minetest-mobs-animal
> minetest-mobs minetest-basic-materials minetest-technic
>   * [x] source code diff reasonable
>
>     minetest-technic: some small documentation and code changes
>     minetest-basic-materials: new subdirectory '.github'.
>     minetest-mobs: some code changes, updated translations
>     minetest-mobs-animal: Some code changes (chickens are different
> now?)
>     minetest-unified-inventory: Lots of code changes, some
> documentation changes, updated translations and images
>     minetest-homedecor-modpack: Some code changes.
>   * [x] Minetest doesn't crash
>     ./pre-inst-env guix shell --pure minetest-homedecor-modpack
> minetest-unified-inventory minetest-mobs-animal minetest-mobs minetest-
> basic-materials minetest-technic minetest -- minetest
>     Create world with all mods enabled.
>     + [x] Inventory mode appears to work
>     + [x] spawning chickens appears to work
>     + [x] rotating blocks from homedecor works

Neat.  I applied them all.

> A TODO for the git-fetch updater:
>   * [ ] following redirects (to avoid mixed case -> lower case changes
> and .git suffixes in GitHub URLs)
>   * [ ] Support (let ((commit ...) (revision ...)) [...]) so more
> packages can be updated automatically

Hmm wasn’t it the reason we introduced ‘package-definition-location’?

Thanks!

Ludo’.
M Jan. 5, 2022, 11:18 p.m. UTC | #2
Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]:
> > A TODO for the git-fetch updater:
> >    * [ ] following redirects (to avoid mixed case -> lower case
> > changes
> > and .git suffixes in GitHub URLs)
> >    * [ ] Support (let ((commit ...) (revision ...)) [...]) so more
> > packages can be updated automatically
> 
> Hmm wasn’t it the reason we introduced ‘package-definition-location’?

Yes, but it has been a while ago so I preferred to not overcomplicate
the patch series more.  There's also the complication that
(revision ...) might need to be incremented. Yet another complication:
minetest mods can have two separate version schemes: the
‘release titles’ on content.minetest.net, and the version they use in
forum posts or git repos.  For minetest-ethereal, they are different,
and worse, the latest version on content.minetest.net doesn't have a
corresponding version on the forum.

Hopefully there's some kind of solution, and presumably
package-definition-location will be part of that solution,
but it will probably be messy.

Or maybe the solution is to ask upstream nicely to give every release
a version number and switch to ContentDB release titles if they
disagree.

Anyway, I'd prefer to leave these complications for future patches.

Greetings,
Maxime.
M Jan. 5, 2022, 11:21 p.m. UTC | #3
Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]:
> Hey Maxime,
> 
> Welcome back!  ;-)
> 
> Maxime Devos <maximedevos@telenet.be> skribis:
> 
> > This patch series updates some minetest packages.
> > They were made with 'guix refresh -u -t minetest', using the 'git-
> > fetch' updater from <https://issues.guix.gnu.org/50072#24>
> > 

btw I left a sneek message on #guix with information on where v5
can be found, apparently it didn't make it onto issues.guix.gnu.org
somehow.
Liliana Marie Prikler Jan. 6, 2022, 12:52 a.m. UTC | #4
Hi,

Am Donnerstag, dem 06.01.2022 um 00:18 +0100 schrieb Maxime Devos:
> Yes, but it has been a while ago so I preferred to not overcomplicate
> the patch series more.  There's also the complication that
> (revision ...) might need to be incremented. Yet another
> complication:
> minetest mods can have two separate version schemes: the
> ‘release titles’ on content.minetest.net, and the version they use in
> forum posts or git repos.  For minetest-ethereal, they are different,
> and worse, the latest version on content.minetest.net doesn't have a
> corresponding version on the forum.
If I recall correctly, this was also a point of debate in the initial
series that added the importer.  Can we establish an ordering/heuristic
here?  E.g. "if we have git tags use those, otherwise use contentdb",
"always use contentdb" or "always use whatever was edited most
recently"?
M Jan. 6, 2022, 9:33 a.m. UTC | #5
Hi,

Liliana Marie Prikler schreef op do 06-01-2022 om 01:52 [+0100]:
> > [...]
> If I recall correctly, this was also a point of debate in the initial
> series that added the importer.  Can we establish an ordering/heuristic
> here?  E.g. "if we have git tags use those, otherwise use contentdb",
> "always use contentdb" or "always use whatever was edited most
> recently"?

Keep in mind that the minetest importer doesn't know about git tag
-- the only interaction it has with git is cloning repositories
and checking out commits by the commit id provided by ContentDB.
I'm assuming you're referring to the generic-git updater here,
or a hypothetical minetest updater that has been modified to
interact with git tags.

* Problem with using git tags: git tags sometimes disappear.
  E.g., in minetest-ethereal, there's currently a tag
  2021-04-06 and 2021-09-23, but there's no tag for 2021-07-28
  (the version currently in guix).

  This could be resolved by including the commit instead of the
  tag in the package definition, and still searching for the git tag,
  but as I understand it, there have been some objections to including
  the commit in the package definition
(https://lists.gnu.org/archive/html/guix-devel/2021-12/msg00259.html).

  Even then, there's another problem: sometimes releases are made
  without a corresponding git tag.  E.g., on ContentDB there's a
  version 2022-01-05 but there's no 2022-01-05 tag in the git
  repository.

  That could be resolved by ‘always use contentdb for minetest
  packages’ or ‘always use whatever was edited most recently’.

* Problem with ‘whatever was edited most recently’: AFAIK git tags
  don't carry that information. Though the commit time/modification
  time in the commit it points to might be a decent approximation
  in practice.

  ContentDB has some information on when a release was released
  (release_date, see https://content.minetest.net/help/api/).

  I suppose this could work, though there's a slight problem:

  The version scheme in guix would occassionally switch between x.y.z
  and YYYY-MM-DD, which would confuse the ‘these packages have been
  upgraded’ logic.

I suppose the best option would be to always use the version from
ContentDB (*), because the exact versioning scheme used doesn't matter
much, as long as it remains consistent over time, and just using
ContentDB is convenient.

(*) Unless it isn't on ContentDB of course, though all minetest
packages currently in Guix are on ContentDB.

Additionally, if the forum versions / git tags / contentdb releases
are inconsistent (e.g. the forum and git tags are x.y.z and the
releases are YYYY-MM-DD), we could inform upstream that guix uses the
release titles because otherwise things become complicated for guix, so
if upstream doesn't want that, they need to use x.y.z in their release
titles as well

Does that seem reasonable to you? I could write a patch to that effect.

Greetings,
Maxime.
Liliana Marie Prikler Jan. 6, 2022, 3:49 p.m. UTC | #6
Hi,

Am Donnerstag, dem 06.01.2022 um 09:33 +0000 schrieb Maxime Devos:
> Hi,
> 
> Liliana Marie Prikler schreef op do 06-01-2022 om 01:52 [+0100]:
> > > [...]
> > If I recall correctly, this was also a point of debate in the initial
> > series that added the importer.  Can we establish an
> > ordering/heuristic here?  E.g. "if we have git tags use those,
> > otherwise use contentdb", "always use contentdb" or "always use
> > whatever was edited most recently"?
> 
> Keep in mind that the minetest importer doesn't know about git tag
> -- the only interaction it has with git is cloning repositories
> and checking out commits by the commit id provided by ContentDB.
> I'm assuming you're referring to the generic-git updater here,
> or a hypothetical minetest updater that has been modified to
> interact with git tags.
Ahh, sure, but imho it could try to make a best effort guess.  E.g. if
git tags are named like Minetest releases and exist in equal count,
assume a mapping from one to the other.  However if I recall correctly
there was a ContentDB policy to only tag once, which makes the mapping
from ContentDB version to git commit unique.  Do I remember correctly?

> * Problem with using git tags: git tags sometimes disappear.
>   E.g., in minetest-ethereal, there's currently a tag
>   2021-04-06 and 2021-09-23, but there's no tag for 2021-07-28
>   (the version currently in guix).
> 
>   This could be resolved by including the commit instead of the
>   tag in the package definition, and still searching for the git tag,
>   but as I understand it, there have been some objections to
>   including the commit in the package definition
> (https://lists.gnu.org/archive/html/guix-devel/2021-12/msg00259.html).
Thanks for linking my thread :)

I do cite minetest.scm as an exception there, since your comments make
very clear what you're doing and why.  However, my actual intent is
blown way out of proportion, which has led to a lot of confusion on all
sides and a rather long discussion.

To clarify it here, I am trying to avoid the following pattern:

  (package 
    (name "hello")
    (version "1.0")
    (source 
      (origin 
        (method git-fetch)
        (uri (git-reference
               (url some-url)
               (commit "abcdef...")))
        [...]))
    [...])

while encouraging both

  (package 
    (name "hello")
    (version "1.0")
    (source 
      (origin 
        (method git-fetch)
        (uri (git-reference
               (url some-url)
               (commit (version->git-tag version))))
        [...]))
    [...])

and

  (package 
    (name "hello")
    (version (git-version "1.0" revision commit))
    (source 
      (origin 
        (method git-fetch)
        (uri (git-reference
               (url some-url)
               (commit commit)))
        [...]))
    [...])

In the latter, revision and commit are let-bound as per Guix'
standards.  Now if you say "minetest mod packages are generally
unreliable, git-version everywhere", that is completely fine by me.

>   Even then, there's another problem: sometimes releases are made
>   without a corresponding git tag.  E.g., on ContentDB there's a
>   version 2022-01-05 but there's no 2022-01-05 tag in the git
>   repository.
> 
>   That could be resolved by ‘always use contentdb for minetest
>   packages’ or ‘always use whatever was edited most recently’.
> 
> * Problem with ‘whatever was edited most recently’: AFAIK git tags
>   don't carry that information. Though the commit time/modification
>   time in the commit it points to might be a decent approximation
>   in practice.
> 
>   ContentDB has some information on when a release was released
>   (release_date, see https://content.minetest.net/help/api/).
> 
>   I suppose this could work, though there's a slight problem:
> 
>   The version scheme in guix would occassionally switch between x.y.z
>   and YYYY-MM-DD, which would confuse the ‘these packages have been
>   upgraded’ logic.
Does ContentDB always use CalVer or are the repo owners in control of
the tags?  If there's a SemVer/CalVer conflict, I would say doing 
(latest-semver)-(calver)-commit would probably be acceptable.  At least
I hope none of these mods release twice daily.

> I suppose the best option would be to always use the version from
> ContentDB (*), because the exact versioning scheme used doesn't
> matter much, as long as it remains consistent over time, and just
> using ContentDB is convenient.
> 
> (*) Unless it isn't on ContentDB of course, though all minetest
> packages currently in Guix are on ContentDB.
If a package was used outside of ContentDB, that's not ContentDB's
requirement, is it?  Now perhaps there is an issue if the contentdb
updater relies on the minetest-mod-build-system being used to determine
that its suitable, rather than something else.

> Additionally, if the forum versions / git tags / contentdb releases
> are inconsistent (e.g. the forum and git tags are x.y.z and the
> releases are YYYY-MM-DD), we could inform upstream that guix uses the
> release titles because otherwise things become complicated for guix,
> so if upstream doesn't want that, they need to use x.y.z in their
> release titles as well
> 
> Does that seem reasonable to you? I could write a patch to that
> effect.
Poking upstream maintainers sounds fun, but before making a hard
decisions, perhaps we should reach out to them and ask what they
consider the most reliable.  If it's a wild mess, we could also use
package-properties with an assumed default of "release title = git tag
= forum post", e.g. (upstream-versioning . forum-post), (upstream-
versioning . contentdb) and (upstream-versioning . git-version) to
indicate who's right in case of a mismatch.

WDYT?
Ludovic Courtès Jan. 11, 2022, 12:53 p.m. UTC | #7
Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]:
>> > A TODO for the git-fetch updater:
>> >    * [ ] following redirects (to avoid mixed case -> lower case
>> > changes
>> > and .git suffixes in GitHub URLs)
>> >    * [ ] Support (let ((commit ...) (revision ...)) [...]) so more
>> > packages can be updated automatically
>> 
>> Hmm wasn’t it the reason we introduced ‘package-definition-location’?
>
> Yes, but it has been a while ago so I preferred to not overcomplicate
> the patch series more.

OK, I thought we more or less had a solution in place back then and just
missed ‘package-definition-location’.

I think we could do something along these lines:

  (edit-expression
    (location->source-properties (package-definition-location p))
    (lambda (str)
      ;; substitute commit ID in STR
      …))

But yes, let’s keep that for a subsequent patch.

Thanks,
Ludo’.
M Jan. 11, 2022, 1:20 p.m. UTC | #8
Ludovic Courtès schreef op di 11-01-2022 om 13:53 [+0100]:
> Maxime Devos <maximedevos@telenet.be> skribis:
> 
> > Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]:
> > > > A TODO for the git-fetch updater:
> > > >    * [ ] following redirects (to avoid mixed case -> lower case
> > > > changes
> > > > and .git suffixes in GitHub URLs)
> > > >    * [ ] Support (let ((commit ...) (revision ...)) [...]) so more
> > > > packages can be updated automatically
> > > 
> > > Hmm wasn’t it the reason we introduced ‘package-definition-location’?
> > 
> > Yes, but it has been a while ago so I preferred to not overcomplicate
> > the patch series more.
> 
> OK, I thought we more or less had a solution in place back then and just
> missed ‘package-definition-location’.
> 
> I think we could do something along these lines:
> 
>   (edit-expression
>     (location->source-properties (package-definition-location p))
>     (lambda (str)
>       ;; substitute commit ID in STR
>       …))
>
> But yes, let’s keep that for a subsequent patch.


See <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=53144>,
in particular the new 'latest-git' updater.

Greetings,
Maxime.