Leo Prikler schreef op do 05-08-2021 om 14:04 [+0200]:
> > +mods, which consists of copying lua code, images and other resources
> > to
> s/lua/Lua/ :)

Fixed.

> > +(define* (install #:key inputs #:allow-other-keys #:rest arguments)
> > +  (apply (@@ (guix build copy-build-system) install)
> > +         #:install-plan (mod-install-plan (apply guess-mod-name
> > arguments))
> > +         arguments))
> @@ is a code smell, as far as Guix is concerned.  Rather import copy-
> build-system with the copy: prefix.

'copy-build-system' does not export 'install', so I have to use '@@' here.
Modifying 'copy-build-system' to export 'install' would presumably entail
a many rebuilds.

> > +(define png-file?
> > +  ((@@ (guix build utils) file-header-match) %png-magic-bytes))
> Likewise import (guix build utils) directly.

Likewise.

> > +(define (lower-mod name . arguments)
> > +  (define lower (build-system-lower gnu-build-system))
> > +  (apply lower
> > +         name
> > +         #:imported-modules %minetest-build-system-modules
> > +         #:modules %default-modules
> > +         #:phases '%standard-phases
> > +         #:implicit-inputs? #f
> > +         ;; Mods are architecture-independent.
> > +         #:target #f
> > +         ;; Ensure nothing sneaks into the closure.
> > +         #:allowed-references '()
> > +         (substitute-keyword-arguments arguments
> > +           ((#:native-inputs native-inputs '())
> > +            (append native-inputs (standard-minetest-packages))))))

> This appears a little confusing.  On first glance, it does not seem to
> allow overriding e.g. #:phases, but on a second look using `apply'
> together with shallowly substituted arguments would enable that. 

I modified the patch to move more things into 'substitute-keyword-arguments'
to reduce confusion.

>  The
> only thing that's missing imo is that #:implicit-inputs? is not
> honoured for (standard-minetest-packages) -- I think you might want to
> rectify that.

I modified 'lower-mod' to _not_ add 'standard-minetest-packages' to
'native-inputs' when #:implicit-inputs? is false, though I don't see
why a package definition for a Minetest mod would do that.

I also found a spellig bug ("to minimisation" -> "to minimise") which
is now fixed.

btw, I submitted the "MINETEST_MOD_PATH" patch upstream, with your suggestion
for "{std::string(...)}" construction:
<https://github.com/minetest/minetest/pull/11515>.

Greetings,
Maxime.