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-fps/freedoom: Add doom engine runtime dependency #12225

Closed
wants to merge 7 commits into from

Conversation

vilhelmgray
Copy link
Contributor

Freedoom is distributed by upstream with the intention to be run by a
doom engine; this is also the expectation of end users. This patch adds
a doom engine as a runtime dependency for Freedoom.

Three possible doom engines are listed in RDEPEND: games-fps/doomsday,
games-fps/gzdoom, and games-fps/prboom. The games-fps/gzdoom is set to
preferred as it is the recommendation of the Freedoom upstream team.

Closes: https://bugs.gentoo.org/687672
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray vilhelm.gray@gmail.com

@gentoo-bot
Copy link

Copyright policy change

Please note that on 2018-09-15 Trustees have approved new Gentoo copyright policy. All contributions made to Gentoo need to follow this policy. If you include the Signed-off-by line in your commit message, you indicate that you have read the policy and agree to its terms. For more detailed explanation, please see the new Gentoo copyright policy explained article.

Pull Request assignment

Submitter: @vilhelmgray
Areas affected: ebuilds
Packages affected: games-fps/freedoom

games-fps/freedoom: @gentoo/games

Linked bugs

Bugs linked: 687672


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 Jun 9, 2019
@vilhelmgray vilhelmgray force-pushed the freedoom-rdepend branch 5 times, most recently from 1fb46da to 11154d2 Compare June 9, 2019 09:35
@a17r
Copy link
Member

a17r commented Jun 9, 2019

There is no reason to do this in a PR separate from #12224 and cause twice the CI load.

@a17r
Copy link
Member

a17r commented Jun 10, 2019

Does it make sense to keep the old version (broken, presumably, without the RDEPEND), or would you add a separate commit to drop -r0?

@vilhelmgray
Copy link
Contributor Author

vilhelmgray commented Jun 10, 2019

Does it make sense to keep the old version (broken, presumably, without the RDEPEND), or would you add a separate commit to drop -r0?

The -r0 version has the benefit of being available for ~arm; I had to drop this arch in the new -r1 since the doom engines aren't available for ~arm.

If that's not a big deal for anyone, we can remove the -r0 version. I'll do so for this pull request in a separate patch.

@a17r
Copy link
Member

a17r commented Jun 10, 2019

Does it make sense to keep the old version (broken, presumably, without the RDEPEND), or would you add a separate commit to drop -r0?

The -r0 version has the benefit of being available for ~arm; I had to drop this arch in the new -r1 since the doom engines aren't available for ~arm.

Is it correct to assume that ~arm should have never been keyworded then?

