[Mageia-dev] get-skype package for submission

Mon Jun 13 14:24:53 CEST 2011

On 12.06.2011 02:37, Barry Jackson wrote:
+> On 11/06/11 18:03, Anssi Hannula wrote:
+>>
+>> - Best to add a comment in the %post script to remind that any new files
+>>    need to have a %ghost entry created.
+>>    (btw: an alternative idea is to create a post-script and the filelist
+>>     at the same time in a single for loop in %build/%install, so that
+>>     the lists can never get out of sync as they are created from a single
+>>     list; however, your current solution is adequate as well)
+> 
+> Note added - keep it simple.
+> 
+
+You added the note in the %files section, not in %post. The issue only
+happens if someone updates %post but not %files, so it doesn't make
+sense to put the reminder in %files :)
+
+>> - You don't check for failures. If the mktemp call fails, you extract
+>>    the tarball into /root etc.etc.. You need to check for failures on
+>>    the mktemp/mkdir/cd commands (mktemp failure can be detected by
+>>    checking if $newtmp string is empty), or alternatively run "set -e"
+>>    to make the shell exit on failures (this leaves the tmpdir polluted,
+>>    but it doesn't matter as much as it is an uncommon failure case and
+>>    /tmp is cleaned periodically anyway).
+> 
+> Added some checks - with clean-up of previously created dirs etc on fail.
+
+> mkdir %{mytmp}
+> [[ -d %{mytmp} ]] || exit 1
+> cd %{mytmp}
+
+cd is not guaranteed to work even if %mytmp exists. Add "|| exit 1".
+
+> cd ${tmpextdir}
+> tar jxf %{mytmp}/skype-%{version}.tar.bz2
+
+Same here...
+
+Well, actually these two aren't that serious, since they can't be
+exploited by non-root anyway. I'm somewhat ok with them, as long as your
+mentor is as well and no one else objects.
+
+Note that your "if ! [[ -d %{tmpdir} ]]; then" check is not 100%
+necessary, as %tmpdir not existing is unlikely and causes no side effects.
+
+Other nits:
+- Use better variable names- %mytmp, $tmpextdir, %tmpdir are confusing.
+- The description starts with an empty line. Remove it.
+
+-- 
+Anssi Hannula
+