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.

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"

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.

Code:

#!/usr/bin/python

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...

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}".

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.)

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.

Code:

#!/usr/bin/python

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}".

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.)

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.

-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/

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.

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.

-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"

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.

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"

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 999 National Security Letters.