Re: [proaudio] [2787] media-video/screencast: Initial ebuild with patch for JACK support.

[Karl Lindén <spray.sucks@xxxxxxxxx>]

2013/8/11  <subversion@xxxxxxxxxxxxx>:
> Revision: 2787
> Author:   dominique
> Date:     2013-08-11 13:58:23 +0200 (Sun, 11 Aug 2013)
> Log Message:
> -----------
> media-video/screencast: Initial ebuild with patch for JACK support.
>
> Modified Paths:
> --------------
>     trunk/overlays/proaudio/00-DETAILED-PACKAGES-LIST
>     trunk/overlays/proaudio/00-PACKAGES-LIST
>
> Added Paths:
> -----------
>     trunk/overlays/proaudio/media-video/screencastor/
>     trunk/overlays/proaudio/media-video/screencastor/ChangeLog
>     trunk/overlays/proaudio/media-video/screencastor/Manifest
>     trunk/overlays/proaudio/media-video/screencastor/files/
>     trunk/overlays/proaudio/media-video/screencastor/files/screencastor
>     trunk/overlays/proaudio/media-video/screencastor/files/screencastor-1.3.0_jack.patch
>     trunk/overlays/proaudio/media-video/screencastor/metadata.xml
>     trunk/overlays/proaudio/media-video/screencastor/screencastor-1.3.0.ebuild
>

Hi Dominique,

Good work! I took at look at the ebuild and I, however, see some issues:

1. You should use python-single-r1 eclass in this case. "The
python-single-r1 eclass is an alternative to python-r1 for packages
which do not support being installed for multiple Python
implementations." [1] I think that holds true for this one and also
you would not risk the (highly hypothetical) case when
PYTHON_TARGETS="-python2_7".

2. You must have something like REQUIRED_USE="${PYTHON_REQUIRED_USE}"
somewhere in the ebuild. [2]

3. python-single-r1 exports PYTHON_USEDEP. You should use it to
specify the dependencies. (For example in dev-python/pygtksourceview
and dev-python/pygtk, which are used in the python script)

4. In python-single-r1 (python-utils-r1, in fact) you need to use
python_scriptinto together with python_doscript instead of
python_replicate_script. This way the shebang is automatically
converted and a wrapper script is installed. [3] You don't need to
manually patch the python script.

5. In screencastor.py: "from xml.dom.minidom import parse". Note that
the module is built into python and triggered with USE="xml", you thus
need to set PYTHON_REQ_USE="xml" before inherit to make sure the xml
modules are there. [2]

6. Why are you not using the prefixed path ${ED} instead of ${D}?

7. You can drop the helper script (${FILESDIR}/screencastor) if you do
something like this: "dosym /usr/share/${PN}/go_${PN}.sh
/usr/bin/${PN}". This is also safe because both paths will be prefixed
(the helper script does not respect prefix).

8. You are not allowed to refer to variables in the HOMEPAGE string.
[4] (I have violated this sometime before, I know. Sorry about that.)

9. I am not sure, but isn't installing locale files into
/usr/share/${PN} non-standard? The .mo file (which is the only one you
need, in fact; the .po file is currently installed too...) is already
named and should not collide if you install into the standard
/usr/share/locale. Just make sure the program knows that the
translation can be found at the standard location.

10. With 4 and 9 in mind the "cp -r * [...]" should probably be
replaced with stating which files you want explicitly (because you
need to install the .mo file and the script explicitly anyway, to
different locations).

11. [NITPICK] You could order the dependencies alphabetically.

Please take these points in consideration and commit the result. I
have not tested

[1] http://www.gentoo.org/proj/en/Python/python-r1/dev-guide.xml
[2] http://devmanual.gentoo.org/eclass-reference/python-single-r1.eclass/index.html
[3] http://devmanual.gentoo.org/eclass-reference/python-utils-r1.eclass/index.html
[4] http://devmanual.gentoo.org/ebuild-writing/variables/index.html

Thanks,
Karl