[bug#49828] [PATCH 06/20] guix: Add ContentDB importer.

[Leo Prikler]

Hi,

Am Dienstag, den 10.08.2021, 13:02 +0200 schrieb Maxime Devos:
> [...]
> FWIW, there is no git/vcs updater, but there is a GitHub updater
> (a git updater looking at tags in a git repository if the package
> origin uses the git-fetch method could be written though).
My bad, I thought that worked for more than just GitHub (having at
least GitLab as well would cover most git repos anyway.)

> The GitHub updater could theoretically work, but look at the output:
> 
> ./pre-inst-env guix refresh minetest-{unified-
> inventory,unifieddyes,worldedit,throwing-
> arrows,throwing,technic,pipeworks,mobs-
> animal,mobs,mesecons,homedecor-modpack,ethereal,coloredwood,basic-
> materials}
> [...]
> Apparently, the git repos of minetest mods often don't keep version
> numbers,
> or aren't on github, or uses multiple version schemes simultanuously
> (x.y[.z] version numbers + dates) ... "minetest-worldedit" is
> properly
> tagged though.  For the other cases, an updater for ContentDB
> packages
> may be required.
> 
> I modified the definition of 'minetest-throwing' to use "1.1" as
> 'version' (the commit remains the same).
Okay, it seems a separate updater using contentdb is still very much
needed.

> 'upstream-name' is used by the refresher of "egg", "cpan" and "opam".
> I searched for "package-properties" in (guix import ...), and the
> only
> package property used by refreshers in 'upstream-name'.  So using
> anything other than 'upstream-name' for Minetest would be unusual.
> 
> The documentation of "guix refresh" mentions:
> 
>    Sometimes the upstream name differs from the package name used in
> Guix, and ‘guix refresh’ needs a little help.  Most updaters honor
> the
> ‘upstream-name’ property in package definitions, which can be used to
> that effect:
> 
>      (define-public network-manager
>        (package
>          (name "network-manager")
>          ;; ...
>          (properties '((upstream-name . "NetworkManager")))))
> 
> So using a different package property than 'upstream-name' would
> be unexpected.
> [...]
Fair enough, I should probably read up on the code in `guix refresh` to
understand how it ensures the correct updater is chosen.

> About pointing the home page to content.minetest.net: that would
> work for the refresher, but the ContentDB page isn't really the
> home page, not unlike how the pypi page for python packages isn't
> the home page.  Likewise for opam and ocaml.  Compare
> 
> https://content.minetest.net/packages/Jeija/mesecons/
> 
> with https://mesecons.net/, which one seems more ‘homey’?  I'd
> say the latter is the home page.
Point taken.

> The revised patch series is attached.  Only
> ‘build-system: Add 'minetest-mod-build-system'’ and ‘gnu: Add
> minetest-throwing’
> have been changed.  It can also be found at
> <https://notabug.org/maximed/guix-gnunet/src/minetest-3>;.
Can you resend this v3 (with or without amendments to the points below)
via `git send-email'?  The series appears pretty "final" to me, but
having one mail per patch might make it easier for others to point out
issues if they find them.

> +           (file-name (if (string-suffix? "-checkout" file-name)
> +                          (substring file-name
> +                                     0
> +                                     (- (string-length file-name)
> +                                        (string-length "-
> minetest")))
> +                          file-name))
It is probably an accident, that "-checkout" and "-minetest" have the
same string length :P
I think we should probably add a (guix build utils) procedure to take
care of such string prefix/suffix stripping in next core-updates.

> +  (cond ((file-exists? "mod.conf")
> +         (read-mod-name "mod.conf"))
> +        ((file-exists? "modpack.conf")
> +         (read-mod-name "modpack.conf" guess))
> +        (#t (guess))))
Why do we yield an error if there's no name in mod.conf instead of
trying to read from modpack.conf or just guessing?

> [PATCH 14/20] gnu: Add minetest-throwing.
LGTM