doins */*.wad
dodoc "${P}"/CREDITS.txt "${P}"/README.html
}

pkg_postinst() {
einfo "Please note that WAD files location is /usr/share/doom-data/${PN}"
einfo "Please note that the WAD files location is ${FREEDOOMWADPATH}"
Copy link
Member

Choose a reason for hiding this comment

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

If this should be known to users of the package after installation, elog should be used instead.

@vilhelmgray
Copy link
Contributor Author

Does it make sense to keep the old version (broken, presumably, without the RDEPEND), or would you add a separate commit to drop -r0?

The -r0 version has the benefit of being available for ~arm; I had to drop this arch in the new -r1 since the doom engines aren't available for ~arm.

Is it correct to assume that ~arm should have never been keyworded then?

No, many doom engines do in fact support ARM -- I don't know about Doomsday, but GZDoom and Odamex certainly do -- so the Gentoo ARM team just needs to add the ~arm keyword for those respective doom engine ebuilds.

However, since none of the existing doom engine packages listed in the freedoom ebuild RDEPEND list have ~arm keyworded, I had to remove freedoom's ~arm keyword.

My preference would be to keep the ~arm keyword, but I'm not familiar with the process to get it added for the respective doom engine ebuilds. Would I simply create a new Bugzilla issue requesting it for the respective package?

@a17r
Copy link
Member

a17r commented Jun 10, 2019

Is it correct to assume that ~arm should have never been keyworded then?

No, many doom engines do in fact support ARM -- I don't know about Doomsday, but GZDoom and Odamex certainly do -- so the Gentoo ARM team just needs to add the ~arm keyword for those respective doom engine ebuilds.

Fine, but I'm looking at it from Gentoo ebuild repo POV - if it was only possible to keyword ~arm because the package had missing dependencies, then it was wrong to begin with.

My preference would be to keep the ~arm keyword, but I'm not familiar with the process to get it added for the respective doom engine ebuilds. Would I simply create a new Bugzilla issue requesting it for the respective package?

Yes, that's how it works - open a new KEYWORDREQ for the necessary package(s).

@vilhelmgray
Copy link
Contributor Author

Is it correct to assume that ~arm should have never been keyworded then?

No, many doom engines do in fact support ARM -- I don't know about Doomsday, but GZDoom and Odamex certainly do -- so the Gentoo ARM team just needs to add the ~arm keyword for those respective doom engine ebuilds.

Fine, but I'm looking at it from Gentoo ebuild repo POV - if it was only possible to keyword ~arm because the package had missing dependencies, then it was wrong to begin with.

Ah, I see what you mean now. I suppose you're right if no existing doom engine ebuild in Gentoo's repository is keyworded for ~arm. The PRBoom engine provided by games-fps/prboom package does have the ~arm keyword -- and it's possible that it can run Freedoom version 0.11.3 -- but since PrBoom is an abandoned project, the Freedoom team has stopped recommending it, and so I did not include it in the RDEPEND list.

@chewi
Copy link
Member

chewi commented Jun 10, 2019

I have thought about adding PrBoom+, which is more or less still maintained, but it's not a priority.

I don't entirely agree with adding engines dependencies to data ebuilds, especially when Doom has so many engines, but I don't feel too strongly about it. virtual/doom wouldn't make sense here as Freedoom is only supported by some engines.

@a17r
Copy link
Member

a17r commented Jun 10, 2019

Regarding prboom+, there's this: https://bugs.gentoo.org/338027

@vilhelmgray
Copy link
Contributor Author

I don't entirely agree with adding engines dependencies to data ebuilds, especially when Doom has so many engines, but I don't feel too strongly about it. virtual/doom wouldn't make sense here as Freedoom is only supported by some engines.

I think virtual/doom would be a good idea since it'll help simplify the maintenance of this doom engine list. So I'll implement it and update this patch accordingly.

I can see what you mean about the separation of data and engine. Since there are so many engines, it may be possible the user wants to use one that doesn't have a Gentoo ebuild, but that doesn't mean they should be forced to install some other engine just to get the Freedoom data.

Perhaps we can implement a games-fps/freedoom-data to get the data, and then reimplement games-fps/freedoom with a BDEPEND="games-fps/freedoom-data" and RDEPEND="virtual/doom". That should allow users to install games-fps/freedoom-data if they want just the data files, while also allowing games-fps/freedoom to be installed as the complete functional game as intended by upstream.

@a17r
Copy link
Member

a17r commented Jun 10, 2019

You could also simply test for known doom engines at the end of freedoom install (pkg_postinst) and warn the user if none are installed. Without adding any dependencies. That's how it is done for e.g. games-board/xboard which will accept various chess engines (or simply be used to connect to multiplayer games instead).

@vilhelmgray
Copy link
Contributor Author

vilhelmgray commented Jun 11, 2019

I realized what @chewi said is true -- virtual/doom wouldn't work here since only certain doom engines support the features required to run the Freedoom single-player campaigns. So I won't implement virtual/doom for now.

You could also simply test for known doom engines at the end of freedoom install (pkg_postinst) and warn the user if none are installed. Without adding any dependencies. That's how it is done for e.g. games-board/xboard which will accept various chess engines (or simply be used to connect to multiplayer games instead).

Regarding a doom engine dependency for Freedoom, I've actually been going back and forth on whether there should be or not. As noted, there a some packages which simply notify the user of compatible engines (e.g. games-board/xboard). But I don't believe these programs and engines are in the same relation as Freedoom is with doom engines.

Take for example the mentioned games-board/xboard package. I haven't used XBoard before, but from the description it looks to be a frontend for several popular chess engines -- it's effectively the graphical data component for a chess game. However, XBoard appears to be designed with independence in mind, running purposefully separate from the underlying chess engine and communicating via a well-defined interface.

The key point is that XBoard is wholly a separate program from the chess engine -- the two programs are just able to work together. So it naturally makes sense for XBoard to not depend on a chess engine.

Freedoom on the other hand is not really separate from a doom engine. The fact that there are multiple doom engines that support Freedoom is a consequence of the doom engines forking from a common codebase. Freedoom is designed with integration in mind, utilizing features of certain doom engines not because there is a standard interface but because those features happen to be available when combined.

For this reason it makes sense to have a doom engine be a runtime dependency, since a doom engine is required to run Freedoom in the way intended and designed by the upstream project.

However, I can understand the problems of adding a runtime dependency. There are in fact cases where installing Freedoom by itself would be preferred:

  • To use with a doom engine not available from Gentoo
  • To modify the Freedoom WADs via wad composers such as games-util/deutex
  • etc.

Because of that, I'd like to see a games-fps/freedoom-data package in addition to a games-fps/freedoom package. However, if such a setup would not be useful, then I'd prefer to keep the doom engine runtime dependency for games-fps/freedoom as is in this pull request.

@vilhelmgray
Copy link
Contributor Author

Currently I have the Freedoom WAD files install into /usr/share/doom/${P}. Since there is no name conflict between the Freedoom WAD files and traditional DOOM WAD files, would you have any opposition if I point the install location instead to the canonical /usr/share/doom directory?

@a17r
Copy link
Member

a17r commented Jun 11, 2019

At that point I'll have to hand over to @chewi, I'm not into the doom (or games at all, for that matter) stuff.

@chewi
Copy link
Member

chewi commented Jun 13, 2019

Currently I have the Freedoom WAD files install into /usr/share/doom/${P}. Since there is no name conflict between the Freedoom WAD files and traditional DOOM WAD files, would you have any opposition if I point the install location instead to the canonical /usr/share/doom directory?

I see that /usr/share/doom-data/${PN} is what we have for Freedoom now and I'm guessing /usr/share/doom is what upstream prefers? Both games-fps/doom-data and games-fps/prboom use /usr/share/doom-data so that's what I'd prefer for consistency, either with or without the subdirectory.

Note that games-misc/yadex references /usr/share/doom-data/freedoom/freedm.wad although I don't like how that hard depends on Freedoom. Upstream is dead so I was feeling inclined to last-rite it as the other editors are probably better but I see that it was fixed up just a year ago using patches from Fedora.

I like your games-fps/freedoom-data data package and games-fps/freedoom meta package idea although BDEPEND is not the right thing to use. It should be RDEPEND.

@vilhelmgray
Copy link
Contributor Author

vilhelmgray commented Jun 14, 2019

I see that /usr/share/doom-data/${PN} is what we have for Freedoom now and I'm guessing /usr/share/doom is what upstream prefers? Both games-fps/doom-data and games-fps/prboom use /usr/share/doom-data so that's what I'd prefer for consistency, either with or without the subdirectory.

I believe /usr/share/games/doom//usr/share/doom is the de facto standard for doom engines. Of the engines I've checked, it's the default search path for PrBoom, Doomsday, Odamex, GZDoom, Crispy Doom, and Chocolate Doom.

I believe what happened in Gentoo is that the games-fps/doom-data ebuild was created and the author chose to place the DOOM WADs in /usr/share/doom-data because it matches the ebuild name; later the games-fps/prboom ebuild was added with /usr/share/doom-data because that was where the games-fps/doom-data ebuild was installing the WADs.

I can update the games-fps/doom-data ebuild to install to the expected /usr/share/doom path so that the installation directory is consistent across the doom engine ebuilds; games-fps/doom-data needs to be updated anyway to EAPI 7, and I think the doomsday USE flag should be removed since it just serves to install games-fps/doomsday (with some magic to work around the existing nonstandard /usr/share/doom-data path).

Note that games-misc/yadex references /usr/share/doom-data/freedoom/freedm.wad although I don't like how that hard depends on Freedoom. Upstream is dead so I was feeling inclined to last-rite it as the other editors are probably better but I see that it was fixed up just a year ago using patches from Fedora.

I'd be surprised if anyone is still using this (hasn't the upstream project been dead since the early 2000s?). Other forks of DEU are more popular nowadays, such as games-util/deutex which I plan on version bumping if no one else does (it's needed to compile Freedoom from source).

In addition, it would be good to drop games-fps/prboom as well if an ebuild is introduced for its successor PrBoom+.

I like your games-fps/freedoom-data data package and games-fps/freedoom meta package idea although BDEPEND is not the right thing to use. It should be RDEPEND.

Ah yes, you're right it should be RDEPEND. I'll make these changes and force-push when I get the chance.

Closes: https://bugs.gentoo.org/687674
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
@vilhelmgray
Copy link
Contributor Author

I combined the FreeDM pull request with this one, and also reimplemented it the same as the Freedoom packages with games-fps/freedm-data and games-fps/freedm.

@vilhelmgray vilhelmgray force-pushed the freedoom-rdepend branch 2 times, most recently from 34a0436 to e44a2d4 Compare June 14, 2019 11:51
Bug: https://bugs.gentoo.org/687674
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Update maintainership info and add long description.

Closes: https://bugs.gentoo.org/687670
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Freedoom is distributed by upstream with the intention to be run by a
doom engine; this is also the expectation of end users. This patch adds
a doom engine as a runtime dependency for Freedoom.

Three possible doom engines are listed in RDEPEND: games-engines/odamex,
games-fps/doomsday, games-fps/gzdoom. The games-fps/gzdoom package is
set to preferred as it is the recommendation of the Freedoom upstream
team.

Closes: https://bugs.gentoo.org/687672
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Remove doomsday USE flag since users can install games-fps/doomsday
directly. Install location changed to the /usr/share/doom directory.

Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2019-06-14 12:30 UTC
Newest commit scanned: 07c7640
Status: ✅ good

All QA issues have been fixed!

@chewi
Copy link
Member

chewi commented Jun 18, 2019

Merged, many thanks for your awesome work.

I'd be surprised if anyone is still using this (hasn't the upstream project been dead since the early 2000s?). Other forks of DEU are more popular nowadays, such as games-util/deutex

You tell me! I haven't really made any DOOM levels since 1996, probably with ye olde DEU 5.21. 😉 Anyway, I've taken your word for it and last-rited Yadex.

@chewi
Copy link
Member

chewi commented Jun 18, 2019

@vilhelmgray, since you seem keen on the DOOM stuff, I'd be grateful if you pick up the prboom+ ebuild. It'll need updating to EAPI 7 before I can merge it.

@vilhelmgray
Copy link
Contributor Author

@chewi I have a prboom-plus ebuild that is just about ready for a pull request, but I discovered upstream failed to provide some assets in their source tarball. I can access the assets from the upstream Subversion repository (revision 4459), so I'm considering just creating a snapshot of the revision of the corresponding latest release and using that. Is it possible to host it on the Gentoo servers -- what is the process to do so?

@chewi
Copy link
Member

chewi commented Jun 20, 2019

Ideally that should be fixed upstream but the GZDoom guy has a mirror on GitHub. Normally I don't trust random mirrors but he seems trustworthy. Taking snapshots from here should include all the content.

@vilhelmgray
Copy link
Contributor Author

@chewi The prboom-plus pull request can be found here: #12297

You should be able to last-rite games-fps/prboom once games-fps/prboom-plus is merged.

If you get a chance, check out the pull requests for Crispy Doom and Chocolate Doom as well. Chocolate Doom in particular is important for speedrun competitions which often require the behavior of the original DOOM release.

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
5 participants