-
Notifications
You must be signed in to change notification settings - Fork 2k
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/gzdoom: New package #11967
Conversation
Copyright policy changePlease 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 assignmentSubmitter: @vilhelmgray games-fps/gzdoom: @gentoo/proxy-maint (new package) Linked bugsNo bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment. In order to force reassignment and/or bug reference scan, please append Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide |
64f0041
to
7e605e1
Compare
ea3a064
to
e41ab8b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the ebuild. Almost LGTM, just a few changes. I hope I didn't miss anything.
games-fps/gzdoom/gzdoom-4.1.2.ebuild
Outdated
|
||
src_configure() { | ||
local mycmakeargs=( | ||
-DINSTALL_DOCS_PATH="${EPREFIX}/usr/share/doc/${P}/docs" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DINSTALL_DOCS_PATH="${EPREFIX}/usr/share/doc/${PF}"
And -DINSTALL_PK3_PATH="${EPREFIX}/usr/share/doom"
So it won't install files into /usr/share/games/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the 20151213 Gentoo council meeting summary states:
The council voted in favor of deprecating the /usr/games and /etc/games
directories. Games packages should not install any files there, but
follow the normal guidelines for install locations instead. Two
exceptions are made: (a) Games packages can install files in
/usr/share/games (instead of /usr/share) if that is the location used
by upstream. (b) Shared high-score or game state files can be placed
in /var/games or a subdirectory of it.
/usr/share/games
is used by GZDoom upstream. Since this is one of the exceptions allowed for game packages, should we permit GZDoom to install files there to keep it in sync with upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any drawbacks in placing these files to /usr/share/doom
? I see that by default the program searches the files in both directories:
[FileSearch.Directories]
Path=$HOME/.config/gzdoom
Path=/usr/local/share/
Path=/usr/local/share/doom
Path=/usr/local/share/games/doom
Path=/usr/share/doom
Path=/usr/share/games/doom
Path=$DOOMWADDIR
Or have I missed something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, you are correct. In that case /usr/share/doom
shouldn't be much of a problem. The latest force push I did should be complete then. If there are any problems you notice, let me know.
<maintainer type="project"> | ||
<email>proxy-maint@gentoo.org</email> | ||
<name>Proxy Maintainers</name> | ||
</maintainer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+ <maintainer type="project">
+ <email>games@gentoo.org</email>
+ <name>Gentoo Games Project</name>
+ </maintainer>
games-fps/gzdoom/gzdoom-4.1.2.ebuild
Outdated
|
||
S="${WORKDIR}/${PN}-g${PV}" | ||
|
||
PATCHES="${FILESDIR}/${PV}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please:
PATCHES=( "${FILESDIR}"/${P}-static-libraries.patch )
games-fps/gzdoom/gzdoom-4.1.2.ebuild
Outdated
media-libs/libsdl2[opengl] | ||
sys-libs/zlib | ||
virtual/jpeg:0 | ||
gtk? ( <x11-libs/gtk+-4:= )" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "="?
And why not x11-libs/gtk+:3
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GZDoom supports GTK+ version 2 and version 3. In order to allow the ebuild to support both slots, I made the dependency be <x11-libs/gtk+-4:=
. The reason for =
is to cause a rebuild if the user switches slots between gtk+:3
and gtk+:2
.
I can remove support for the gtk+:2
slot if you think that would be better; for what it's worth, I don't think much of the GZDoom team is using GTK+ version 2 anymore, despite support for it still in the code. Is gtk+:2
deprecated for Gentoo packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. =
forces rebuild if a subslot is changed. It is not needed, gtk+ has no subslots at all, it will only slow down portage.
If gtk+-3 is the primary version now, but gtk+-2 is supported then add a local USE flag gtk2
:
gtk? (
!gtk2? ( x11-libs/gtk+:3 )
gtk2? ( x11-libs/gtk+:2 )
)
(or USE=gtk3 if otherwise)
PATCHES="${FILESDIR}/${PV}" | ||
|
||
src_prepare() { | ||
cmake-utils_src_prepare |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do rm -rf docs/licenses || die
here, so the license files won't be installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a license used for some of the math functions in GZDoom that I'm unsure how to handle: https://github.com/coelckers/gzdoom/blob/master/docs/licenses/cephes.txt
Should I merge this license file into gentoo/licenses
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add it to licenses/cephes
in a separate commit before adding the package. For reference see section «Adding New Licenses» here: https://devmanual.gentoo.org/general-concepts/licenses/index.html
And https://cgit.gentoo.org/repo/gentoo.git/log/licenses
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
bd3af25
to
aa78852
Compare
Package-Manager: Portage-2.3.65, Repoman-2.3.12 Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Pull request CI reportReport generated at: 2019-06-01 17:33 UTC Issues already there before the PR (double-check them): |
Amended and pushed. Thanks. |
Package-Manager: Portage-2.3.65, Repoman-2.3.12
Signed-off-by: William Breathitt Gray vilhelm.gray@gmail.com
This package has also been introduced to the GURU overlay repository. I'll force-push any improvements to this package from GURU overlay repository to this Github pull request.