[Mageia-dev] get-skype package for submission

[Anssi Hannula]

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