media-tv/plex-media-server new ebuild #20320
Conversation
gentoo-bot
added
new package
| @@ -0,0 +1,8 @@ | |||
| --- /usr/share/applications/plexmediaserver.desktop.org 2020-05-23 01:39:34.105959787 -0500 | |||
There was a problem hiding this comment.
Let's trim patches where we can - drop the date parts. We also usually use patch level 1, so we should prefix the paths with a/ and b/ respectively.
| <email>proxy-maint@gentoo.org</email> | ||
| <name>Proxy Maintainers</name> | ||
| </maintainer> | ||
| <maintainer type="person"> |
There was a problem hiding this comment.
Put yourself first (before proxy-maint). We also need proxied="yes" for you
media-tv/plex-media-server/plex-media-server-1.22.2.4282.ebuild
Outdated
Show resolved
Hide resolved
| DESCRIPTION="A free media library that is intended for use with a plex client" | ||
| HOMEPAGE="https://www.plex.tv/" | ||
|
|
||
| _COMMIT="a97b03fad" |
There was a problem hiding this comment.
I'd prefer MY_COMMIT or just COMMIT, if that's okay.
| MY_PV="${PV}-${_COMMIT}" | ||
|
|
||
| URI="https://downloads.plex.tv/plex-media-server-new" | ||
| #URI="https://artifacts.plex.tv/plex-media-server-experimental/" |
There was a problem hiding this comment.
URI is not a standard variable and looks a bit confusing here. Maybe use MY_URI or MY_BASE_URI?
media-tv/plex-media-server/plex-media-server-1.22.2.4282.ebuild
Outdated
Show resolved
Hide resolved
media-tv/plex-media-server/plex-media-server-1.22.2.4282.ebuild
Outdated
Show resolved
Hide resolved
|
|
||
| #fix-gnustack -f "${ED%/}/usr/lib/plexmediaserver/lib/libgnsdk_dsp.so.3.10.1" || die | ||
|
|
||
| # pax-mark m "${ED%/}/usr/lib/plexmediaserver/Plex Script Host" || die |
There was a problem hiding this comment.
Let's drop lines like this if they're useless. Same for other commented lines.
There was a problem hiding this comment.
I removed all the commented old code.
| } | ||
|
|
||
| pkg_postinst() { | ||
| einfo "" |
There was a problem hiding this comment.
How about:
| einfo "" |
media-tv/plex-media-server/plex-media-server-1.22.2.4282.ebuild
Outdated
Show resolved
Hide resolved
| "usr/lib/plexmediaserver/Resources/Python/lib/python2.7/.*" | ||
| "usr/lib/plexmediaserver/Resources/Python/lib/python2.7/lib-dynload/_hashlib.so" | ||
| ) | ||
| "usr/lib/plexmediaserver/Resources/Python/lib/python2.7/.*" |
There was a problem hiding this comment.
Remember to test! All of these are part of the same array, so this is invalid syntax :)
We need to put these two quoted lines in the array defined above.
| unpack_deb ${A} | ||
| } | ||
|
|
||
| src_prepare() { |
| "${FILESDIR}/${PN}-1.22.2.4282-desktop-file.patch" | ||
| ) | ||
|
|
||
| S="${WORKDIR}" |
There was a problem hiding this comment.
I usually put this under SRC_URI but it's not a hard requirement.
| } | ||
|
|
||
| pkg_postinst() { | ||
| einfo "Plex Media Server is now installed. Please check the configuration file |
There was a problem hiding this comment.
I bet we want to either make multiple einfo calls here or, possibly even better, use readme.gentoo-r1?
There was a problem hiding this comment.
This should be in good shape, now.
media-tv/plex-media-server/plex-media-server-1.22.2.4282.ebuild
Outdated
Show resolved
Hide resolved
| cp "${FILESDIR}/start_pms" "${S}/usr/sbin/" || die | ||
|
|
||
| # Copy main files over to image and preserve permissions so it is portable | ||
| cp -rp usr/ "${ED}" || die |
There was a problem hiding this comment.
Oh, no, duh, we do need to keep this actually.
media-tv/plex-media-server/plex-media-server-1.22.2.4282.ebuild
Outdated
Show resolved
Hide resolved
| DESCRIPTION="A free media library that is intended for use with a plex client" | ||
| HOMEPAGE="https://www.plex.tv/" | ||
|
|
||
| MY_COMMIT="a97b03fad" |
There was a problem hiding this comment.
My personal preference is to put MY_* variables above DESCRIPTION, then have no whitespace between DESCRIPTION/HOMEPAGE/SRC_URI/S, then a newline, then LICENSE and friends. Up to you though.
|
Need to check CI failure and also squash all commits when we're done. Looking very good now! |
| @@ -0,0 +1 @@ | |||
| SEARCH_DIRS_MASK="/usr/lib64/plexmediaserver/lib /usr/lib/plexmediaserver/Resources/Python/lib/python2.7/lib-dynload /usr/lib/plexmediaserver/Resources/Python/lib/python2.7/site-packages/lxml" | |||
There was a problem hiding this comment.
On x86, do we need /usr/lib/plexmediaserver/lib?
There was a problem hiding this comment.
yes, however there is no /usr/lib64/plexmediaserver, so I need to fix that..
There was a problem hiding this comment.
This has been updated
|
This got worse the more I poked at it. I don't know what I need to do to fix this. I messed up on the header of the commit message and it looks like I still have not squashed the commit' correctly. |
|
It should be sane and working, now. Thanks everyone! |
|
You may want to add [please reassign] in PR's title in order to force bot to reassign labels |
ephemer0l
changed the title
|
@ephemer0l sorry I haven't told you... it must be at the end of title, not at start |
gentoo-bot
changed the title
gentoo-bot
added
new package
| "usr/lib/plexmediaserver/lib/.*" | ||
| "usr/lib/plexmediaserver/Resources/Python/lib/python2.7/.*" | ||
| "usr/lib/plexmediaserver/Resources/Python/lib/python2.7/lib-dynload/_hashlib.so" |
There was a problem hiding this comment.
These are whitespaces instead of tabs. Be sure to fix these (same thing at line 40)
| dodir /etc/revdep-rebuild | ||
| insinto /etc/revdep-rebuild | ||
| doins "${FILESDIR}"/etc/revdep-rebuild/80plexmediaserver |
There was a problem hiding this comment.
doins already creates dedicated folders, right @thesamesam? https://devmanual.gentoo.org/eclass-reference/ebuild/
ephemer0l
changed the title
gentoo-bot
changed the title
gentoo-bot
added
the
assigned
Pull request CI reportReport generated at: 2021-05-25 22:21 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
| <pkgmetadata> | ||
| <maintainer type="person" proxied="yes"> | ||
| <email>om@organizedmagnetism.com</email> | ||
| <name>Scott Martin</name> |
|
|
||
| inherit systemd unpacker | ||
|
|
||
| DESCRIPTION="A free media library that is intended for use with a plex client" |
There was a problem hiding this comment.
I prefer to drop the A, i.e. ="Free media...
| LICENSE="Plex" | ||
| SLOT="0" | ||
| KEYWORDS="-* ~amd64 ~x86" | ||
| RESTRICT="mirror bindist strip" |
There was a problem hiding this comment.
| RESTRICT="mirror bindist strip" | |
| RESTRICT="bindist mirror" |
I find adding strip to binary-only packages to be a poor trend I see across many packages. As I see it, this is for if you want to keep symbols (or if stripping breaks things) and not because it happens to be pre-stripped.
QA_PREBUILT is already there to prevent QA warnings regarding pre-stripped files.
Nevermind if you know of actual issues with stripping.
|
|
||
| # Remove shipped openssl library | ||
| if use system-openssl; then | ||
| rm -f usr/lib/plexmediaserver/libssl.so.1.0.0 || die |
There was a problem hiding this comment.
Without -f you would've been informed that this version doesn't ship with 1.0.0 anymore (uses 1.1) and this is deleting nothing.
|
|
||
| RDEPEND=" | ||
| acct-group/plex | ||
| acct-user/plex |
There was a problem hiding this comment.
acct-* is being used at build time (by the ebuild), should be in DEPEND as well.
| einfo "Plex Media Server is now installed. Please check the configuration file" | ||
| einfo "it can be found in /etc/plex/plexmediaserver to verify the default settings." | ||
| einfo "To start the Plex Server, run 'rc-config start plex-media-server'" | ||
| einfo "You will then be able to access your library at http://localhost:32400/manage" |
There was a problem hiding this comment.
einfo has very little visibility, odds are nobody will see this unless they look at the build log.
A show-only-on-first-install using ${REPLACING_VERSIONS} and elog may be fair, or (as sam already suggested) readme.gentoo-r1 if you want to add more things, show it once, and keep it as reference.
|
|
||
| src_install() { | ||
| # Remove Debian specific files | ||
| rm -r "${S}/usr/share/doc" || die |
There was a problem hiding this comment.
| rm -r "${S}/usr/share/doc" || die | |
| rm -r usr/share/doc || die |
| MY_COMMIT="6119e8eed" | ||
| MY_PV="${PV}-${MY_COMMIT}" |
There was a problem hiding this comment.
Let's take nitpicking this one further... seeing how it's only used once, maybe combine?
But if you feel MY_COMMIT makes this clearer or easier to edit on bumps, leaving it is fine too.
| MY_COMMIT="6119e8eed" | |
| MY_PV="${PV}-${MY_COMMIT}" | |
| MY_PV="${PV}-6119e8eed" |
| fi | ||
|
|
||
| # Add startup wrapper | ||
| dosbin "${FILESDIR}/start_pms" || die |
There was a problem hiding this comment.
dosbin already handles || die internally
| doinitd "${FILESDIR}"/init.d/${PN} | ||
| doconfd "${FILESDIR}"/conf.d/${PN} |
There was a problem hiding this comment.
I see these are in subdirectories, e.g. files/conf.d/plex-media-server.
I'd prefer if they were, e.g. files/plex-media-server.confd then
newconfd "${FILESDIR}"/${PN}.confd ${PN}
Not that there's a hard rule against this, but subdirectories is an uncommon practice.
ephemer0l
changed the title
gentoo-bot
changed the title
gentoo-bot
added
new package
ephemer0l
changed the title
gentoo-bot
changed the title
Pull Request assignmentSubmitter: @ephemer0l media-tv/plex-media-server: @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. New packagesThis Pull Request appears to be introducing new packages only. Due to limited manpower, adding new packages is considered low priority. This does not mean that your Pull Request will not receive any attention, however, it might take quite some time for it to be reviewed. In the meantime, your new ebuild might find a home in the GURU project repository: the ebuild repository maintained collaboratively by Gentoo users. GURU offers your ebuild a place to be reviewed and improved by other Gentoo users, while making it easy for Gentoo users to install it and enjoy the software it adds. 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 |
gentoo-bot
added
new package
Pull request CI reportReport generated at: 2021-05-26 15:11 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
There was a problem hiding this comment.
If getting lost in comments, may want to have a look at
https://github.com/gentoo/gentoo/pull/20320/files
To see if anything still applies.
| if use system-openssl; then | ||
| rm usr/lib/plexmediaserver/libssl.so.1.0.0 || die | ||
| fi |
There was a problem hiding this comment.
That you removed -f is nice, but did you test this with USE=system-openssl? There was a second part to this.
| doinitd "${FILESDIR}/${PN}.init.d" | ||
| doconfd "${FILESDIR}/${PN}.conf.d" |
There was a problem hiding this comment.
Good to see directory gone, but this would install them as ${PN}.conf.d rather than ${PN}.
I previously suggested newconfd (and newinitd) to handle that.
https://devmanual.gentoo.org/function-reference/install-functions/
| DESCRIPTION="Free media library that is intended for use with a plex client" | ||
| HOMEPAGE="https://www.plex.tv/" | ||
|
|
||
| MY_PV="${PV}-6119e8eed" | ||
|
|
||
| MY_URI="https://downloads.plex.tv/plex-media-server-new" |
There was a problem hiding this comment.
Not a hard requirement but it would be nice to re-arrange these to keep the usual variable block together.
Not everyone does the same but usually I'd do something like:
inherit ...
MY_PV=
MY_URI=
DESCRIPTION=
HOMEPAGE=
SRC_URI=
S=
LICENSE=
|
For some extra notes
|
There was a problem hiding this comment.
Looks almost there, just few more small things.
Also, in the commit message, bugrefs are normally right above the sign-off line with appropriate GLEP66 format, e.g.
app/foo: summary
text wrapped at ~72 characters with blank line before and after
Bug: https://bugs.gentoo.org/735396
Signed-off-by: ...
https://devmanual.gentoo.org/ebuild-maintenance/git/index.html#git-commit-message-format
media-tv/plex-media-server/Manifest
Outdated
| DIST plexmediaserver_1.23.1.4571-6119e8eed_amd64.deb 81587156 BLAKE2B 009cb6c849ff13b7f4d5a1d079fc5761a1c16727133782d203b760db653d3e843abaf556e1829721db0c2caf78ba4321a83548d0d9a83e869688e54d585410af SHA512 7f59327fb091df4a45c401acb477344128c4330fde2bd7c45fd2d07078fc1f1f3d432055756c3bc354e35cfa47b42e0d01e7233cd8ddf67dd5e4f461934bee6c | ||
| DIST plexmediaserver_1.23.1.4571-6119e8eed_i386.deb 74605040 BLAKE2B 8c0d6565fe5b4a1d0c7641a4ede7564d7e6b9d8a74ae40d8caba967156ab0d1a50e9bb80475cb270b23e656f22f15533c41da8e8a7c87c638b5e81ee29a8ba9d SHA512 69e5330f487d1d500380ded607ddaf9d5a586984839d87e9922e7355f307c5704eba0a3ed5308d627b64d7dee55f5d89869b7bc1080c32b841358b5ef312eabe | ||
| EBUILD plex-media-server-1.23.1.4571.ebuild 1958 BLAKE2B dddf8c1c7af474ebf4daa843c9bee055d66da93d53851a793c1d845da73bb7bc15ecf70d6a55534a912463625dd13426ea778aaf4a51e8522e6ef613debc56bd SHA512 b64e3c571f3a60deb229e6fd8d86bb922623d3cd8f8e4f088ae9dc9b32c03c968ac0d7843582ebe9e56e8f5273b7744551081797461a46023e74c55c08df14de | ||
| MISC metadata.xml 746 BLAKE2B 32ddcdfc95a50cd336dec02a232ed50eeeca7d74d410cd327068da50fa55ac2090338854cff3fbc1da5c5c7027f0114db9c007cdb4ffc4426bf69ee8a5bdba59 SHA512 0f95894ecb5a28aaa0254435ad9b29552623e33340a23358ed3287c8e23d6a98e3cde838916f279970990ccc0113bd662a0c9d32259be3a50fec70fcd1ae2530 |
There was a problem hiding this comment.
The previous Manifest you gave only had the two DIST lines (and that was right), it seems like you used the rsync tree or an overlay set to thin-manifests = false (in metadata/layout.conf) to generate this.
|
|
||
| SRC_URI=" | ||
| amd64? ( ${MY_URI}/${MY_PV}/debian/plexmediaserver_${MY_PV}_amd64.deb ) | ||
| x86? ( ${MY_URI}/${MY_PV}/debian/plexmediaserver_${MY_PV}_i386.deb )" | ||
|
|
||
| S="${WORKDIR}" |
There was a problem hiding this comment.
Just nitpicks again, but usually no blank line between HOMEPAGE / SRC_URI and SRC_URI / S
| acct-user/plex" | ||
| RDEPEND=" | ||
| ${DEPEND} | ||
| system-openssl? ( dev-libs/openssl )" |
There was a problem hiding this comment.
Overlooked that before, but be good to use dev-libs/openssl:0/1.1 since it's the only one it can use.
Pull request CI reportReport generated at: 2021-05-26 22:36 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
| # Adds the precompiled plex libraries to the revdep-rebuild's mask list | ||
| # so it doesn't try to rebuild libraries that can't be rebuilt. | ||
| insinto /etc/revdep-rebuild | ||
| doins "${FILESDIR}"/etc/revdep-rebuild/80plexmediaserver |
There was a problem hiding this comment.
I missed that one, it's also using a files/ subdirectories for nothing, I kind of wonder who still use revdep-rebuild though.
(on a side-note, bugref in commit message is missing its Bug: https://... prefix tag, these matter for auto-detection by bots and so on)
Pull request CI reportReport generated at: 2021-05-26 22:56 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
This package does bundle all deps, and it's not ported to py3 it can run standalone. Bug: https://bugs.gentoo.org/735396 Signed-off-by: Scott Martin <om@organizedmagnetism.com>
Pull request CI reportReport generated at: 2021-05-29 23:31 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Signed-off-by: Scott Martin om@organizedmagnetism.com