View previous topic :: View next topic |
Author |
Message |
paluszak Apprentice
Joined: 28 Jun 2004 Posts: 265 Location: Warsaw, Poland
|
Posted: Sat Sep 29, 2012 3:00 pm Post subject: my first ebuild - procexp-1.0.178 - please review |
|
|
Hello,
I wrote my very first ebuild for a graphical top-like tool. I would be grateful if someone a tad more experienced have a look before I submit it to bugzilla. TIA.
J.
Code: | # Copyright 1999-2012 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
EAPI="3"
inherit eutils
DESCRIPTION="Process Explorer for Linux, inspired by a Windows process explorer made by Mark Russinovich."
HOMEPAGE="http://sourceforge.net/apps/mediawiki/procexp/index.php?title=Main_Page"
SRC_URI="mirror://sourceforge/project/procexp/sources_v1.0/${PN}_${PV}.orig.tar.gz"
LICENSE="GPL-2"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE="network"
PYTHON_DEPEND="2:2.6"
RESTRICT_PYTHON_ABIS="2.*"
inherit python
RDEPEND="dev-python/PyQt4 \
dev-python/configobj \
dev-python/pyqwt \
network? ( sys-apps/ethtool )"
DEPEND="${RDEPEND}"
pkg_setup() {
python_set_active_version 2
}
src_prepare() {
mv -f procexp/icon.png procexp/procexp.png
sed -e 's|Icon=/usr/share/icons/procexp/icon.png|Icon=/usr/share/pixmaps/procexp.png|' < procexp/procexp.desktop > ${T}/procexp.desktop
rm -f procexp/procexp.desktop
sed -e 's|python|python2|' < procexp/procexp.sh > ${T}/procexp.sh
rm -f procexp/procexp.sh
rm -f procexp/process_explorer.spec
rm -f procexp/make_rpm.py
find procexp/ -type d -name .svn -print0 | xargs -0 /bin/rm -rf
}
src_install() {
doicon procexp/procexp.png
rm -f procexp/procexp.png
domenu ${T}/procexp.desktop
mkdir ${D}/usr/bin
mv -f ${T}/procexp.sh ${D}/usr/bin/
chmod +x ${D}/usr/bin/procexp.sh
mkdir ${D}/usr/lib
mkdir ${D}/usr/lib/procexp
cp -Rf ${WORKDIR}/procexp/* ${D}/usr/lib/procexp/
}
pkg_postinst() {
python_mod_optimize ${ROOT}usr/lib/procexp/
if use network; then
elog Run as root to take advantage of network monitoring features \(via ethtool\).
fi
}
pkg_postrm() {
python_mod_cleanup ${ROOT}usr/lib/procexp/
} |
|
|
Back to top |
|
|
John R. Graham Administrator
Joined: 08 Mar 2005 Posts: 10589 Location: Somewhere over Atlanta, Georgia
|
Posted: Sat Sep 29, 2012 3:59 pm Post subject: |
|
|
Hi, @paluszak. Although you'll get help here too, for quicker feedback, you may want to contact the Gentoo Sunrise folks on IRC at #gentoo-sunrise.
- John _________________ I can confirm that I have received between 0 and 499 National Security Letters.
Last edited by John R. Graham on Mon Oct 01, 2012 6:44 pm; edited 1 time in total |
|
Back to top |
|
|
phajdan.jr Retired Dev
Joined: 23 Mar 2006 Posts: 1777 Location: Poland
|
Posted: Sun Sep 30, 2012 8:59 am Post subject: Re: my first ebuild - procexp-1.0.178 - please review |
|
|
nit: Why not EAPI="4"?
paluszak wrote: | Code: |
src_prepare() {
mv -f procexp/icon.png procexp/procexp.png
sed -e 's|Icon=/usr/share/icons/procexp/icon.png|Icon=/usr/share/pixmaps/procexp.png|' < procexp/procexp.desktop > ${T}/procexp.desktop
rm -f procexp/procexp.desktop
sed -e 's|python|python2|' < procexp/procexp.sh > ${T}/procexp.sh
rm -f procexp/procexp.sh
rm -f procexp/process_explorer.spec
rm -f procexp/make_rpm.py
find procexp/ -type d -name .svn -print0 | xargs -0 /bin/rm -rf
}
|
|
You need || die after each of those commands. Instead of piping find's output to xargs, just tell find to delete things (or we might have code for that in some eclass).
Are you sure all this code in src_prepare() is needed? Have you considered contacting upstream to make their build system more distro-friendly?
paluszak wrote: | Code: |
src_install() {
doicon procexp/procexp.png
rm -f procexp/procexp.png
domenu ${T}/procexp.desktop
mkdir ${D}/usr/bin
mv -f ${T}/procexp.sh ${D}/usr/bin/
chmod +x ${D}/usr/bin/procexp.sh
mkdir ${D}/usr/lib
mkdir ${D}/usr/lib/procexp
cp -Rf ${WORKDIR}/procexp/* ${D}/usr/lib/procexp/
}
|
|
Same here, you need || die, and doesn't upstream provide make install?
paluszak wrote: | Code: |
pkg_postinst() {
python_mod_optimize ${ROOT}usr/lib/procexp/
if use network; then
elog Run as root to take advantage of network monitoring features \(via ethtool\).
fi
}
|
|
Why don't you quote the elog message in double quotes? _________________ http://phajdan-jr.blogspot.com/ |
|
Back to top |
|
|
Hu Moderator
Joined: 06 Mar 2007 Posts: 21633
|
Posted: Sun Sep 30, 2012 3:36 pm Post subject: Re: my first ebuild - procexp-1.0.178 - please review |
|
|
phajdan.jr wrote: | paluszak wrote: | Code: | src_prepare() {
rm -f procexp/procexp.desktop
rm -f procexp/procexp.sh
rm -f procexp/process_explorer.spec
rm -f procexp/make_rpm.py
}
|
| You need || die after each of those commands. | OP: beware that rm -f will silently exit success on attempts to remove a missing file, while rm without a -f will exit failure on attempts to remove a missing file. If you add the || die suggested by phajdan.jr, you may also want to remove the -f if you want to detect when the files do not exist. |
|
Back to top |
|
|
paluszak Apprentice
Joined: 28 Jun 2004 Posts: 265 Location: Warsaw, Poland
|
Posted: Sun Sep 30, 2012 4:01 pm Post subject: Re: my first ebuild - procexp-1.0.178 - please review |
|
|
phajdan.jr wrote: |
nit: Why not EAPI="4"? | I updated it to EAPI=4. Quote: | paluszak wrote: | Code: |
src_prepare() {
mv -f procexp/icon.png procexp/procexp.png
sed -e 's|Icon=/usr/share/icons/procexp/icon.png|Icon=/usr/share/pixmaps/procexp.png|' < procexp/procexp.desktop > ${T}/procexp.desktop
rm -f procexp/procexp.desktop
sed -e 's|python|python2|' < procexp/procexp.sh > ${T}/procexp.sh
rm -f procexp/procexp.sh
rm -f procexp/process_explorer.spec
rm -f procexp/make_rpm.py
find procexp/ -type d -name .svn -print0 | xargs -0 /bin/rm -rf
}
|
|
You need || die after each of those commands. Instead of piping find's output to xargs, just tell find to delete things (or we might have code for that in some eclass).
|
I added || die and found escn_clean in eutils. Quote: |
Are you sure all this code in src_prepare() is needed? Have you considered contacting upstream to make their build system more distro-friendly?
| Just a raw tarball and couple of debs and rpms on the project page. Quote: |
paluszak wrote: | Code: |
src_install() {
doicon procexp/procexp.png
rm -f procexp/procexp.png
domenu ${T}/procexp.desktop
mkdir ${D}/usr/bin
mv -f ${T}/procexp.sh ${D}/usr/bin/
chmod +x ${D}/usr/bin/procexp.sh
mkdir ${D}/usr/lib
mkdir ${D}/usr/lib/procexp
cp -Rf ${WORKDIR}/procexp/* ${D}/usr/lib/procexp/
}
|
|
Same here, you need || die, and doesn't upstream provide make install?
| I added || die, for the second question, see above. Quote: |
paluszak wrote: | Code: |
pkg_postinst() {
python_mod_optimize ${ROOT}usr/lib/procexp/
if use network; then
elog Run as root to take advantage of network monitoring features \(via ethtool\).
fi
}
|
|
Why don't you quote the elog message in double quotes? | Done.
Hu wrote: | phajdan.jr wrote: | paluszak wrote: | Code: | src_prepare() {
rm -f procexp/procexp.desktop
rm -f procexp/procexp.sh
rm -f procexp/process_explorer.spec
rm -f procexp/make_rpm.py
}
|
| You need || die after each of those commands. | OP: beware that rm -f will silently exit success on attempts to remove a missing file, while rm without a -f will exit failure on attempts to remove a missing file. If you add the || die suggested by phajdan.jr, you may also want to remove the -f if you want to detect when the files do not exist. | Thanks, I removed the -f's, streamlined it a little bit (rm accepts more than one file to remove) and made it multilib-friendly.
Now it looks like this: Code: | # Copyright 1999-2012 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
EAPI="4"
PYTHON_DEPEND="2:2.6"
RESTRICT_PYTHON_ABIS="2.*"
inherit eutils python multilib
DESCRIPTION="Process Explorer for Linux, inspired by a Windows process explorer made by Mark Russinovich."
HOMEPAGE="http://sourceforge.net/apps/mediawiki/procexp/index.php?title=Main_Page"
SRC_URI="mirror://sourceforge/project/procexp/sources_v1.0/${PN}_${PV}.orig.tar.gz"
LICENSE="GPL-2"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE="network"
RDEPEND="dev-python/PyQt4 \
dev-python/configobj \
dev-python/pyqwt \
network? ( sys-apps/ethtool )"
src_unpack() {
unpack ${A}
mv procexp ${P} || die
}
pkg_setup() {
python_set_active_version 2
}
src_prepare() {
mv -f icon.png ${T}/procexp.png || die
sed -e "s|Icon=/usr/share/icons/procexp/icon.png|Icon=/usr/share/pixmaps/procexp.png|" < procexp.desktop > ${T}/procexp.desktop || die
sed -e "s|python /usr/lib/procexp/procexp.py|python2 /usr/$(get_libdir)/procexp/procexp.py|" < procexp.sh > ${T}/procexp.sh || die
rm procexp.desktop procexp.sh process_explorer.spec make_rpm.py || die
esvn_clean
}
src_install() {
doicon ${T}/procexp.png
domenu ${T}/procexp.desktop
mkdir ${D}/usr/bin || die
mv -f ${T}/procexp.sh ${D}/usr/bin/ || die
chmod +x ${D}/usr/bin/procexp.sh || die
if [ ! -d ${D}/usr/$(get_libdir) ]; then
mkdir ${D}/usr/$(get_libdir) || die
fi
mkdir ${D}/usr/$(get_libdir)/${PN} || die
cp -Rf ${WORKDIR}/${P}/* ${D}/usr/$(get_libdir)/${PN}/ || die
}
pkg_postinst() {
python_mod_optimize ${ROOT}usr/$(get_libdir)/${PN}/
if use network; then
elog "Run as root to take advantage of network monitoring features (via ethtool)."
fi
}
pkg_postrm() {
python_mod_cleanup ${ROOT}usr/$(get_libdir)/${PN}/
} |
|
|
Back to top |
|
|
dol-sen Retired Dev
Joined: 30 Jun 2002 Posts: 2805 Location: Richmond, BC, Canada
|
Posted: Sun Sep 30, 2012 8:36 pm Post subject: |
|
|
I had a look at the source. It should have a setup.py and install itself into site-packages properly instead of on it's own. Also instead of the procexp.sh script, the procexp.py file can be installed into /usr/bin without the .py extension as long as it has the correct shebang. In that way it can properly be installed to multiple python versions if it supports it. As well, then python-updater would work for it too. The current install method, it would likely need manual updating, rebuilding. _________________ Brian
Porthole, the Portage GUI frontend irc@freenode: #gentoo-guis, #porthole, Blog
layman, gentoolkit, CoreBuilder, esearch... |
|
Back to top |
|
|
steveL Watchman
Joined: 13 Sep 2006 Posts: 5153 Location: The Peanut Gallery
|
Posted: Sun Sep 30, 2012 11:11 pm Post subject: |
|
|
On a far more fundamental level, you must learn to quote parameter expansions.
unpack ${A} should be: unpack "${A}" or (more simply to read and write): unpack "$A". You only need the ${} construction when the next character in the script would otherwise be considered part of the variable name (so a letter, number or underscore), eg echo "${foo}bar". Or when you're doing manipulative expansions like echo "${foo#*bar}".
mv -f icon.png ${T}/procexp.png should be: mv -f icon.png "${T}/procexp.png"
mkdir ${D}/usr/bin should be: mkdir "${D}/usr/bin" and so on.
Otherwise, if there are spaces in the directory names, the shell will split the parameter into two or more arguments (at the spaces.) Additionally any * or ? will be matched as a wildcard character, as will anything between [ and ] in the string. (This last part about wildcarding is not so urgent here, but is still something you need to know about if shell-scripting.)
http://www.grymoire.com/Unix/Quote.html is also good. Usually, you should quote every parameter expansion ($foo).
I would strongly recommend you join #bash on chat.freenode.org to learn more about portable bash scripting. It's worth it, believe me. |
|
Back to top |
|
|
phajdan.jr Retired Dev
Joined: 23 Mar 2006 Posts: 1777 Location: Poland
|
Posted: Mon Oct 01, 2012 8:08 am Post subject: Re: my first ebuild - procexp-1.0.178 - please review |
|
|
paluszak wrote: | Code: | src_unpack() {
unpack ${A}
mv procexp ${P} || die
}
|
|
You can just set $S instead.
steveL wrote: | unpack ${A} should be: unpack "${A}" |
While your general comment about quoting is right, the above example is wrong. Do not quote $A! See http://devmanual.gentoo.org/ebuild-writing/functions/src_unpack/index.html . The reason is that A may contain multiple URLs, space-delimited. _________________ http://phajdan-jr.blogspot.com/ |
|
Back to top |
|
|
paluszak Apprentice
Joined: 28 Jun 2004 Posts: 265 Location: Warsaw, Poland
|
Posted: Mon Oct 01, 2012 5:05 pm Post subject: |
|
|
dol-sen wrote: | I had a look at the source. It should have a setup.py and install itself into site-packages properly instead of on it's own. Also instead of the procexp.sh script, the procexp.py file can be installed into /usr/bin without the .py extension as long as it has the correct shebang. In that way it can properly be installed to multiple python versions if it supports it. As well, then python-updater would work for it too. The current install method, it would likely need manual updating, rebuilding. | 1. Sadly, the upstream doesn't support any setup.py
2. The package supports only python 2:2.6 as defined in the ebuild.
3. Doesn't calling python_need_rebuild mark the package as needing a rebuild in case python-updater is called?
steveL wrote: | On a far more fundamental level, you must learn to quote parameter expansions.
unpack ${A} should be: unpack "${A}" or (more simply to read and write): unpack "$A". You only need the ${} construction when the next character in the script would otherwise be considered part of the variable name (so a letter, number or underscore), eg echo "${foo}bar". Or when you're doing manipulative expansions like echo "${foo#*bar}".
mv -f icon.png ${T}/procexp.png should be: mv -f icon.png "${T}/procexp.png"
mkdir ${D}/usr/bin should be: mkdir "${D}/usr/bin" and so on.
Otherwise, if there are spaces in the directory names, the shell will split the parameter into two or more arguments (at the spaces.) Additionally any * or ? will be matched as a wildcard character, as will anything between [ and ] in the string. (This last part about wildcarding is not so urgent here, but is still something you need to know about if shell-scripting.)
http://www.grymoire.com/Unix/Quote.html is also good. Usually, you should quote every parameter expansion ($foo).
I would strongly recommend you join #bash on chat.freenode.org to learn more about portable bash scripting. It's worth it, believe me. | Thanks, I put the quotes around everything that might require it. And thanks for the links. phajdan.jr wrote: | paluszak wrote: | Code: | src_unpack() {
unpack ${A}
mv procexp ${P} || die
}
|
|
You can just set $S instead. | I got the same idea after posting here.
So the ebuild now looks like this (after making it profix-friendly): Code: | # Copyright 1999-2012 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
EAPI="4"
PYTHON_DEPEND="2:2.6"
RESTRICT_PYTHON_ABIS="2.*"
inherit eutils python multilib
DESCRIPTION="Process Explorer for Linux, inspired by a Windows process explorer made by Mark Russinovich."
HOMEPAGE="http://sourceforge.net/apps/mediawiki/procexp/index.php?title=Main_Page"
SRC_URI="mirror://sourceforge/project/procexp/sources_v1.0/${PN}_${PV}.orig.tar.gz"
LICENSE="GPL-2"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE="network"
RDEPEND="dev-python/PyQt4 \
dev-python/configobj \
dev-python/pyqwt \
network? ( sys-apps/ethtool )"
S="${WORKDIR}/procexp"
pkg_setup() {
python_set_active_version 2
python_pkg_setup
}
src_prepare() {
mv -f icon.png "${T}/procexp.png" || die
sed -e "s|Icon=/usr/share/icons/procexp/icon.png|Icon=${EROOT}usr/share/pixmaps/procexp.png|" < procexp.desktop > "${T}/procexp.desktop" || die
sed -e "s|python /usr/lib/procexp/procexp.py|python2 ${EROOT}usr/$(get_libdir)/procexp/procexp.py|" < procexp.sh > "${T}/procexp.sh" || die
rm procexp.desktop procexp.sh process_explorer.spec make_rpm.py || die
esvn_clean
}
src_install() {
doicon "${T}/procexp.png"
domenu "${T}/procexp.desktop"
mkdir "${ED}/usr/bin" || die
mv -f "${T}/procexp.sh" "${ED}/usr/bin/" || die
chmod +x "${ED}/usr/bin/procexp.sh" || die
if [ ! -d "${ED}/usr/$(get_libdir)" ]; then
mkdir "${ED}/usr/$(get_libdir)" || die
fi
mkdir "${ED}/usr/$(get_libdir)/${PN}" || die
cp -Rf "${S}/"* "${ED}/usr/$(get_libdir)/${PN}/" || die
}
pkg_postinst() {
python_need_rebuild
python_mod_optimize "${EROOT}usr/$(get_libdir)/${PN}/"
if use network; then
elog "Run as root to take advantage of network monitoring features (via ethtool)."
fi
}
pkg_postrm() {
python_mod_cleanup "${EROOT}usr/$(get_libdir)/${PN}/"
} |
|
|
Back to top |
|
|
steveL Watchman
Joined: 13 Sep 2006 Posts: 5153 Location: The Peanut Gallery
|
Posted: Tue Oct 02, 2012 6:39 am Post subject: Re: my first ebuild - procexp-1.0.178 - please review |
|
|
Ah my bad, thanks for picking up on it. |
|
Back to top |
|
|
phajdan.jr Retired Dev
Joined: 23 Mar 2006 Posts: 1777 Location: Poland
|
Posted: Tue Oct 02, 2012 10:55 am Post subject: |
|
|
I see the ebuild has quite improved now, nice progress.
paluszak wrote: | 1. Sadly, the upstream doesn't support any setup.py |
Thanks for checking. However, I insist on asking upstream to change that. One part of high-quality packaging (it's essential, you seriously cannot do without it, except truly trivial cases) is good cooperation with upstream (that includes sending them patches etc). By the way, it also makes the ebuild maintainable - more code in upstream's build system, less in the ebuild etc.
paluszak wrote: | 2. The package supports only python 2:2.6 as defined in the ebuild. |
That's fine IMO.
paluszak wrote: | Code: | S="${WORKDIR}/procexp" |
|
I think you could use $PN above.
paluszak wrote: | Code: | mv -f icon.png "${T}/procexp.png" || die |
|
Is the -f above necessary?
paluszak wrote: | Code: |
sed -e "s|Icon=/usr/share/icons/procexp/icon.png|Icon=${EROOT}usr/share/pixmaps/procexp.png|" < procexp.desktop > "${T}/procexp.desktop" || die
sed -e "s|python /usr/lib/procexp/procexp.py|python2 ${EROOT}usr/$(get_libdir)/procexp/procexp.py|" < procexp.sh > "${T}/procexp.sh" || die
|
|
Are those sed calls truly necessary for the package to work or for other important reasons? Are you aware of sed's --in-place (-i) option?
paluszak wrote: | Code: |
rm procexp.desktop procexp.sh process_explorer.spec make_rpm.py || die
|
|
Consider adding a comment above to explain why the files are being removed.
paluszak wrote: | Code: |
src_install() {
doicon "${T}/procexp.png"
domenu "${T}/procexp.desktop"
mkdir "${ED}/usr/bin" || die
mv -f "${T}/procexp.sh" "${ED}/usr/bin/" || die
chmod +x "${ED}/usr/bin/procexp.sh" || die
if [ ! -d "${ED}/usr/$(get_libdir)" ]; then
mkdir "${ED}/usr/$(get_libdir)" || die
fi
mkdir "${ED}/usr/$(get_libdir)/${PN}" || die
cp -Rf "${S}/"* "${ED}/usr/$(get_libdir)/${PN}/" || die
}
|
|
-f option for mv and cp shouldn't be needed. Similarly, you shouldn't need the "if" - the staging directory is initially empty, so you should know what's there. Also, it's better to use "mkdir -p". There are also useful helpers like dodir, newdir, doins, insinto, etc. _________________ http://phajdan-jr.blogspot.com/ |
|
Back to top |
|
|
paluszak Apprentice
Joined: 28 Jun 2004 Posts: 265 Location: Warsaw, Poland
|
Posted: Tue Oct 02, 2012 12:42 pm Post subject: |
|
|
phajdan.jr wrote: | I see the ebuild has quite improved now, nice progress. |
Thanks.
phajdan.jr wrote: | paluszak wrote: | 1. Sadly, the upstream doesn't support any setup.py |
Thanks for checking. However, I insist on asking upstream to change that. One part of high-quality packaging (it's essential, you seriously cannot do without it, except truly trivial cases) is good cooperation with upstream (that includes sending them patches etc). By the way, it also makes the ebuild maintainable - more code in upstream's build system, less in the ebuild etc. |
I opened an artifact on the project's tracker.
phajdan.jr wrote: | paluszak wrote: | 2. The package supports only python 2:2.6 as defined in the ebuild. |
That's fine IMO.
paluszak wrote: | Code: | S="${WORKDIR}/procexp" |
|
I think you could use $PN above. | Makes sense phajdan.jr wrote: | paluszak wrote: | Code: | mv -f icon.png "${T}/procexp.png" || die |
|
Is the -f above necessary? | Not really. I got rid of all mv's anyway. phajdan.jr wrote: | paluszak wrote: | Code: |
sed -e "s|Icon=/usr/share/icons/procexp/icon.png|Icon=${EROOT}usr/share/pixmaps/procexp.png|" < procexp.desktop > "${T}/procexp.desktop" || die
sed -e "s|python /usr/lib/procexp/procexp.py|python2 ${EROOT}usr/$(get_libdir)/procexp/procexp.py|" < procexp.sh > "${T}/procexp.sh" || die
|
|
Are those sed calls truly necessary for the package to work or for other important reasons? Are you aware of sed's --in-place (-i) option? | 1. I got rid of the unnecessary shell script. Adding a shebang, setting executable bit abd creating a symlink seem more elegant to me.
2. sed'ing procexp.desktop is not necessary per se, but without correct paths in *.desktop a menu entry would be broken.
3. I forgot about it sed -i. phajdan.jr wrote: | paluszak wrote: | Code: |
rm procexp.desktop procexp.sh process_explorer.spec make_rpm.py || die
|
|
Consider adding a comment above to explain why the files are being removed. | Added some explanation. phajdan.jr wrote: | paluszak wrote: | Code: |
src_install() {
doicon "${T}/procexp.png"
domenu "${T}/procexp.desktop"
mkdir "${ED}/usr/bin" || die
mv -f "${T}/procexp.sh" "${ED}/usr/bin/" || die
chmod +x "${ED}/usr/bin/procexp.sh" || die
if [ ! -d "${ED}/usr/$(get_libdir)" ]; then
mkdir "${ED}/usr/$(get_libdir)" || die
fi
mkdir "${ED}/usr/$(get_libdir)/${PN}" || die
cp -Rf "${S}/"* "${ED}/usr/$(get_libdir)/${PN}/" || die
}
|
|
-f option for mv and cp shouldn't be needed. Similarly, you shouldn't need the "if" - the staging directory is initially empty, so you should know what's there. Also, it's better to use "mkdir -p". There are also useful helpers like dodir, newdir, doins, insinto, etc. | I changed everything into doins, doicon, dodir etc, got rid of if/fi and a direct call to cp. Now it looks like this: Code: | # Copyright 1999-2012 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
EAPI="4"
PYTHON_DEPEND="2:2.6"
RESTRICT_PYTHON_ABIS="2.*"
inherit eutils python multilib
DESCRIPTION="Process Explorer for Linux, inspired by a Windows process explorer made by Mark Russinovich."
HOMEPAGE="http://sourceforge.net/apps/mediawiki/procexp/index.php?title=Main_Page"
SRC_URI="mirror://sourceforge/project/procexp/sources_v1.0/${PN}_${PV}.orig.tar.gz"
LICENSE="GPL-2"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE="network"
RDEPEND="dev-python/PyQt4 \
dev-python/configobj \
dev-python/pyqwt \
network? ( sys-apps/ethtool )"
S="${WORKDIR}/${PN}"
pkg_setup() {
python_set_active_version 2
python_pkg_setup
}
src_prepare() {
einfo "Setting correct paths and adding shebang..."
sed -e "s|Icon=/usr/share/icons/procexp/icon.png|Icon=${EROOT}usr/share/pixmaps/procexp.png|" \
-e "s|Exec=procexp.sh|Exec=procexp|" -i procexp.desktop || die
sed -e '1i#!/usr/bin/env python2' -i procexp.py || die
einfo "Removing unnecessary svn cruft and packaging system..."
esvn_clean
rm procexp.sh make_rpm.py process_explorer.spec || die
}
src_install() {
newicon icon.png procexp.png
rm icon.png || die
domenu procexp.desktop
rm procexp.desktop || die
dodir "/usr/$(get_libdir)/${PN}"
insinto "/usr/$(get_libdir)/${PN}/"
doins -r *
dodir /usr/bin
dosym "${EROOT}usr/$(get_libdir)/${PN}/procexp.py" /usr/bin/procexp
chmod +x "${ED}usr/$(get_libdir)/${PN}/procexp.py" || die
}
pkg_postinst() {
python_need_rebuild
python_mod_optimize "${EROOT}usr/$(get_libdir)/${PN}/"
if use network; then
elog "Run as root to take advantage of network monitoring features (via ethtool)."
fi
}
pkg_postrm() {
python_mod_cleanup "${EROOT}usr/$(get_libdir)/${PN}/"
} |
|
|
Back to top |
|
|
phajdan.jr Retired Dev
Joined: 23 Mar 2006 Posts: 1777 Location: Poland
|
Posted: Tue Oct 02, 2012 2:14 pm Post subject: |
|
|
paluszak wrote: | Code: |
RDEPEND="dev-python/PyQt4 \
dev-python/configobj \
dev-python/pyqwt \
network? ( sys-apps/ethtool )"
|
|
Those backslashes ("\") inside variables are not really needed and should be removed.
paluszak wrote: | Code: |
einfo "Setting correct paths and adding shebang..."
einfo "Removing unnecessary svn cruft and packaging system..."
|
|
The above lines are not really needed unless the process takes noticeably long (e.g. few seconds) on a reasonably fast machine. In my earlier feedback I meant adding a shell comment (starting with #), not einfo.
paluszak wrote: | Code: |
sed -e '1i#!/usr/bin/env python2' -i procexp.py || die
|
|
There should be some function to handle the shebang in the python eclass.
Ah, and ideally put the link to upstream bug report about the build system in a comment inside the ebuild. _________________ http://phajdan-jr.blogspot.com/ |
|
Back to top |
|
|
paluszak Apprentice
Joined: 28 Jun 2004 Posts: 265 Location: Warsaw, Poland
|
Posted: Tue Oct 02, 2012 3:00 pm Post subject: |
|
|
phajdan.jr wrote: | paluszak wrote: | Code: |
RDEPEND="dev-python/PyQt4 \
dev-python/configobj \
dev-python/pyqwt \
network? ( sys-apps/ethtool )"
|
|
Those backslashes ("\") inside variables are not really needed and should be removed. | Yep, I saw it somewhere and put it here without thinking. Removed. phajdan.jr wrote: | paluszak wrote: | Code: |
einfo "Setting correct paths and adding shebang..."
einfo "Removing unnecessary svn cruft and packaging system..."
|
|
The above lines are not really needed unless the process takes noticeably long (e.g. few seconds) on a reasonably fast machine. In my earlier feedback I meant adding a shell comment (starting with #), not einfo. | Ok. phajdan.jr wrote: | paluszak wrote: | Code: |
sed -e '1i#!/usr/bin/env python2' -i procexp.py || die
|
|
There should be some function to handle the shebang in the python eclass. | As far as I could find there's only a function to convert shebangs (python_convert_shebangs). However, there's no shebang at all in the executable python file provided by upstream. phajdan.jr wrote: | Ah, and ideally put the link to upstream bug report about the build system in a comment inside the ebuild. |
Code: | # Copyright 1999-2012 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
EAPI="4"
PYTHON_DEPEND="2:2.6"
RESTRICT_PYTHON_ABIS="2.*"
inherit eutils python multilib
DESCRIPTION="Process Explorer for Linux, inspired by a Windows process explorer made by Mark Russinovich."
HOMEPAGE="http://sourceforge.net/apps/mediawiki/procexp/index.php?title=Main_Page"
SRC_URI="mirror://sourceforge/project/procexp/sources_v1.0/${PN}_${PV}.orig.tar.gz"
LICENSE="GPL-2"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE="network"
RDEPEND="dev-python/PyQt4
dev-python/configobj
dev-python/pyqwt
network? ( sys-apps/ethtool )"
S="${WORKDIR}/${PN}"
# No build system provided by upstream, bug filed: https://sourceforge.net/tracker/?func=detail&aid=3573774&group_id=309156&atid=1301952
pkg_setup() {
python_set_active_version 2
python_pkg_setup
}
src_prepare() {
# Setting correct paths and adding shebang.
sed -e "s|Icon=/usr/share/icons/procexp/icon.png|Icon=${EROOT}usr/share/pixmaps/procexp.png|" \
-e "s|Exec=procexp.sh|Exec=procexp|" -i procexp.desktop || die
sed -e '1i#!/usr/bin/env python2' -i procexp.py || die
# Removing unnecessary svn cruft, packaging system and shell script.
esvn_clean
rm procexp.sh make_rpm.py process_explorer.spec || die
}
src_install() {
newicon icon.png procexp.png
rm icon.png || die
domenu procexp.desktop
rm procexp.desktop || die
dodir "/usr/$(get_libdir)/${PN}"
insinto "/usr/$(get_libdir)/${PN}/"
doins -r *
dodir /usr/bin
dosym "${EROOT}usr/$(get_libdir)/${PN}/procexp.py" /usr/bin/procexp
chmod +x "${ED}usr/$(get_libdir)/${PN}/procexp.py" || die
}
pkg_postinst() {
python_need_rebuild
python_mod_optimize "${EROOT}usr/$(get_libdir)/${PN}/"
if use network; then
elog "Run as root to take advantage of network monitoring features (via ethtool)."
fi
}
pkg_postrm() {
python_mod_cleanup "${EROOT}usr/$(get_libdir)/${PN}/"
} |
|
|
Back to top |
|
|
phajdan.jr Retired Dev
Joined: 23 Mar 2006 Posts: 1777 Location: Poland
|
|
Back to top |
|
|
paluszak Apprentice
Joined: 28 Jun 2004 Posts: 265 Location: Warsaw, Poland
|
|
Back to top |
|
|
John R. Graham Administrator
Joined: 08 Mar 2005 Posts: 10589 Location: Somewhere over Atlanta, Georgia
|
Posted: Wed Oct 03, 2012 2:29 pm Post subject: |
|
|
If no developer is initially interested in maintaining it, that's a good route. Doesn't hurt to do it anyway and makes it available to others through a standard mechanism: layman and the the sunrise overlay.
- John _________________ I can confirm that I have received between 0 and 499 National Security Letters. |
|
Back to top |
|
|
paluszak Apprentice
Joined: 28 Jun 2004 Posts: 265 Location: Warsaw, Poland
|
Posted: Wed Oct 03, 2012 3:28 pm Post subject: |
|
|
Guys on #gentoo-sunrise gave me some additional tips and the latest version looks like this: Code: | # Copyright 1999-2012 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
EAPI="4"
PYTHON_DEPEND="2:2.6"
inherit eutils python multilib gnome2-utils
DESCRIPTION="Process Explorer for Linux, inspired by a Windows process explorer made by Mark Russinovich"
HOMEPAGE="http://sourceforge.net/apps/mediawiki/procexp/index.php?title=Main_Page"
SRC_URI="mirror://sourceforge/project/procexp/sources_v1.0/${PN}_${PV}.orig.tar.gz"
LICENSE="GPL-2"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE=""
RDEPEND="dev-python/PyQt4
dev-python/configobj
dev-python/pyqwt"
S="${WORKDIR}/${PN}"
# No build system provided by upstream, bug filed: https://sourceforge.net/tracker/?func=detail&aid=3573774&group_id=309156&atid=1301952
pkg_setup() {
python_set_active_version 2
python_pkg_setup
}
src_prepare() {
# Adding shebang.
sed -e '1i#!/usr/bin/env python2' -i procexp.py || die
# Removing unnecessary svn cruft, packaging system and shell script.
esvn_clean
rm procexp.desktop procexp.sh make_rpm.py process_explorer.spec || die
}
src_install() {
newicon -s 48 icon.png procexp.png
rm icon.png || die
make_desktop_entry ${PN} "Linux Process Explorer" /usr/share/icons/hicolor/48x48/apps/${PN}.png "System;Utility;"
dodir "/usr/$(get_libdir)/${PN}"
insinto "/usr/$(get_libdir)/${PN}/"
doins -r *
dodir /usr/bin
dosym "${EROOT}usr/$(get_libdir)/${PN}/procexp.py" /usr/bin/procexp
fperms +x "/usr/$(get_libdir)/${PN}/procexp.py" || die
}
pkg_preinst() {
gnome2_icon_savelist
}
pkg_postinst() {
python_need_rebuild
python_mod_optimize "${EROOT}usr/$(get_libdir)/${PN}/"
gnome2_icon_cache_update
elog "Emerge sys-apps/ethtool and run as root to take advantage of network monitoring features (via ethtool)."
}
pkg_postrm() {
python_mod_cleanup "${EROOT}usr/$(get_libdir)/${PN}/"
gnome2_icon_cache_update
} |
|
|
Back to top |
|
|
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
|