GNU bug report logs - #50677
[PATCH] Minetest basic_materials really depends on moreores

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 50677 in the body.
You can then email your comments to 50677 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: bug-guix <at> gnu.org
Subject: [PATCH] Minetest basic_materials really depends on moreores
Date: Sun, 19 Sep 2021 17:23:50 +0200

[Message part 1 (text/plain, inline)]

Dear guix,

basic_materials actually requires moreores, otherwise some craft
recipes are just not possible. For instance,
basic_materials:silver_wire can only be crafted with
moreores:silver_ingot. This silver wire is actually needed for basic
technic machines, such as the LV compressor.

So, let’s add minetest-moreores!

Vivien

[0002-gnu-minetest-basic-materials-Depend-on-minetest-more.patch (text/x-patch, attachment)]

[0001-gnu-Add-minetest-moreores.patch (text/x-patch, attachment)]

Message #8 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Vivien Kraus <vivien <at> planete-kraus.eu>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Sun, 19 Sep 2021 19:40:50 +0200

[Message part 1 (text/plain, inline)]

Vivien Kraus via Bug reports for GNU Guix schreef op zo 19-09-2021 om 17:23 [+0200]:
> Dear guix,
> 
> basic_materials actually requires moreores, otherwise some craft
> recipes are just not possible. For instance,
> basic_materials:silver_wire can only be crafted with
> moreores:silver_ingot. This silver wire is actually needed for basic
> technic machines, such as the LV compressor.
> 
> So, let’s add minetest-moreores!

In creative mode, crafting isn't necessary, you can give yourself all
the items and blocks you want.  OTOH, letting the crafting recipes
work (at least when all the mods are enabled for the world,
they seem to be disabled by default) seems rather important for ‘survival’
players, so adding a propagated-input doesn't seem unreasonable here.

> + (version "v2.1.0")

Versions in guix don't use any leading "v" prefix.  If I'm not mistaken,
Ludovic applied the patched to remove them automatically.

