[bug#49828] [PATCH 05/20] build-system: minetest: Don't retain references to "bash-minimal".

[Maxime Devos <maximedevos@telenet.be>]

[-- Attachment #1.1: Type: text/plain, Size: 2146 bytes --]

Leo Prikler schreef op di 03-08-2021 om 11:17 [+0200]:
> Hi,
> 
> I'd merge this and 04/20 into a single patch.  04/20 does of its own
> give a good incentive as to why a new build system is to be used (this
> could instead be handled by the importer), with this phase added it
> makes slightly more sense.

As an argument for having a 'minetest-mod-build-system', consider
some ways 'minetest-mod-build-system' could be improved in the future:

  * a phase could be added to minimise PNG images (e.g. using 'optipng')
  * likewise, for lua code
  * some basic tests could be added (e.g. creating a new world and
    loading the mod, testing that Minetest doesn't raise an error during
    mod loading)

Also, having "#:install-plan '(("." "share/minetest/mods/the-mod-name"))"
appear in every package definition seems rather repetitive to me.

The idea behind "04/20" and "05/20" being separate patches, is to start
with a basic "minetest-mod-build-system" and gradually improve it.
The small improvements (currently only one, i.e., 05/20) could be reviewed
separately from each other and whether there should be a
"minetest-mod-build-system" at all.

E.g., see attached a patch that sets #:allowed-references '(), ensuring
nothing sneaks into the closure.  Another change I'm thinking of, is including
only "tar", "gzip" and the like as implicit inputs, and not "bash" or "coreutils",
though that's probably useless if shebang patching has been disabled.

> OTOH, perhaps we shouldn't install those shell scripts in the first
> place?  Perhaps we can instead make the importer generate packages
> based directly on copy-build-system, in which those static strings are
> already evaluated.  WDYT?

Directly using 'copy-build-system' makes it more difficult to make the
improvements listed above. I don't know what you mean with ‘in which those
static strings are already evaluated’ -- what are ‘those static strings’ here?

I suppose it is possible to exclude shell scripts from installation, but
just installing everything (and disabling shebang patching) seems simpler.

Greetings,
Maxime.

[-- Attachment #1.2: 0001-build-system-copy-Support-allowed-references.patch --]
[-- Type: text/x-patch, Size: 2176 bytes --]

From eef6cb11a923458cba50bbc4e6440c0b2f372da2 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Tue, 3 Aug 2021 13:50:49 +0200
Subject: [PATCH 1/2] build-system/copy: Support #:allowed-references.

* guix/build-system/copy.scm
  (copy-build): Add #:allowed-references argument.
  (copy-build)[canonicalize-reference]: New procedure.
---
 guix/build-system/copy.scm | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/guix/build-system/copy.scm b/guix/build-system/copy.scm
index d1bf8fb654..1cd0a95150 100644
--- a/guix/build-system/copy.scm
+++ b/guix/build-system/copy.scm
@@ -92,8 +92,22 @@
                      (system (%current-system))
                      (imported-modules %copy-build-system-modules)
                      (modules '((guix build copy-build-system)
-                                (guix build utils))))
+                                (guix build utils)))
+                     allowed-references)
   "Build SOURCE using INSTALL-PLAN, and with INPUTS."
+  ;; XXX: procedure copied from (guix build-system gnu)
+  (define canonicalize-reference
+    (match-lambda
+     ((? package? p)
+      (derivation->output-path (package-derivation store p system
+                                                   #:graft? #f)))
+     (((? package? p) output)
+      (derivation->output-path (package-derivation store p system
+                                                   #:graft? #f)
+                               output))
+     ((? string? output)
+      output)))
+
   (define builder
     `(begin
        (use-modules ,@modules)
@@ -131,6 +145,10 @@
                                 #:system system
                                 #:inputs inputs
                                 #:modules imported-modules
+                                #:allowed-references
+                                (and allowed-references
+                                     (map canonicalize-reference
+                                          allowed-references))
                                 #:outputs outputs
                                 #:guile-for-build guile-for-build))
 
-- 
2.32.0


[-- Attachment #1.3: 0002-build-system-minetest-Don-t-let-anything-sneak-into-.patch --]
[-- Type: text/x-patch, Size: 901 bytes --]

From f383aa1c886701631f6ae924a93e13cdad2eaa59 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Tue, 3 Aug 2021 13:52:37 +0200
Subject: [PATCH 2/2] build-system/minetest: Don't let anything sneak into the
 closure.

* guix/build-system/minetest.scm (lower-mod): Set #:allowed-references
  to the empty list.
---
 guix/build-system/minetest.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/guix/build-system/minetest.scm b/guix/build-system/minetest.scm
index 993c5631eb..1d63e5ffcf 100644
--- a/guix/build-system/minetest.scm
+++ b/guix/build-system/minetest.scm
@@ -51,6 +51,8 @@
     `'(("." ,(string-append "share/minetest/mods/"
                             (guix-name->mod-name name))))
     #:phases %standard-phases
+    ;; Ensure nothing sneaks into the closure.
+    #:allowed-references '()
     arguments))
 
 (define minetest-mod-build-system
-- 
2.32.0


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]