On 01/01/2018 10:44 PM, goodman Leung via aur-general wrote:

hi team

this is my first start to q PKGBUILD,  i am not sure it is very correct , any one can help to check the script

You *must* quote "${srcdir}" and "${pkgdir}" as they can contain spaces.

You should remove the empty variables that you aren't using, as well as the commented-out first version of the source array.

Looks like http://yydl.duowan.com as well as http://www.yy.com/ offer https:// as well, might as well use it.

Why do you use arch=('i686' 'x86_64') when this only downloads a Windows executable and some shellscripts and runs it in wine? The source downloads don't appear to differentiate between architectures via source_i686=() and source_x86_64=() either. arch=('any') is designated for packages that, once built, can be run on either. For example, scripting languages and data files.

I don't really know where that run.sh file comes from or what it looks like, so I cannot comment on it.

Instead of using `cp ${srcdir}/{files.7z,run.sh,YYSetup-${YYver}-zh-CN.exe} -i opt/${pkgname}`

Use `install`, like this:

``` install -Dt opt/${pkgname} -m644 "${srcdir}"/{files.7z,YYSetup-${YYver}-zh-CN.exe} install -Dt opt/${pkgname} -m755 "${srcdir}"/run.sh ```

This also avoids the use of an interactive `cp -i`, since PKGBUILDs should *never* require user interaction to build.

I also would avoid the use of `msg` as that is not interesting information the user needs to be alerted to with colorful messages. Just let e.g. sed run on its own.

sed can change multiple things in one go, no need to call it multiple times, or change directories so much, when you can do:

``` sed -e "s/PKGNAME/${pkgname}/" \ -e "s/APPVERSION/${YYver}/" \ -i "${srcdir}"/run.sh sed -e "s/PKGNAME/${pkgname}/" \ -e "s/Name=/Name=1/" \ -i "${srcdir}"/usr/share/applications/YY_Voice.desktop

```

But I would ask instead, why is this necessary at all, and why doesn't upstream already set this properly?