You are not logged in.

#1 2018-11-04 02:53:33

isch
Member
Registered: 2018-11-04
Posts: 6

PKGBUILD review request

Hi there,

First time maintainer, here. I tried my best to read as many forum threads, wiki pages, etc as possible when writing this PKGBUILD, but want some peer review from those more experienced than I before submission to AUR.

https://github.com/ischoonover/wickr-bin

There are some peculiarities to note. This proprietary software's download source is a zip of two deb files. One of them creates app-specific libraries and places them in /usr/local. I'm aware this is a no-no, but the app itself links these libraries, and there's no way I can change that. The app doesn't statically link version-specific libraries that are required. I found some in AUR already, and another, I had to submit myself. The app would not start when I tried symlinking the specific version of the so files to latest.

I've verified on a fresh install that the app does indeed start and function with this PKGBUILD. Hopefully with everyone's help I can get this submitted to AUR soon! It's nice to give back to the community smile

Last edited by isch (2018-11-04 03:12:03)

Offline

#2 2018-11-04 03:38:57

Trilby
Inspector Parrot
Registered: 2011-11-29
Posts: 29,442
Website

Re: PKGBUILD review request

This is particularly odd:

_real_package_filename=`ls -1 *.deb | grep WickrMe

First, there is no need for the -1 as ls output is by line when piped.  Second, why use ls for a generic glob and pass it through grep: first, just use `ls *WickrMe*.deb` then second realize that ls there is pointless:

_real_package_filename=*WickrMe*.deb

Of course I also wonder why you need globs at all - you are using a fixed source, so surely you could know the complete filename.

EDIT: also, the install file does not need to be listed as a source - and in fact in this case it isn't as you overwrite the source variable a couple lines later, so remove the first useless instance of the source variable setting.

EDIT 2: Get rid of the 'noextract' and the 'unzip' command in the prepare function.  Why prevent makepkg from doing this when you then do it yourself.  Just let makepkg do what it does.  And then get rid of the makedep of unzip as it is not needed.  You'll also need to install/move a license file.

Last edited by Trilby (2018-11-04 03:44:20)


"UNIX is simple and coherent..." - Dennis Ritchie, "GNU's Not UNIX" -  Richard Stallman

Online

#3 2018-11-04 03:48:34

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: PKGBUILD review request

You overwrite the source=() array by declaring it twice. Just get rid of the first time, as the .install file does not need to be listed as a source file. The install script is kind of pointless anyway as I imagine everyone knows by now what it's been renamed to -- but it's a GUI application typically started by a .desktop file, why do they care???

Don't forcibly set PKGEXT, this is supposed to be a user-configurable choice in makepkg.conf and if people are bothered by long compression times, they should consider setting this as a global option...

There's absolutely no need to override the DLAGENTS, especially by using -k/--insecure since the SSL certificate for s3.amazonaws.com is totally fine.

Why do you use noextract and then extract it yourself using unzip? makepkg already handles this just fine by itself, extracting the exact same files using bsdtar instead.

license=('Proprietary') but you don't actually install the license, you should add a source element containing their EULA or whatever -- and the current AUR package is not in compliance either.

...

The rpath for this binary is incredible... /usr/local/wickr/Qt-5.9:/usr/local/wickr/Qt-5.9/lib:/usr/lib/WickrMe/lib:/home/gitlab-runner/builds/d9cdf721/0/wickr/desktop/wickr-desktop/autobuild-messenger-release/clients/enterprise/../../libs/qzxing/src

It's a very badly behaved binary and should properly depend on only relocatable libraries relative to $ORIGIN/lib, and then install directly to /opt

Anyway, your dependencies make no sense as this uses its own qt5 installation but you're depending on the system copies as well. If the system version works, then don't try to install wickr-qt_5.9.4_amd64.deb as well. If not, don't depend on things you don't use.

Last edited by eschwartz (2018-11-04 03:52:10)


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#4 2018-11-04 15:17:00

isch
Member
Registered: 2018-11-04
Posts: 6

Re: PKGBUILD review request

Thanks to both for the knowledge and tips! I've learned much from your help, and feel good about submitting to AUR now. I better understand this process and the inner workings of the toolchains that power it, now.

Good eye, @eschartz finding the Qt5 issue. Sure enough, the binary wanted its own version. I had added all the dependencies that ldd suggested, which is what led to this situation. Some assumptions and vestiges led to some of the other issues. I appreciate the second set of eyes and the time you put into helping me correct course as quality is important in my work.

Offline

#5 2018-11-04 15:48:05

loqs
Member
Registered: 2014-03-06
Posts: 17,192

Re: PKGBUILD review request

You seem to have missed some of the issues pointed out to you:
license file is not included https://wickr.com/terms/
install file is listed in sources
PKGEXT is overridden
DLAGENTS is overriden
rpath is not truncated

Offline

#6 2018-11-04 15:51:22

isch
Member
Registered: 2018-11-04
Posts: 6

Re: PKGBUILD review request

loqs wrote:

You seem to have missed some of the issues pointed out to you:
license file is not included https://wickr.com/terms/
install file is listed in sources
PKGEXT is overridden
DLAGENTS is overriden
rpath is not truncated

I merged my changes to master before I wrote that last post, and see that they are indeed reflected. I think perhaps you may be looking at an old commit.

Offline

#7 2018-11-04 16:00:55

loqs
Member
Registered: 2014-03-06
Posts: 17,192

Re: PKGBUILD review request

https://aur.archlinux.org/cgit/aur.git/ … =wickr-bin Last Updated:     2016-03-10 17:33
I can not see any commits by you https://aur.archlinux.org/cgit/aur.git/log/?h=wickr-bin apologies what name should I look for in AUR?
Edit:
I misread you had updated the AUR PKGBUILD not were going to do so.  From the github PKGBUILD just the rpath is not truncated issue seems to still be present.
Do you think the license is compatible with those of the dependencies the PKGBUILD lists?

Last edited by loqs (2018-11-04 16:13:39)

Offline

#8 2018-11-04 16:26:59

isch
Member
Registered: 2018-11-04
Posts: 6

Re: PKGBUILD review request

loqs wrote:

https://aur.archlinux.org/cgit/aur.git/ … =wickr-bin Last Updated:     2016-03-10 17:33
I can not see any commits by you https://aur.archlinux.org/cgit/aur.git/log/?h=wickr-bin apologies what name should I look for in AUR?
Edit:
I misread you had updated the AUR PKGBUILD not were going to do so.  From the github PKGBUILD just the rpath is not truncated issue seems to still be present.
Do you think the license is compatible with those of the dependencies the PKGBUILD lists?

I'm trying to sort out the rpath now, as I was curious by your assertion that it could be done.

I changed it on my local install with patchelf --set-rpath /usr/lib/WickrMe/lib, as I wanted to test if the binary could use a newer version of the Qt5 packages. Now, it refuses to start as it can't seem to load a library which is not only in that directory, but also installed on the system. Using eu-readelf -d /usr/bin/WickrMe to check that my changes are reflected, they do seem to be proper.

Offline

#9 2018-11-04 16:45:34

loqs
Member
Registered: 2014-03-06
Posts: 17,192

Re: PKGBUILD review request

Does strace reveal if WickrME attempts to load the system version of the library or it just fails to find any version?

Offline

#10 2018-11-04 17:34:42

isch
Member
Registered: 2018-11-04
Posts: 6

Re: PKGBUILD review request

loqs wrote:

Does strace reveal if WickrME attempts to load the system version of the library or it just fails to find any version?

It does look on the system, so the package I have installed must no longer contain this library. It never tries searching its rpath.

[ian@antegros wickr-bin]$ strace WickrMe
openat(AT_FDCWD, "/usr/lib/tls/x86_64/x86_64/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib/tls/x86_64/x86_64", 0x7fffce170d50) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/tls/x86_64/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib/tls/x86_64", 0x7fffce170d50) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/tls/x86_64/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib/tls/x86_64", 0x7fffce170d50) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/tls/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib/tls", 0x7fffce170d50)    = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/x86_64/x86_64/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib/x86_64/x86_64", 0x7fffce170d50) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/x86_64/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib/x86_64", 0x7fffce170d50) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/x86_64/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib/x86_64", 0x7fffce170d50) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/libcjson.so.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
stat("/usr/lib", {st_mode=S_IFDIR|0755, st_size=135168, ...}) = 0
writev(2, [{iov_base="WickrMe", iov_len=7}, {iov_base=": ", iov_len=2}, {iov_base="error while loading shared libra"..., iov_len=36}, {iov_base=": ", iov_len=2}, {iov_base="libcjson.so.1", iov_len=13}, {iov_base=": ", iov_len=2}, {iov_base="cannot open shared object file", iov_len=30}, {iov_base=": ", iov_len=2}, {iov_base="No such file or directory", iov_len=25}, {iov_base="\n", iov_len=1}], 10WickrMe: error while loading shared libraries: libcjson.so.1: cannot open shared object file: No such file or directory
) = 120
exit_group(127)                         = ?
+++ exited with 127 +++

[ian@antegros wickr-bin]$ eu-readelf -d /usr/bin/WickrMe
...
RUNPATH    Library runpath: [/usr/lib/WickrMe/lib]
...
[ian@antegros wickr-bin]$ ls -la /usr/lib/WickrMe/lib/libcjson.so.1
lrwxrwxrwx 1 root root 17 Nov  4 06:57 /usr/lib/WickrMe/lib/libcjson.so.1 -> libcjson.so.1.5.7
[ian@antegros wickr-bin]$ ls -la /usr/lib/WickrMe/lib/libcjson.so.1.5.7
-rw-r--r-- 1 root root 30984 Nov  4 06:57 /usr/lib/WickrMe/lib/libcjson.so.1.5.7

EDIT:

patchelf was the problem. When using it to change back to the original rpath, the problem persisted. Replacing the original binary, and using chrpath got me my answer as to whether system qt5 packages would work.

Once everything was moved into /opt, strace reveals yet more searching for /usr/local locations of Qt-5.9:

                                                                                           
stat("/usr/local/wickr/Qt-5.9/plugins", 0x7fff2adbc580) = -1 ENOENT (No such file or directory)                                                     
openat(AT_FDCWD, "/usr/local/wickr/Qt-5.9/qtlogging.ini", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)                              
stat("/home/ian/.config/QtProject/qtlogging.ini", 0x7fff2adbc3f0) = -1 ENOENT (No such file or directory)                                          
stat("/etc/xdg/QtProject/qtlogging.ini", 0x7fff2adbc3f0) = -1 ENOENT (No such file or directory)                                                              
geteuid()                               = 1000                                  
getuid()                                = 1000                                  
openat(AT_FDCWD, "/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = 3      
fstat(3, {st_mode=S_IFREG|0644, st_size=3031664, ...}) = 0                      
mmap(NULL, 3031664, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f375db98000              
close(3)                                = 0                                     
getpid()                                = 6661                                  
stat("/proc/6661/exe", {st_mode=S_IFREG|0755, st_size=80207064, ...}) = 0       
lstat("/proc/6661/exe", {st_mode=S_IFLNK|0777, st_size=0, ...}) = 0              
lstat("/proc", {st_mode=S_IFDIR|0555, st_size=0, ...}) = 0                       
lstat("/proc/6661", {st_mode=S_IFDIR|0555, st_size=0, ...}) = 0                  
lstat("/proc/6661/exe", {st_mode=S_IFLNK|0777, st_size=0, ...}) = 0              
readlink("/proc/6661/exe", "/opt/wickr-bin/bin/WickrMe", 4095) = 26              
lstat("/opt", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0                     
lstat("/opt/wickr-bin", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0           
lstat("/opt/wickr-bin/bin", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0       
lstat("/opt/wickr-bin/bin/WickrMe", {st_mode=S_IFREG|0755, st_size=80207064, ...}) = 0
stat("/opt/wickr-bin/bin/qt.conf", 0x7fff2adbc130) = -1 ENOENT (No such file or directory)
stat("/opt/wickr-bin/bin/qt.conf", 0x7fff2adbbb80) = -1 ENOENT (No such file or directory)
stat("/usr/local/wickr/Qt-5.9/plugins", 0x7fff2adbbda0) = -1 ENOENT (No such file or directory)
lstat("/opt", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0                     
lstat("/opt/wickr-bin", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0           
lstat("/opt/wickr-bin/bin", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0       
stat("/opt/wickr-bin/bin", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0        
stat("/opt/wickr-bin/bin/platforms/.", 0x7fff2adbbdf0) = -1 ENOENT (No such file or directory)
rt_sigprocmask(SIG_UNBLOCK, [ABRT], NULL, 8) = 0                                 
rt_sigprocmask(SIG_BLOCK, ~[RTMIN RT_1], [], 8) = 0                              
getpid()                                = 6661                                   
gettid()                                = 6661                                   
tgkill(6661, 6661, SIGABRT)             = 0                                      
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0                                     
--- SIGABRT {si_signo=SIGABRT, si_code=SI_TKILL, si_pid=6661, si_uid=1000} ---             
+++ killed by SIGABRT (core dumped) +++                              
Aborted (core dumped)

PKGBUILD changes here: https://github.com/ischoonover/wickr-bi … s/PKGBUILD

Last edited by isch (2018-11-04 20:16:20)

Offline

#11 2018-11-04 20:38:17

eschwartz
Fellow
Registered: 2014-08-08
Posts: 4,097

Re: PKGBUILD review request

At a minimum you want to remove the rpath for /home/gitlab-runner


Managing AUR repos The Right Way -- aurpublish (now a standalone tool)

Offline

#12 2018-11-04 22:17:23

isch
Member
Registered: 2018-11-04
Posts: 6

Re: PKGBUILD review request

eschwartz wrote:

At a minimum you want to remove the rpath for /home/gitlab-runner

Alright. I've done this, as well as make a few changes to the app's .desktop file (like change the version and description).

I'll submit this to AUR unless I get an objection. Thanks again everyone for the help on my first PKGBUILD!

Offline

Board footer

Powered by FluxBB