Bug 843413 – games-roguelike/stone-soup[tiles]: BDEPEND on either app-arch/advancecomp or media-gfx/pngcrush

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Brian Wong 2022-05-09 11:17:15 UTC]

At build time, Dungeon Crawl Stone Soup can use either advpng (provided in app-arch/advancecomp) or pngcrush (media-gfx/pngcrush); see bug URL.

Furthermore, this dependency is not required at runtime, only at build time. I'm not sure which of the two produces better results (smaller files in /usr/share/stone-soup-0.28/dat/tiles/), but I'd want to say that the dependency should be listed as a BDEPEND, not DEPEND, for either of these two packages.

[Comment 1 Brian Wong 2022-05-09 11:21:33 UTC]

The commit supporting this was made as early as 0.8.0-a1: https://github.com/crawl/crawl/commit/87b6e4cb8eef72a38189625f5d0219b714ef739c

There might need to be some testing to verify that this does not break the game at runtime to confirm that it can be a BDEPEND.

[Comment 2 Erik Mackdanz 2022-05-21 20:01:14 UTC]

Thanks for flagging this.  I'll be able to take a look in the next few days.

[Comment 3 Erik Mackdanz 2022-05-21 21:54:30 UTC]

pngcrush is already a build-time dependency in DEPEND.  Runtime dependencies are in RDEPEND.  I'm willing to split out BDEPEND from DEPEND but both are for build-time-only deps and are only subtly different: https://devmanual.gentoo.org/general-concepts/dependencies/#build-dependencies

Note that the Makefile will run both: pngcrush followed by advpng, if both are available, and the comment for the commit you linked indicates there's benefit to using both.

The real bug here is the use of "shell which pngcrush" and "shell which advpng" to test for what's already available on the system.  Sniffing tests like that are forbidden in Gentoo and ebuilds must patch them out so that builds run in a deterministic way on different systems.  In this case, given one system with advpng installed and another system without it, different .png files would be installed even with identical USE flags.  That's a bug.

With some effort, those shell tests could be patched out and the two dependencies placed behind two different USE flags, and users could do science to figure out which program shrank the .png files more.

OR both can be put into BDEPEND, which takes almost no effort, doesn't explode USE, and should produce files that are even smaller than either program alone.  I'd rather do this.

[Comment 4 Larry the Git Cow 2022-05-23 00:04:15 UTC]

The bug has been closed via the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=ad15f92151f402b95fc352d61b55d037efe2d5bb

commit ad15f92151f402b95fc352d61b55d037efe2d5bb
Author:     Erik Mackdanz <[e-mail]>
AuthorDate: 2022-05-22 23:34:01 +0000
Commit:     Erik Mackdanz <[e-mail]>
CommitDate: 2022-05-23 00:04:25 +0000

    games-roguelike/stone-soup: add USE=advpng
    
    ... as a .png compression option.  Patches makes the advpng/pngcrush
    decision deterministic not automagic.
    
    Also split DEPEND/BDEPEND
    
    Signed-off-by: Erik Mackdanz <[e-mail]>
    Closes: https://bugs.gentoo.org/843413
    Package-Manager: Portage-3.0.30, Repoman-3.0.3

 games-roguelike/stone-soup/files/make-advpng.patch |  21 ++
 .../stone-soup/files/make-no-png-dep-fix.patch     |  94 +++++++++
 games-roguelike/stone-soup/files/make.patch        |  45 ++++-
 games-roguelike/stone-soup/metadata.xml            |   6 +-
 .../stone-soup/stone-soup-0.25.1-r102.ebuild       |   2 +-
 .../stone-soup/stone-soup-0.25.1-r103.ebuild       | 203 +++++++++++++++++++
 .../stone-soup/stone-soup-0.26.1-r1.ebuild         |   2 +-
 .../stone-soup/stone-soup-0.26.1-r2.ebuild         | 207 +++++++++++++++++++
 .../stone-soup/stone-soup-0.27.1-r1.ebuild         | 203 +++++++++++++++++++
 .../stone-soup/stone-soup-0.27.1.ebuild            |   2 +-
 .../stone-soup/stone-soup-0.28.0-r1.ebuild         | 222 +++++++++++++++++++++
 .../stone-soup/stone-soup-0.28.0.ebuild            |   2 +-
 12 files changed, 993 insertions(+), 16 deletions(-)

[Comment 5 Erik Mackdanz 2022-05-23 00:09:18 UTC]

I'd read the Makefile wrong.  It is set up to use either pngcrush or advpng but not both.

I added USE=advpng, not enabled by default

The dat/tiles dir results:
5788k with advpng
6056k with pngcrush

... so not a big difference either way, and not entirely worth the effort.

I'd already done the work by that point though, and anyway it was necessary to remove the automagic dependency detection for those two tools.

[Comment 6 Sam James 2022-05-23 00:20:38 UTC]

(Tangent: we're trying to kill off `which` usage in Gentoo, so it had another silver lining: bug 646588. :))

[Comment 7 Brian Wong 2022-05-23 03:26:05 UTC]

Huh. TIL about the `which` issue. Thanks :)