diff mbox series

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

Message ID c754f97a29f31dc4724cd38fc61fee3cca734d71.camel@telenet.be
State Accepted
Headers show
Series [bug#49828,v3,01/20] gnu: minetest: Respect --without-tests. | expand

Commit Message

M Aug. 10, 2021, 11:02 a.m. UTC
Hi,

I re-ran "./pre-inst-env guix build [all-minetest-mods]" and it turns
out "minetest-unified-inventory" retains a reference to the corresponding
git checkout.  Using 'strip-store-file-name' instead of 'basename'
in 'guess-mod-name' in ‘build-system: Add 'minetest-mod-build-system'’
fixed this.

Leo Prikler schreef op ma 09-08-2021 om 23:45 [+0200]:
> Hi,
> 
> [...]
> In other words, taking minetest-uri as is currently doesn't seem too
> good of an idea.  Does the plain git/vcs updater work for those
> packages then?  I assume at least some of the packages should be tagged
> properly in git, are they not?

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).

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}

gnu/packages/minetest.scm:29:2: warning: no updater for minetest-basic-materials
gnu/packages/minetest.scm:54:2: warning: no updater for minetest-coloredwood
gnu/packages/minetest.scm:87:4: warning: no updater for minetest-ethereal
gnu/packages/minetest.scm:111:2: warning: no updater for minetest-homedecor-modpack
gnu/packages/minetest.scm:147:13: minetest-mesecons would be upgraded from 1.2.1-0.db58797 to 2017.03.05
gnu/packages/minetest.scm:178:2: warning: no updater for minetest-mobs
gnu/packages/minetest.scm:207:2: warning: no updater for minetest-mobs-animal
gnu/packages/minetest.scm:234:2: warning: no updater for minetest-pipeworks
gnu/packages/minetest.scm:265:2: warning: 'github' updater failed to determine available releases for minetest-technic
gnu/packages/minetest.scm:302:13: warning: 2020-08-14 is greater than the latest known version of minetest-throwing (1.1)
gnu/packages/minetest.scm:330:15: warning: 1.1-0.059cc89 is greater than the latest known version of minetest-throwing-arrows (1.1)
gnu/packages/minetest.scm:355:13: 1.3 is already the latest version of minetest-worldedit
gnu/packages/minetest.scm:376:2: warning: no updater for minetest-unifieddyes
gnu/packages/minetest.scm:408:13: minetest-unified-inventory would be upgraded from 2021-03-25-1 to 20180810

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).

> All in all, I think I'm rather content with this patch now, but I have
> a final nitpick w.r.t the updater.  "upstream-name" is a weird name for
> a property that will supposedly be used by only one updater (and even
> if different updaters were to use it, would not each one have slightly
> different, but perhaps sometimes overlapping expectations for that
> field?)  I think a better solution would be to set home-page to the
> ContentDB page, assuming that is acceptable.  If not, perhaps having a
> "refresh-url" that is well defined over all refreshers and supersedes
> home-page if specified might be a more appropriate solution.  

'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.

I assume 'refresh-url' would be
"https://content.minetest.net/api/packages/AUTHOR/NAME" for minetest
packages, "https://github.com/ocaml/opam-repository/packages/NAME"
for opam packages?  That seems rather verbose though, and every
refresher would have a different idea of how to fetch the package
description from the URL and interpret it, so this doesn't seem
to be an improvement over 'upstream-name' to me.

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.

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>.

Greetings,
Maxime.

Comments

Leo Prikler Aug. 10, 2021, 12:16 p.m. UTC | #1
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
diff mbox series

Patch

From dbca87ad489d883487047af2c2a957a665096477 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Sun, 1 Aug 2021 20:03:07 +0200
Subject: [PATCH 20/20] gnu: Add minetest-homedecor-modpack.

* gnu/packages/minetest.scm
  (minetest-homedecor-modpack): New variable.
---
 gnu/packages/minetest.scm | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/gnu/packages/minetest.scm b/gnu/packages/minetest.scm
index 3cd14a6df8..3bd640da45 100644
--- a/gnu/packages/minetest.scm
+++ b/gnu/packages/minetest.scm
@@ -107,6 +107,37 @@  special items, intending to make an interesting adventure.")
       (license (list license:cc0 license:expat))
       (properties `((upstream-name . "TenPlus1/ethereal"))))))
 
+(define-public minetest-homedecor-modpack
+  (package
+    (name "minetest-homedecor-modpack")
+    ;; Upstream doesn't tag releases, so use the release title from
+    ;; ContentDB as version.
+    (version "2021-03-27-1")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://gitlab.com/VanessaE/homedecor_modpack")
+             (commit "9ffe2b7d691133e1a067546574fbe7364fd02f32")))
+       (sha256
+        (base32 "1lfajqvc2adf9hqskghky4arccqzpjw4i9a01hv4qcckvivm04ag"))
+       (file-name (git-file-name name version))))
+    (build-system minetest-mod-build-system)
+    (propagated-inputs
+     `(("minetest-basic-materials" ,minetest-basic-materials)
+       ("minetest-unifieddyes" ,minetest-unifieddyes)))
+    (home-page (minetest-topic 2041))
+    (synopsis "Home decor mod for Minetest")
+    (description
+     ;; TRANSLATORS: ‘homedecor’ is the name is the name of a Minetest mod
+     ;; and should not be translated.
+     "The homedecor Minetest mod provides a large seleection of items that
+might be found inside and around homes, such as sofas, chairs, tables, fences
+and a variety of other stuff.")
+    (license
+     (list license:cc-by-sa4.0 license:lgpl3))
+    (properties `((upstream-name . "VanessaE/homedecor_modpack")))))
+
 (define-public minetest-mesecons
   ;; The release on ContentDB does not have its own version number.
   (let ((commit "db5879706d04d3480bc4863ce0c03fa73e5f10c7")
-- 
2.32.0