Re: [PATCH] gnu: Add nss.

[宋文武]

Ludovic Courtès <address@hidden> writes:

> 宋文武 <address@hidden> skribis:
>
>> Mark H Weaver <address@hidden> writes:
>>
>>> 宋文武 <address@hidden> writes:
>>>
>>>> * gnu/packages/patches/nss.patch: New file.
>>>> * gnu-system.scm (dist_patch_DATA): Add it.
>>>> * gnu/packages/polkit.scm (nss): New variable.
>>>
>>> polkit.scm seems like the wrong place for 'nss', although admittedly
>>> it's not obvious where it should go.
>> Maybe move it and nspr gnuzilla.scm? Since they are both from mozilla.
>
> Yes please, sounds better.
>
>> NSS contains many static libraries which I think will not mostly used
>> by other. How about split them to a 'static-lib' (or a better name).
>
> Or just build it with --disable-static until someone asks for them?
Um, I leave it untouched. (It add about 7MB to Qt5.)
>
>> From f56c7ef06f8eb697e9b0e0eac2e6c8117520d5d0 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?=E5=AE=8B=E6=96=87=E6=AD=A6?= <address@hidden>
>> Date: Tue, 6 Jan 2015 23:57:31 +0800
>> Subject: [PATCH] gnu: Add nss.
>>
>> * gnu/packages/patches/nss-pkgconfig.patch: New file.
>> * gnu-system.scm (dist_patch_DATA): Add it.
>> * gnu/packages/polkit.scm (nss): New variable.
>
> [...]
>
>> diff --git a/gnu/packages/patches/nss-pkgconfig.patch 
>> b/gnu/packages/patches/nss-pkgconfig.patch
>> new file mode 100644
>> index 0000000..3832dc3
>> --- /dev/null
>> +++ b/gnu/packages/patches/nss-pkgconfig.patch
>> @@ -0,0 +1,220 @@
>> +Install nss.pc and nss-config.
>
> This is too terse compared to the size of the patch.  Surely one should
> justify why it takes so many lines to install two files.  :-)
>
>> +Based on nss-3.17-gentoo-fixups.patch from nixpkgs.
>
> Please make it clear who wrote this patch, where you took it, whether it
> differs from the Gentoo patch and how.
>
>> +       (alist-replace
>> +        'configure
>> +        (lambda* (#:key system inputs #:allow-other-keys)
>> +          (when (string-prefix? "x86_64" system)
>> +            (setenv "USE_64" "1")))
>
> Please add a comment explaining why this is needed, and return #t.
>
>> +        (alist-replace
>> +         'check
>> +         (lambda _
>> +           (setenv "DOMSUF" "(none)")
>> +           (setenv "USE_IP" "TRUE")
>> +           (setenv "IP_ADDRESS" "127.0.0.1")
>> +           (zero? (system "./nss/tests/all.sh")))
>
> Likewise, a comment must explain this.  Please use ‘system*’.
OK.
>
>> +         (alist-replace
>> +          'install
>> +          (lambda* (#:key outputs #:allow-other-keys)
>> +            (let* ((out (assoc-ref outputs "out"))
>> +                   (bin (string-append (assoc-ref outputs "bin") "/bin"))
>> +                   (inc (string-append out "/include/nss"))
>> +                   (lib (string-append out "/lib/nss"))
>> +                   (obj (match (scandir "dist" (cut string-suffix? "OBJ" 
>> <>))
>> +                          ((obj) (string-append "dist/" obj)))))
>> +              ;; Install nss-config to $out/bin.
>> +              (mkdir-p (string-append out "/bin"))
>> +              (copy-file (string-append obj "/bin/nss-config")
>> +                         (string-append out "/bin/nss-config"))
>> +              (delete-file (string-append obj "/bin/nss-config"))
>> +              ;; Install nss.pc to $out/lib/pkgconfig.
>
> So does this do the same thing that the big patch is supposed to do?
> I’m confused.
Actually, the patch do 'create', here do 'install'.
>
> Thank you,
> Ludo’.
Updated patches sent separated.