[bug#30340] [PATCH 1/6] gnu: qtbase: Use the store paths for other packages and dynamically loaded libs.

[Marius Bakke <mbakke@fastmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3060 bytes --]

Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Am 07.02.2018 um 17:16 schrieb Marius Bakke:
>> Hartmut Goebel <h.goebel@crazy-compilers.com> writes:
>>
>>> Adobt the NixOS patches as of 2018-01-19:
>> I don't see any patches in this series. 
>
> I only *adopted* what NixOs does with patches, not the patches itself. I
> will rework this.
>
>>  FWIW I think we deviate enough
>> from NixOS at this point that the comments are unnecessary.
>
> Are you referring to patch (1/6)? Or do you mean patches 2, 3, 5 and 6
> are unnecessary?
>
> I do not care about the comments, but FMPOV it is important to document
> somehow in the code or in the commits that all patches as of 2018-01-19
> have been considered. an alternative would be to group these few commits
> into a (very short) branch and documenting the fact in the merge-commit.
>
> WDYT?

I don't think the comments are all that useful.  They only add noise in
the commit log and the code -- since we have a working and up-to-date
Qt, it is implied that we don't need anything more from NixOS or
elsewhere.

>>> - src/corelib/tools/qtimezoneprivate_tz.cpp: NixOS uses $TZDIR, we use
>>>   hardcoded path to tzdata.
>> Why hardcode the path?  We set TZDIR as well in (gnu system).
>
> The upstream code (qt.com) uses hard-coded path (/usr/share/zoneinfo/),
> so for me it seems to be much more natural to simply change this - and
> stay closer to upstream. NixOS seems to require TZDATA since some things
> work differently compared to guix. E.g. nixos is deriving library search
> paths from $PATH in some other patch. This is something guix does not need.

For tzdata in particular, we are trying to reduce the number of
dependent packages so that we can update it directly on 'master'.  Often
a new tzdata release introduces changes with near-immediate effects, so
it's important to be able to update it fast.

Adding it to qtbase would add 282 new dependent packages, which is
unfortunate.  So I much prefer using TZDIR, even though it would be
technically better to reference it directly.

>>> +         (add-after 'unpack 'patch-paths
>>> +           ;; Use the absolute paths for dynamically loaded libs, otherwise
>>> +           ;; the lib will be searched in the actual executable's RUNPATH,
>>> +           ;; which may not include the requested lib.
>> Is there any reason we cannot add these libraries to RUNPATH instead?
>> The below approach seems somewhat fragile to me.
>
> Rethinking this, this comment is wrong and I'll correct it. QLibrary
> (which is used an all these cases) is documented with:
>
>     When loading the library, QLibrary
>     <http://doc.qt.io/qt-5/qlibrary.html> searches in all the
>     system-specific library locations (e.g. |LD_LIBRARY_PATH| on Unix),
>     unless the file name has an absolute path.
>
> But guix does not set LD_LIBRARYPATH (e.g. in "guix environment"), thus
> we need to have absolute paths for the libraries.
>
> Does this make sense?

Yes, that makes sense :)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]