diff mbox

[4/10] argon2: Install pkg-config file.

Message ID 8760pjr43x.fsf@openmailbox.org
State New
Headers show

Commit Message

Lukas Gradl Sept. 25, 2016, 10:45 p.m. UTC
Hi Danny,

Sorry for the delay!

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Lukas,
>
> I thought about it some more.
>
> On Sun, 18 Sep 2016 16:43:03 -0500
> Lukas Gradl <lgradl@openmailbox.org> wrote:
>> +               (and
>> +                (install-file "argon2" (string-append out "/bin"))
>> +                (install-file "libargon2.a" (string-append out "/lib"))
>> +                (install-file "libargon2.so" (string-append out "/lib"))
>> +                (install-file "argon2.pc"
>> +                              (string-append out "/lib/pkgconfig"))
>
> Hmm. I looked at the implementation of "install-file" and its return
> value seems to be the one of "copy-file". And the Guile manual
> specifies that the return value of "copy-file" is
> unspecified. (Instead it does dynamic unwinding on error)
>
> So I'd do the "install-file" and "copy-recursively" calls outside the "(and ...)".

OK.

>
>> +                (copy-recursively "include"
>> +                                  (string-append out "/include"))
>> +                (zero? (system* "ln" "-s"
>> +                                (string-append out "/lib/libargon2.so")
>> +                                (string-append out "/lib/libargon2.so.0")))
>
> I think that there's a "symlink" function in Guile for that (which has unspecified return value). Would probably be marginally faster, too.

>> +                (zero? (system* ; Fix compatability for libtool based builds.
>> +                        "ln" "-s"
>> +                        (string-append out "/lib/libargon2.so")
>> +                        (string-append out "/lib/libargon2.so.0.0.0"))))))))))
>
> Likewise.