> +          (lambda* (#:key inputs #:allow-other-keys)

inputs is unused, so you can write '(lambda _' here instead (somewhat simpler)

> /share/minetest/mods/moreores/_config.txt

Is this "_config.txt" file moreores-specific?  If not, maybe minetest-mod-build-system
could be modified to install these files by default?

Greetings,
Maxime

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Maxime Devos <maximedevos <at> telenet.be>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Sun, 19 Sep 2021 20:54:02 +0200

[Message part 1 (text/plain, inline)]

Hello,

Le dimanche 19 septembre 2021 à 19:40 +0200, Maxime Devos a écrit :
> > + (version "v2.1.0")
> 
> Versions in guix don't use any leading "v" prefix.  If I'm not
> mistaken,
> Ludovic applied the patched to remove them automatically.
Right, I just had not guix pulled lately, I confirm it works with a
more recent guix version.

> > /share/minetest/mods/moreores/_config.txt
> 
> Is this "_config.txt" file moreores-specific?  If not, maybe
> minetest-mod-build-system
> could be modified to install these files by default?
I don’t know, but a quick web search for:

   minetest mod "_config.txt"

gave me 2 hits, one for moreores, and one for moreblocks, by the same
author. I supposed it is an author preference.

I forgot to say, I could not figure out why the package did not build,
because the check phase in the minetest build system stops after the
first error line of output, which only stated that there was a problem
with init.lua. I had to disable the error line detection in the build
system implementation to know it was a problem with this file. So, I
figured out that what we need to do is gather all error lines, until
either the server stops or starts despite the error, and fail only
then.

I prefer the exceptions API because it can handle multiple lines of
errors while displaying them cleanly (unlike the error function, that
displays an ugly ~a and prints the arguments on 1 line). But, it’s only
for the "new" guile 3.0.

What do you think?

Vivien

[0003-gnu-minetest-basic-materials-Depend-on-minetest-more.patch (text/x-patch, attachment)]

[0002-gnu-Add-minetest-moreores.patch (text/x-patch, attachment)]

[0001-guix-minetest-build-system-Report-all-error-lines-be.patch (text/x-patch, attachment)]

Message #14 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Maxime Devos <maximedevos <at> telenet.be>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Sun, 19 Sep 2021 20:59:54 +0200

[Message part 1 (text/plain, inline)]

Le dimanche 19 septembre 2021 à 20:54 +0200, Vivien Kraus a écrit :
> I forgot to say, I could not figure out why the package did not
> build,
> because the check phase in the minetest build system stops after the
> first error line of output, which only stated that there was a
> problem
> with init.lua. I had to disable the error line detection in the build
> system implementation to know it was a problem with this file. So, I
> figured out that what we need to do is gather all error lines, until
> either the server stops or starts despite the error, and fail only
> then.
> 
> I prefer the exceptions API because it can handle multiple lines of
> errors while displaying them cleanly (unlike the error function, that
> displays an ugly ~a and prints the arguments on 1 line). But, it’s
> only
> for the "new" guile 3.0.
Sorry, the patterns were not in the correct order. This version is
better.

[0001-guix-minetest-build-system-Report-all-error-lines-be.patch (text/x-patch, attachment)]

Message #17 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Vivien Kraus <vivien <at> planete-kraus.eu>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Sun, 19 Sep 2021 21:11:57 +0200

[Message part 1 (text/plain, inline)]

Vivien Kraus schreef op zo 19-09-2021 om 20:54 [+0200]:
> Hello,
> 
> Le dimanche 19 septembre 2021 à 19:40 +0200, Maxime Devos a écrit :
> > > + (version "v2.1.0")
> > 
> > Versions in guix don't use any leading "v" prefix.  If I'm not
> > mistaken,
> > Ludovic applied the patched to remove them automatically.
> Right, I just had not guix pulled lately, I confirm it works with a
> more recent guix version.
> 
> > > /share/minetest/mods/moreores/_config.txt
> > 
> > Is this "_config.txt" file moreores-specific?  If not, maybe
> > minetest-mod-build-system
> > could be modified to install these files by default?
> I don’t know, but a quick web search for:
> 
>    minetest mod "_config.txt"
> 
> gave me 2 hits, one for moreores, and one for moreblocks, by the same
> author. I supposed it is an author preference.

I found a use of "config.txt" (https://forum.minetest.net/viewtopic.php?t=2290),
"settingstypes.txt" (https://github.com/NathanSalapat/more_fire/blob/master/settingtypes.txt)
and another "_config.txt" (https://forum.minetest.net/viewtopic.php?t=25275).

"settingstypes.txt" is already in mod-install-plan (guix build minetest-build-system).
I suppose you can add "config.txt" and "_config.txt" as well.

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Vivien Kraus <vivien <at> planete-kraus.eu>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Sun, 19 Sep 2021 21:18:34 +0200

[Message part 1 (text/plain, inline)]

Vivien Kraus schreef op zo 19-09-2021 om 20:59 [+0200]:
> Le dimanche 19 septembre 2021 à 20:54 +0200, Vivien Kraus a écrit :
> > I forgot to say, I could not figure out why the package did not
> > build,
> > because the check phase in the minetest build system stops after the
> > first error line of output, which only stated that there was a
> > problem
> > with init.lua. I had to disable the error line detection in the build
> > system implementation to know it was a problem with this file. So, I
> > figured out that what we need to do is gather all error lines, until
> > either the server stops or starts despite the error, and fail only
> > then.
> >
> > I prefer the exceptions API because it can handle multiple lines of
> > errors while displaying them cleanly (unlike the error function, that
> > displays an ugly ~a and prints the arguments on 1 line). But, it’s
> > only
> > for the "new" guile 3.0.

This modified error reporting seems reasonable to me, but I haven't tested
it.  It only being for the "new" guile 3.0 is probably fine.  IIUC, within
the build environment, guile 3.0.SOMETHING is used, and configure.ac
only checks for guile 3.0:

dnl Search for 'guile' and 'guild'.  This macro defines
dnl 'GUILE_EFFECTIVE_VERSION'.
GUILE_PKG([3.0])
GUILE_PROGS
if test "x$GUILD" = "x"; then
   AC_MSG_ERROR(['guild' binary not found; please check your Guile installation.])
fi

Greetings,
Maxime

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Maxime Devos <maximedevos <at> telenet.be>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Sun, 19 Sep 2021 23:05:09 +0200

[Message part 1 (text/plain, inline)]

Le dimanche 19 septembre 2021 à 21:11 +0200, Maxime Devos a écrit :
> I found a use of "config.txt" (
> https://forum.minetest.net/viewtopic.php?t=2290),
> "settingstypes.txt" (
> https://github.com/NathanSalapat/more_fire/blob/master/settingtypes.txt
> )
> and another "_config.txt" (
> https://forum.minetest.net/viewtopic.php?t=25275).
> 
> "settingstypes.txt" is already in mod-install-plan (guix build
> minetest-build-system).
> I suppose you can add "config.txt" and "_config.txt" as well.

OK.

> Vivien Kraus schreef op zo 19-09-2021 om 20:59 [+0200]:
> > Le dimanche 19 septembre 2021 à 20:54 +0200, Vivien Kraus a écrit :
> > > I forgot to say, I could not figure out why the package did not
> > > build,
> > > because the check phase in the minetest build system stops after
> the
> > > first error line of output, which only stated that there was a
> > > problem
> > > with init.lua. I had to disable the error line detection in the
> build
> > > system implementation to know it was a problem with this file.
> So, I
> > > figured out that what we need to do is gather all error lines,
> until
> > > either the server stops or starts despite the error, and fail
> only
> > > then.
> > >
> > > I prefer the exceptions API because it can handle multiple lines
> of
> > > errors while displaying them cleanly (unlike the error function,
> that
> > > displays an ugly ~a and prints the arguments on 1 line). But,
> it’s
> > > only
> > > for the "new" guile 3.0.
> 
> This modified error reporting seems reasonable to me, but I haven't
> tested
> it.

I link my output without 0001 (fail at first error line) nor 0002 (fail
because of _config.txt): the build fails without a mention of
_config.txt; and my output just without 0002: the build fails more
nicely. Also, all other packages returned by "guix search minetest"
still build successfully.

Vivien

[0004-gnu-minetest-basic-materials-Depend-on-minetest-more.patch (text/x-patch, attachment)]

[0003-gnu-Add-minetest-moreores.patch (text/x-patch, attachment)]

[0002-guix-minetest-build-system-Also-install-config.txt-a.patch (text/x-patch, attachment)]

[0001-guix-minetest-build-system-Report-all-error-lines-be.patch (text/x-patch, attachment)]

[without-0001-0002-patch.txt (text/plain, attachment)]

[without-0002-patch.txt (text/plain, attachment)]

Message #26 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Maxime Devos <maximedevos <at> telenet.be>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Wed, 22 Sep 2021 18:46:27 +0200

[Message part 1 (text/plain, inline)]

lylip on the #guix chat suggested to save the log to disk, so that the
errors can be read in context. I think it is the best thing to do, what
do you think?

Vivien

[0004-gnu-minetest-basic-materials-Depend-on-minetest-more.patch (text/x-patch, attachment)]

[0003-gnu-Add-minetest-moreores.patch (text/x-patch, attachment)]

[0002-guix-minetest-build-system-Also-install-config.txt-a.patch (text/x-patch, attachment)]

[0001-guix-minetest-build-system-Save-the-full-test-log.patch (text/x-patch, attachment)]

Message #29 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Vivien Kraus <vivien <at> planete-kraus.eu>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Wed, 22 Sep 2021 19:41:33 +0200

[Message part 1 (text/plain, inline)]

Vivien Kraus schreef op wo 22-09-2021 om 18:46 [+0200]:
> +        (define log-file (open-output-file "guix-test.log"))
> +        (let loop ((has-errors? #f))
> +          (let ((line (read-line port)))
> +            (unless (eof-obj	ect? line)
> +              (write-line line log-file))
> +            (match `(,line ,has-errors?)
> +              (((? error? line) _)
> +               (loop #t))
> +              (((? stop?) #f)
> +               (kill pid SIGINT)
> +               (close-port port)
> +               (waitpid pid))
> +              (((? eof-object?) #f)
> +               (error "minetest didn't start"))
> +              (((or (? stop?) (? eof-object?)) #t)

'(close-port port)' is missing in this branch.
This is important if 'port' is buffered, to flush the buffers.
From a test at the Guile REPL, it appears ports from 'open-output-file'
are buffered.  

> +               (error "minetest stopped after an error happened, see source/guix_testworld/guix-test.log"))

This requires the user to do "--keep-failed".  It would be more convenient
if the output of Minetest was send to current-output-port, that way the
errors can be read from the build log (possibly at <https://ci.guix.gnu.org/>).

Suggestion: don't keep guix-test.log, or only print errors, but rather
output all ouputs of "minetest" to (current-output-port).

Alternative suggestion (to make building less verbose when there are no errors):
this patch, but run "cat" or a guile equivalent thereof (maybe open-input-file
+ sendfile + stat:size) on guix-test.log when there are errors.

> +              (((? string? line) has-error?)
> +               (display line)
> +               (newline)
> +               (loop has-error?)))))))))

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #32 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Maxime Devos <maximedevos <at> telenet.be>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Wed, 22 Sep 2021 20:04:40 +0200

[Message part 1 (text/plain, inline)]

Le mercredi 22 septembre 2021 à 19:41 +0200, Maxime Devos a écrit :
> It would be more convenient
> if the output of Minetest was send to current-output-port,
OK, so it’s like the existing solution, except it should not stop just
at the first error.

[0004-gnu-minetest-basic-materials-Depend-on-minetest-more.patch (text/x-patch, attachment)]

[0003-gnu-Add-minetest-moreores.patch (text/x-patch, attachment)]

[0002-guix-minetest-build-system-Also-install-config.txt-a.patch (text/x-patch, attachment)]

[0001-guix-minetest-build-system-Gather-the-whole-output-w.patch (text/x-patch, attachment)]

Message #35 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Vivien Kraus <vivien <at> planete-kraus.eu>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Wed, 22 Sep 2021 20:15:48 +0200

[Message part 1 (text/plain, inline)]

Vivien Kraus schreef op wo 22-09-2021 om 20:04 [+0200]:
> Le mercredi 22 septembre 2021 à 19:41 +0200, Maxime Devos a écrit :
> > It would be more convenient
> > if the output of Minetest was send to current-output-port,
> OK, so it’s like the existing solution, except it should not stop just
> at the first error.

The latest patch you sent looks good to me, but I didn't test ti.

Greetings

[signature.asc (application/pgp-signature, inline)]

Message #38 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Maxime Devos <maximedevos <at> telenet.be>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Wed, 22 Sep 2021 20:37:10 +0200

Le mercredi 22 septembre 2021 à 20:15 +0200, Maxime Devos a écrit :
> The latest patch you sent looks good to me, but I didn't test ti.

I tested it by comparing the build of minetest-moreores when it fails
because of the missing file and when it succeeds. Do you think it
should be enough if it were to be tested that way?

Vivien

Message #41 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Vivien Kraus <vivien <at> planete-kraus.eu>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Wed, 22 Sep 2021 20:43:55 +0200

[Message part 1 (text/plain, inline)]

Vivien Kraus schreef op wo 22-09-2021 om 20:37 [+0200]:
> Le mercredi 22 septembre 2021 à 20:15 +0200, Maxime Devos a écrit :
> > The latest patch you sent looks good to me, but I didn't test ti.
> 
> I tested it by comparing the build of minetest-moreores when it fails
> because of the missing file and when it succeeds. Do you think it
> should be enough if it were to be tested that way?

That should be enough I think.

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #44 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Vivien Kraus <vivien <at> planete-kraus.eu>
To: Maxime Devos <maximedevos <at> telenet.be>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Thu, 14 Oct 2021 17:14:04 +0200

Hello Maxime,

Le mercredi 22 septembre 2021 à 20:43 +0200, Maxime Devos a écrit :
> Vivien Kraus schreef op wo 22-09-2021 om 20:37 [+0200]:
> > Le mercredi 22 septembre 2021 à 20:15 +0200, Maxime Devos a écrit :
> > > The latest patch you sent looks good to me, but I didn't test ti.
> > 
> > I tested it by comparing the build of minetest-moreores when it
> > fails
> > because of the missing file and when it succeeds. Do you think it
> > should be enough if it were to be tested that way?
> 
> That should be enough I think.
> 
> Greetings,
> Maxime.

On IRC you said that you did one test. Aside from you, it seems that
noone cares about minetest.

Would you accept this patch? If not, then I’ll trash the error handler
modification and just keep the new packages. What do you think?

Best regards,

Vivien

Message #47 received at 50677 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Vivien Kraus <vivien <at> planete-kraus.eu>, 50677 <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Fri, 15 Oct 2021 20:18:02 +0000

Vivien Kraus schreef op do 14-10-2021 om 17:14 [+0200]:
> On IRC you said that you did one test. Aside from you, it seems that
> noone cares about minetest.
> 
> Would you accept this patch? [...]

IIRC, I tested the patch, it worked, and it looked good to me. 
However, I'm not a committer.

Anyway, I won't be working on Guix for a while (and I've unsubscribed
from the mailing lists) (though I intend to continue eventually), so I
won't be available for (minetest-related) reviews.

Greetings,
Maxime

--
The computer I'm typing this on doesn't have the secret key,
so the e-mail isn't signed.

Message #52 received at 50677-done <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: Vivien Kraus <vivien <at> planete-kraus.eu>
Cc: 50677-done <at> debbugs.gnu.org
Subject: Re: bug#50677: [PATCH] Minetest basic_materials really depends on
 moreores
Date: Sat, 16 Oct 2021 17:32:07 +0200

[Message part 1 (text/plain, inline)]

Vivien,

Thanks!  I've pushed this series as 
df2404482495bc3af03e7fc902af1e6b0d10b4a4 et al.

I made a only few small tweaks, mainly to the commit message:

> gnu: minetest-basic-materials: Depend on minetest-moreores.

‘Depend’ sounds wrong to me; it either does, or it doesn't, we 
don't determine that.  Changed to ‘Propagate …’

> * minetest.scm (minetest-basic-materials): Add minetest-moreores 
> as a propagated input.

Keep Git commit message body lines <=72 characters.  Most 
(non-Guix) tooling is built around that assumption.

Also use full relative file names, and use [field] notation where 
appropriate:

 * gnu/packages/foo.scm (foo)[propagated-inputs]: Add bar.

For the build system patches, I changed

> guix: minetest-build-system: […]

to

 build-system/minetest: […]

Why?  Because it's shorter.  Is this a documented rule?  I can't 
say!

> ;; basic_materials:silver_wire cannot be crafted without
> ;; moreores:silver_ingot.

Documenting this is a great idea.

> (commit "0b6f669df4c9b7771c03e0e6ba8effb471cdfcae")))

I changed this to

 (commit (string-append "v" version))

to use the git tag.

Both have advantages & disadvantages, but this is our currently 
preferred style.

> (synopsis "Adds new ore types")
> (description "More ores for Minetest.")

These aren't acceptable.  See (guix)Synopses and Descriptions.

This was my attempt at improving the importer's lazywork:

 (synopsis "Additional ore types, tools, swords, and rails for 
 Minetest")
 (description
  "This Minetest mod adds new ore types to the game (mithril, 
  silver) as well
as swords and tools made of different materials.  It also adds 
copper rails.")

Bit vague, but you know why.

Closing; thanks!

T G-R

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.