Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

games-roguelike/dwarf-fortress: add version 0.47.04, bugfix #17940

Closed

Conversation

sveneusewig
Copy link
Contributor

The bug has already been confirmed but has not yet been fixed.
The changes for the bug fix were posted by the user Cheaterman
https://www.bay12games.com/dwarves/mantisbt/view.php?id=11564

Signed-off-by: Sven Eusewig sveneusewig@yahoo.de
Bug: https://bugs.gentoo.org/729002
Package-Manager: Portage-3.0.4, Repoman-3.0.1

The bug has already been confirmed but has not yet been fixed.
The changes for the bug fix were posted by the user Cheaterman
https://www.bay12games.com/dwarves/mantisbt/view.php?id=11564

Signed-off-by: Sven Eusewig <sveneusewig@yahoo.de>
Bug: https://bugs.gentoo.org/729002
Package-Manager: Portage-3.0.4, Repoman-3.0.1
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @sveneusewig
Areas affected: ebuilds
Packages affected: games-roguelike/dwarf-fortress

games-roguelike/dwarf-fortress: @gentoo/games

Linked bugs

Bugs linked: 729002


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Oct 15, 2020
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-10-15 20:21 UTC
Newest commit scanned: a7aa4f5
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/bb9c686e5c/output.html

@Cheaterman
Copy link
Contributor

Thank you :-) 👍

@sveneusewig
Copy link
Contributor Author

@Cheaterman , @chewi
Who else needs to know?

Copy link
Contributor

@DarthGandalf DarthGandalf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also closes https://bugs.gentoo.org/717752 so please add it to commit message

@@ -43,6 +43,9 @@ RESTRICT="strip"
src_prepare() {
rm -f libs/*.so* || die
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove -f

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

@@ -1,12 +1,12 @@
# Copyright 1999-2020 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

EAPI=6
EAPI=7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In EAPI 7 BDEPEND should be split from DEPEND

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add this: BDEPEND="virtual/pkgconfig" and remove "virtual/pkgconfig" from DEPEND.

Bug: https://bugs.gentoo.org/729002
Reviewed-by: Alexey Sokolov <alexey+gentoo@asokolov.org>
Package-Manager: Portage-3.0.9, Repoman-3.0.2
Signed-off-by: Sven Eusewig <sveneusewig@yahoo.de>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-12-11 17:43 UTC
Newest commit scanned: cf30ca8
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/56ed3c2f80/output.html

Copy link
Member

@chewi chewi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't new but I really don't like how the launcher for this game just copies the entire thing into HOME. Is this really the only way it can work? I don't have the energy to argue with upstream about it.

sed -i -e '1i#include <cmath>' g_src/ttf_manager.cpp || die

eapply "${FILESDIR}/${P}-segfault-fix-729002.patch"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eapply is generally frowned upon now. You should be able to use PATCHES instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I remove this there.

@@ -31,8 +31,8 @@ RDEPEND="media-libs/glew:0
DEPEND="${RDEPEND}
media-libs/libsndfile
media-libs/openal
sys-libs/ncurses-compat:5[unicode]
virtual/pkgconfig"
sys-libs/ncurses-compat:5[unicode]"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you haven't changed this but ncurses-compat in DEPEND doesn't make any sense. This package does not install any headers. I can see that the sources are hardcoded to load libncurses.so.5. This is really bad if you're building with the headers from libncurses.so.6. I can see that it falls back to libncurses.so but that's no use if it finds .so.5 first. The fact that it builds means it must be compatible with .so.6 so I guess you should patch renderer_curses.cpp accordingly. This whole thing is really horrible though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see what I can find. In the worst case it looks like this:
https://i.imgur.com/Kx4NvI4.gif

@chewi
Copy link
Member

chewi commented Dec 27, 2020

This isn't new but I really don't like how the launcher for this game just copies the entire thing into HOME. Is this really the only way it can work? I don't have the energy to argue with upstream about it.

Ah sorry, I now realise this launcher script is our own. 😨 Presumably it was done like this for a reason though. Please find out whether it's still necessary.

libgraphics.so now opens libncursesw.so.6 first before
libncursesw.so.5 and others. DEPEND on ncurses-combat
has been replaced to ncurses.

Bug: https://bugs.gentoo.org/729002
Reviewed-by: James Le Cuirot <chewi@aura-online.co.uk>
Package-Manager: Portage-3.0.9, Repoman-3.0.2
Signed-off-by: Sven Eusewig <sveneusewig@yahoo.de>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-12-31 17:33 UTC
Newest commit scanned: 954787d
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/96b38fd85f/output.html

@rayment
Copy link
Contributor

rayment commented Feb 1, 2021

You may want to update this to 0.47.05 now that it's out. I've tested your patch in the new version with a custom libgraphic build and it still works.

@sveneusewig
Copy link
Contributor Author

I'm making a new pull request.

@sveneusewig sveneusewig closed this Apr 3, 2021
@sveneusewig sveneusewig deleted the games-roguelike/dwarf-fortress branch April 3, 2021 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR.
Projects
None yet
8 participants