I changed the first of these two to use Guile's symlink function and
removed the second one ('libargon2.so.0.0.0) because I think it is not
needed after all.  Since the return value of "(symlink ...)" is
unspecified, then we don't need the "(and ...)" at all, right?  I
dropped it for now.

> About the part of the toolchain that sets the soname, either libtool mode=link does it ("-version-info") or it's an option "-soname" to ld (or gcc with "-Wl," prefix). 
>
> The official soname used is set in argon2 Makefile:
>
>         SO_LDFLAGS := -Wl,-soname,libargon2.so.0
>                                   ^^^^^^^^^
>
> But I'd say it's fine to manually symlink - since it works. Please still check how the client of argon2 in this case (libring ?) loads the shared object.

Thank you for this explanation!

I looked into this some more.  The error that I saw earlier that
prompted me to symlink actually occurred during the phase
'validate-runpath' of the build of libring.  The build itself finished
without any problems, however libargon2.so.0 is not found in the
runpath:

---8<--- cut here -------------------- start --->8---
starting phase `validate-runpath'
validating RUNPATH of 0 binaries in "/gnu/store/mfanlirixm9468z02vf4c8dqvjl5gm01-libring-2.2.0-1.41e032c/lib"...
validating RUNPATH of 1 binaries in "/gnu/store/mfanlirixm9468z02vf4c8dqvjl5gm01-libring-2.2.0-1.41e032c/sbin"...
/gnu/store/mfanlirixm9468z02vf4c8dqvjl5gm01-libring-2.2.0-1.41e032c/sbin/dring: error: depends on 'libargon2.so.0', which cannot be found in RUNPATH ("/gnu/store/b6d8j2w72fi9m67llz0mwgxjffw6ykms-dbus-c++-0.9.0/lib" "/gnu/store/311lp7c9w9slsb408dskj7cy5caz894q-speex-1.2rc1/lib" "/gnu/store/571c58j8f06x8svykg4n5s0ip36kna5c-util-linux-2.27/lib" "/gnu/store/1i3xmm18dw9kq6wi46f6sj9nxy9pckjl-alsa-lib-1.0.27.1/lib" "/gnu/store/2yx0rhssryy4n1wxvzi1rg03ipv4abid-pulseaudio-9.0/lib" "/gnu/store/2yx0rhssryy4n1wxvzi1rg03ipv4abid-pulseaudio-9.0/lib/pulseaudio" "/gnu/store/r8cavha98fk7x13n43h65m2x448bhyfc-json-c-0.12/lib" "/gnu/store/bmlkg9mbqj1k0y7kdq2rdll42aicglyk-dbus-1.10.8/lib" "/gnu/store/8pskv4xpdjg1grx3c2an3g5fqj8pcrgh-gdbm-1.12/lib" "/gnu/store/jy0644xc70hkvrv7gn6l5hys51dv2r38-libsamplerate-0.1.8/lib" "/gnu/store/ijyypnxnrv4wq9gg2jy2dp4wf129da5v-libsndfile-1.0.26/lib" "/gnu/store/ah4jgqr54yzn5wm8i81mf33b8ph9sh53-flac-1.3.1/lib" "/gnu/store/jdf37llsrlldwm6kq29hya5j4cfr8qhv-libvorbis-1.3.5/lib" "/gnu/store/qs7cl4890ja9s2p4k8l0nzw2ak7dg22r-libogg-1.3.2/lib" "/gnu/store/8fi2fsdwjmz8sar3l4dyya4bzal0n9x9-libupnp-1.6.20/lib" "/gnu/store/zwc6ck9j0wv80kz5snw5acwb39ws88m1-pcre-8.38/lib" "/gnu/store/xbcwlpy7szxgr4pccbn1gkrpqa8j7npm-opendht-0.6.1/lib" "/gnu/store/1qv5i6rfxjc4d0rg7z6r9dapmf85kzmy-gnutls-3.5.2/lib" "/gnu/store/2z0lv1px7izrmzxsqi3555yavqa56mkq-libidn-1.32/lib" "/gnu/store/3fhnmvbdxlsh64pr3zg4y74x3hlx33qw-libtasn1-4.8/lib" "/gnu/store/cdkrfbl10kbyyjjw3yfk9hckfw8n1b7g-gmp-6.1.0/lib" "/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib" "/gnu/store/q6sc9lws85pk034dny8ikl9z33n94zjm-jack-0.124.1/lib64" "/gnu/store/zvg8qyjsz20vjd170h4ilk7pnv7bqmm6-eudev-3.1.5/lib" "/gnu/store/m9vxvhdj691bq1f85lpflvnhcvrdilih-glibc-2.23/lib" "/gnu/store/y51frvdprzpq3skac6w392px91vyp4rq-libcap-2.24/lib" "/gnu/store/5992iq1v7arqa14ym3di58n4la0893nv-zlib-1.2.8/lib" "/gnu/store/7zhj5adf7kq7pr6j46wj1gjdc8vi0mwk-nettle-3.2/lib" "/gnu/store/8z4lfj32b7q308xigfj8w1nmgbgyvd6g-argon2-20160406/lib" "/gnu/store/00qwdmjrzflh0q4g9n18wcx8nvhhxcj1-bdb-6.2.23/lib" "/gnu/store/62px2z0gh6ygvi4pjdb1yqwqr8243vbp-ffmpeg-3.1.3/lib" "/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.3/../../..")
phase `validate-runpath' failed after 0.0 seconds
builder for `/gnu/store/7hy39m8nlvy2rjd6iljvgrbpqbwq1grz-libring-2.2.0-1.41e032c.drv' failed with exit code 1
@ build-failed /gnu/store/7hy39m8nlvy2rjd6iljvgrbpqbwq1grz-libring-2.2.0-1.41e032c.drv - 1 builder for `/gnu/store/7hy39m8nlvy2rjd6iljvgrbpqbwq1grz-libring-2.2.0-1.41e032c.drv' failed with exit code 1
guix build: error: build failed: build of `/gnu/store/7hy39m8nlvy2rjd6iljvgrbpqbwq1grz-libring-2.2.0-1.41e032c.drv' failed
---8<--- cut here -------------------- end ----->8---

I produced this after commenting all the symlink calls.  I think this
error points out that the following can be found in the offending binary
'dring':

---8<--- cut here -------------------- start --->8---
lukas@serenity [env]$ objdump -p /gnu/store/mfanlirixm9468z02vf4c8dqvjl5gm01-libring-2.2.0-1.41e032c/sbin/dring |grep argon2
  NEEDED               libargon2.so.0
  RUNPATH              /gnu/store/b6d8j2w72fi9m67llz0mwgxjffw6ykms-dbus-c++-0.9.0/lib:/gnu/store/311lp7c9w9slsb408dskj7cy5caz894q-speex-1.2rc1/lib:/gnu/store/571c58j8f06x8svykg4n5s0ip36kna5c-util-linux-2.27/lib:/gnu/store/1i3xmm18dw9kq6wi46f6sj9nxy9pckjl-alsa-lib-1.0.27.1/lib:/gnu/store/2yx0rhssryy4n1wxvzi1rg03ipv4abid-pulseaudio-9.0/lib:/gnu/store/2yx0rhssryy4n1wxvzi1rg03ipv4abid-pulseaudio-9.0/lib/pulseaudio:/gnu/store/r8cavha98fk7x13n43h65m2x448bhyfc-json-c-0.12/lib:/gnu/store/bmlkg9mbqj1k0y7kdq2rdll42aicglyk-dbus-1.10.8/lib:/gnu/store/8pskv4xpdjg1grx3c2an3g5fqj8pcrgh-gdbm-1.12/lib:/gnu/store/jy0644xc70hkvrv7gn6l5hys51dv2r38-libsamplerate-0.1.8/lib:/gnu/store/ijyypnxnrv4wq9gg2jy2dp4wf129da5v-libsndfile-1.0.26/lib:/gnu/store/ah4jgqr54yzn5wm8i81mf33b8ph9sh53-flac-1.3.1/lib:/gnu/store/jdf37llsrlldwm6kq29hya5j4cfr8qhv-libvorbis-1.3.5/lib:/gnu/store/qs7cl4890ja9s2p4k8l0nzw2ak7dg22r-libogg-1.3.2/lib:/gnu/store/8fi2fsdwjmz8sar3l4dyya4bzal0n9x9-libupnp-1.6.20/lib:/gnu/store/zwc6ck9j0wv80kz5snw5acwb39ws88m1-pcre-8.38/lib:/gnu/store/xbcwlpy7szxgr4pccbn1gkrpqa8j7npm-opendht-0.6.1/lib:/gnu/store/1qv5i6rfxjc4d0rg7z6r9dapmf85kzmy-gnutls-3.5.2/lib:/gnu/store/2z0lv1px7izrmzxsqi3555yavqa56mkq-libidn-1.32/lib:/gnu/store/3fhnmvbdxlsh64pr3zg4y74x3hlx33qw-libtasn1-4.8/lib:/gnu/store/cdkrfbl10kbyyjjw3yfk9hckfw8n1b7g-gmp-6.1.0/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib:/gnu/store/q6sc9lws85pk034dny8ikl9z33n94zjm-jack-0.124.1/lib64:/gnu/store/zvg8qyjsz20vjd170h4ilk7pnv7bqmm6-eudev-3.1.5/lib:/gnu/store/m9vxvhdj691bq1f85lpflvnhcvrdilih-glibc-2.23/lib:/gnu/store/y51frvdprzpq3skac6w392px91vyp4rq-libcap-2.24/lib:/gnu/store/5992iq1v7arqa14ym3di58n4la0893nv-zlib-1.2.8/lib:/gnu/store/7zhj5adf7kq7pr6j46wj1gjdc8vi0mwk-nettle-3.2/lib:/gnu/store/8z4lfj32b7q308xigfj8w1nmgbgyvd6g-argon2-20160406/lib:/gnu/store/00qwdmjrzflh0q4g9n18wcx8nvhhxcj1-bdb-6.2.23/lib:/gnu/store/62px2z0gh6ygvi4pjdb1yqwqr8243vbp-ffmpeg-3.1.3/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.3/../../..
---8<--- cut here -------------------- end ----->8---

My understanding is now that if this 'dring' binary were to get
executed, it would look for 'libargon2.so.0' in the Runpath.  However,
there is only 'libargon2.so' in
'/gnu/store/8z4lfj32b7q308xigfj8w1nmgbgyvd6g-argon2-20160406/lib', which
it ignores.


My next question was why it was looking for 'libargon2.so.0' at all if
it does not exist.

Actually, 'libargon2.so' calls itself 'libargon2.so.0':

---8<--- cut here -------------------- start --->8---
lukas@serenity [env]$ objdump -p /gnu/store/5fh9w9x3ihvy9n7pw0q58xafj6x21w99-argon2-20160406/lib/libargon2.so 
[...]
Dynamic Section:
  NEEDED               libgcc_s.so.1
  NEEDED               libpthread.so.0
  NEEDED               libc.so.6
  SONAME               libargon2.so.0
  RUNPATH              /gnu/store/m9vxvhdj691bq1f85lpflvnhcvrdilih-glibc-2.23/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib:/gnu/store/9nifwk709wajpyfwa0jzaa3p6mf10vxs-gcc-4.9.3-lib/lib/gcc/x86_64-unknown-linux-gnu/4.9.3/../../..
[...]
---8<--- cut here -------------------- end ----->8---

Libring requires Opendht via pkg-config.  The pkg-config file of Opendht
then tells the libring build process that 'argon2' is required.  Then
'libargon2.so' is found and maybe used during the build of libring, but
somehow (I am not sure how) the soname of 'libargon2.so' which does not
match the filename ends up in the 'NEEDED' section of 'dring' as seen
above.


To fix this situation it would probably suffice to make sure that the
soname and the filename of the Argon2 library match.  To that end, one
could either change the soname or the filename to match the other one.
I think that changing either of these is risky because other dependent
libraries might depend on one of those.  So I would keep the symlink
'libargon2.so.0' -> 'libargon2.so' but remove the 'libargon2.so.0.0.0'
since it appears unneeded.  Building libring works with these changes.

I hope this makes sense.  My understanding of libtool is still very
superficial.

Thank you!

Best,
Lukas

Comments

Danny Milosavljevic Sept. 25, 2016, 11:13 p.m. UTC | #1
Hi Lukas,

On Sun, 25 Sep 2016 17:45:06 -0500
Lukas Gradl <lgradl@openmailbox.org> wrote:

> Since the return value of "(symlink ...)" is
> unspecified, then we don't need the "(and ...)" at all, right?  

Right.

> My understanding is now that if this 'dring' binary were to get
> executed, it would look for 'libargon2.so.0' in the Runpath.  However,
> there is only 'libargon2.so' in
> '/gnu/store/8z4lfj32b7q308xigfj8w1nmgbgyvd6g-argon2-20160406/lib', which
> it ignores.

Yeah, .so files without version number in the name (xxx.so) are only used by gcc when linking. The actual running program doesn't do anything with them.

I think ldconfig updates a database from soname to actual file name. I'm not sure what the rules for the symlinks in ldconfig are.

> My next question was why it was looking for 'libargon2.so.0' at all if
> it does not exist.

I think it's because of the soname. The soname is the main ABI in this case, the files are just an implementation detail. The actual matching should be done via comparing sonames. It's just that it doesn't scan all files on the system in order to find it. 

>$ objdump -p /gnu/store/mfanlirixm9468z02vf4c8dqvjl5gm01-libring-2.2.0-1.41e032c/sbin/dring |grep argon2
>  NEEDED               libargon2.so.0
>$ objdump -p /gnu/store/5fh9w9x3ihvy9n7pw0q58xafj6x21w99-argon2-20160406/lib/libargon2.so 
>Dynamic Section:
>  SONAME               libargon2.so.0

So far so good.

It should just be in file libargon2.so.0 in order to be used. So I think the symlink that you are doing from the existing file libargo2.so to the required file libargon2.so.0 is fine.

> To fix this situation it would probably suffice to make sure that the
> soname and the filename of the Argon2 library match.

I agree.

>  To that end, one
> could either change the soname or the filename to match the other one.
> I think that changing either of these is risky because other dependent
> libraries might depend on one of those.  

Especially changing the soname would be bad. It's part of the ABI. Don't touch it - especially when it actually lists a major API version - as it does.

What we could do is rename libargon2.so to libargo2.so.0. But then gcc won't find it - so I wouldn't recommend it.

>So I would keep the symlink
> 'libargon2.so.0' -> 'libargon2.so' but remove the 'libargon2.so.0.0.0'
> since it appears unneeded.  Building libring works with these changes.

Yes. I think that is a good approach.

Could you send an updated patchset?

Thanks!
diff mbox

Patch

From 5fb85fef7b1475baca6c29beb26799ca8f3d814a Mon Sep 17 00:00:00 2001
From: Lukas Gradl <lgradl@openmailbox.org>
Date: Tue, 9 Aug 2016 16:49:19 -0500
Subject: [PATCH 04/10] gnu: argon2: Install pkg-config file.

* gnu/packages/password-utils.scm (argon2)[source]: Create pkg-config file.
[arguments]: Install it.
---
 gnu/packages/password-utils.scm | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/password-utils.scm b/gnu/packages/password-utils.scm
index 7288da6..f62d041 100644
--- a/gnu/packages/password-utils.scm
+++ b/gnu/packages/password-utils.scm
@@ -311,6 +311,21 @@  through the pass command.")
          "https://codeload.github.com/P-H-C/phc-winner-"
          name "/tar.gz/" version))
        (file-name (string-append name "-" version ".tar.gz"))
+       (snippet
+        '(let ((p (open-file "argon2.pc" "w")))
+           (begin
+             (display
+              (string-append "prefix=/usr/local\n"
+                             "exec_prefix=${prefix}\n"
+                             "includedir=${prefix}/include\n"
+                             "libdir=${prefix}/lib\n\n"
+                             "Name: Argon2\n"
+                             "Description: "
+                             "The Argon2 password hashing algorithm\n"
+                             "Version: 1.0.0\n"
+                             "Cflags: -I${includedir}/\n"
+                             "Libs: -L${libdir} -largon2\n") p)
+             (close-output-port p))))
        (sha256
         (base32
          "0g6wa94sh639xl1qc8z21q43r1mp8y77r1zf8nwx5pfsxd8fmyzv"))))
@@ -321,14 +336,23 @@  through the pass command.")
        #:phases
        (modify-phases %standard-phases
          (delete 'configure)
+         (add-after 'unpack 'fix-pkg-config
+           (lambda _
+             (substitute* "argon2.pc"
+               (("/usr/local")
+                (assoc-ref %outputs "out")))))
          (replace 'install
            (lambda _
              (let ((out (assoc-ref %outputs "out")))
                (install-file "argon2" (string-append out "/bin"))
                (install-file "libargon2.a" (string-append out "/lib"))
                (install-file "libargon2.so" (string-append out "/lib"))
+               (install-file "argon2.pc"
+                             (string-append out "/lib/pkgconfig"))
                (copy-recursively "include"
-                                 (string-append out "/include"))))))))
+                                 (string-append out "/include"))
+               (symlink (string-append out "/lib/libargon2.so")
+                        (string-append out "/lib/libargon2.so.0"))))))))
     (home-page "https://www.argon2.com/")
     (synopsis "Password hashing library")
     (description "Argon2 provides a key derivation function that was declared
-- 
2.9.0