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

guix-patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From:	Maxime Devos
Subject:	[bug#49828] [PATCH 06/20] guix: Add ContentDB importer.
Date:	Sat, 07 Aug 2021 20:31:05 +0200
User-agent:	Evolution 3.34.2

Leo Prikler schreef op do 05-08-2021 om 18:41 [+0200]:
> Hi,
> 
> Am Montag, den 02.08.2021, 17:50 +0200 schrieb Maxime Devos:
> > * guix/import/contentdb.scm: New file.
> > * guix/scripts/import/contentdb.scm: New file.
> > * tests/contentdb.scm: New file.
> > * Makefile.am (MODULES, SCM_TESTS): Register them.
> > * po/guix/POTFILES.in: Likewise.
> > * doc/guix.texi (Invoking guix import): Document it.
> > [...]
> > diff --git a/doc/guix.texi b/doc/guix.texi
> > index 43c248234d..d06c9b73c5 100644
> > --- a/doc/guix.texi
> > +++ b/doc/guix.texi
> > @@ -11313,6 +11313,31 @@ and generate package expressions for all
> > those packages that are not yet
> >  in Guix.
> >  @end table
> >  
> > +@item contentdb
> > +@cindex ContentDB
> > +Import metadata from @uref{https://content.minetest.net, ContentDB}.
> > +Information is taken from the JSON-formatted metadata provided
> > through
> > +@uref{https://content.minetest.net/help/api/, ContentDB's API} and
> > +includes most relevant information, including dependencies.  There
> > are
> > +some caveats, however.  The license information on ContentDB does
> > not
> > +distinguish between GPLvN-only and GPLvN-or-later.

ContentDB uses SPDX license identifiers now, and distinguishes between
GPL-N-only and GPL-N-or-later, so I adjusted the documentation appropriately.

> >   The commit id is
> > +sometimes missing.  The descriptions are in the Markdown format, but
> > +Guix uses Texinfo instead.  Texture packs and subgames are
> > unsupported.
> What is the "commit id"?  Is it the hash?  A tag?  Anything that
> resolves to a commit?

It's the SHA-1 of the Git commit.  I changes this to ‘the commit's SHA-1’.

> Also, since ContentDB sounds fairly generic (a database of content?),
> perhaps we ought to call this the "minetest" importer instead?

Technically, minetest has another mod repository as well:
<https://bower.minetest.org/>.  It's unmoderated though, and
<https://content.minetest.net> has some moderation and seems more
‘official’ (it's integrated in Minetest itself).  I replaced
(guix import contentdb) with (guix import minetest), likewise
for (guix script import minetest) and tests/minetest.scm.

> > +;; Minetest package.
> > +;;
> > +;; API endpoint: /packages/AUTHOR/NAME/
> > +(define-json-mapping <package> make-package package?
> > +  json->package
> > +  (author            package-author) ; string
> > +  (creation-date     package-creation-date ; string
> > +                     "created_at")
> > +  (downloads         package-downloads) ; integer
> > +  (forums            package-forums "forums" natural-or-false) ;
> > natural | #f
> This comment and some others like it seem to simply be repeating
> already present information.  Is there a use for them?  Should we
> instead provide a third argument on every field to verify/enforce the
> type?

I first added the ‘; natural-or-false’.  I only added the procedure
"natural-false" later.  Indeed, ‘; natural-or-false’ is redundant.
I removed the redundant ones in the revised patch.

I don't think there is need to verify types for each field.
Most aren't used by Guix.  If a type check would fail, that would
presumably mean the type check guix is incorrect (or not up-to-date).
Except for perhaps a backtrace, ill-typed fields are harmless.

> > +(define (contentdb-fetch author name)
> > +  "Return a <package> record for package NAME by AUTHOR, or #f on
> > failure."
> > +  (and=> (json-fetch
> > +          (string-append (%contentdb-api) "packages/" author "/"
> > name "/"))
> > +         json->package))
> Is there a reason for author and name to be separate keys?  For me it
> makes more sense to take AUTHOR/NAME as a singular search string from
> users and then perform queries based on that.

Not really actually, AUTHOR tends to go togehter with NAME except for
some exceptions.  I modified the code such that AUTHOR/NAME is a single
string.  It simplified code somewhat.

>   If ContentDB allows
> searching, we might also resolve NAME to a singular package where
> possible and otherwise error out, telling the user to choose one.

ContentDB allows searching.  I wrote some a procedure 'elaborate-contentdb-name'
used by (guix scripts import contentdb) that resolves "mesecons" to 
"Jeija/mesecons",
using the search API and added some tests.  If there are multiple candidates,
the one with the highest ‘score’ is choosen (alternatively, --sort=downloads can
be used instead).

> > +(define (important-dependencies dependencies author name)
> > +  (define dependency-list
> > +    (assoc-ref dependencies (string-append author "/" name)))
> > +  (filter-map
> > +   (lambda (dependency)
> > +     (and (not (dependency-optional? dependency))
> > +          ;; "default" must be provided by the 'subgame' in use
> > +          ;; and does not refer to a specific minetest mod.
> > +          ;; "doors", "bucket" ... are provided by the default
> > minetest
> > +          ;; subgame.
> > +          (not (member (dependency-name dependency)
> > +                       '("default" "doors" "beds" "bucket" "doors"
> > "farming"
> > +                         "flowers" "stairs" "xpanes")))

I tested this some more, and it appears that some mods depend on "dyes",
which is part of the default Minetest game, so I added all the mods
provided by the default (sub?)game.  The list began looking a little
long, so I replaced it with a hash table.

> > +          ;; Dependencies often have only one implementation.
> > +          (let* ((/name (string-append "/" (dependency-name
> > dependency)))
> > +                 (likewise-named-implementations
> > +                  (filter (cut string-suffix? /name <>)
> > +                          (dependency-packages dependency)))
> > +                 (implementation
> > +                  (and (not (null? likewise-named-implementations))
> > +                       (first likewise-named-implementations))))
> > +            (and implementation
> > +                 (apply cons (string-split implementation #\/))))))
> > +   dependency-list))
> What exactly does the likewise-named-implementations bit do here?

The list returned by 'dependency-packages' not only contains the mod
we need, but possibly also various ‘subgames’ that include that mod.
Filtering on '/name' filters out these subgames we don't need.

Also, theoretically another mod could implement the same interface.
The filtering would filter out the alternative implementations.

Anyway, I changes the implementation a bit.  It now explicitely
filters out ‘subgames’ and ‘texture packs’ using the ‘package-mod?’
procedure.  The resulting list tends to consist of only a single
element.  If it consists of multiple, the one with the highest score
(or the one with the highest download count, depending on --sort)
will be choosen (and a warning is printed).

> > +;; A list of license names is available at
> > +;; <https://content.minetest.net/api/licenses/>;;.
> > +(define (string->license str)
> > +  "Convert the string STR into a license object." [...]
> The link mentions, that ContentDB now supports all SPDX identifiers. 
> Do we have a SPDX->Guix converter lying around in some other importer
> that we could use as default case here (especially w.r.t. "or later")

There's a a converter in (guix import utils): spdx-string->license.
The old license identifiers appear to be removed, now only SPDX information
is available.  I modified the code to use spdx->string-license and removed
string->license.

It turns out it does not recognise GPL-N-only and GPL-N-or-later,
so I added a patch ‘import/utils: Recognise GPL-3.0-or-later and friends.’.

I tried implementing "guix refresh -t minetest ...".  It seems to work, but
requires some changes to (guix upstream) that needs some more work, so I left it
out of the revised patch set.  The refresher needs to know the author name
(or perform extra HTTP requests), so I added 'upstream-name' the package 
properties.

The revised patch series is attached.  It can also be found at
<https://notabug.org/maximed/guix-gnunet/src/minetest-2>.  It includes
the latest MINETEST_MOD_PATH patch.  I'll make the patch to export more things 
in
(guix build utils) later (for core-updates).

reetings,
Maxime.