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

[Leo Prikler]

Hi,

Am Dienstag, den 03.08.2021, 13:59 +0200 schrieb Maxime Devos:
> 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)
Of course there's more that can be done here, but this just reaffirms
my earlier point, that the build system on its own as it is in 04/20
looks rather unfinished.  I'd personally prefer introducing it as one
whole as it gives a bigger picture of the whole thing rather than
digging into every detail.

> Also, having "#:install-plan '(("." "share/minetest/mods/the-mod-
> name"))"
> appear in every package definition seems rather repetitive to me.
Perhaps, but it's not like there aren't other groups of things that can
be implemented trivially in terms of copy-build-system.

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

> 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.
IMO that patch should also be merged "as one" with the others.

> > 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?
‘Those static strings’ are exactly "the-mod-name" in 
> #:install-plan '(("." "share/minetest/mods/the-mod-name"))
I'm still not quite convinced, that whatever improvements you seem
there to be can't be made by using a good enough include regexp.

> I suppose it is possible to exclude shell scripts from installation,
> but just installing everything (and disabling shebang patching) seems
> simpler.
Likewise leaving shell references in there and using plain copy-build-
system would be simpler than making a new build system, wouldn't it?  I
don't think that's a good reason not to make a proper install plan.

Greetings