Re: 07/09: gnu: kinit: Use LIBRARY_PATH to search for dynamically loaded libs.

Re: 07/09: gnu: kinit: Use LIBRARY_PATH to search for dynamically loaded libs.

[Ludovic Courtès]

Hi Hartmut,

guix-commits@gnu.org skribis:

> commit 16b8aff85bcdb9799496c4a27257210cd45158e5
> Author: Hartmut Goebel <h.goebel@crazy-compilers.com>
> Date:   Thu Mar 1 23:08:44 2018 +0100
>
>     gnu: kinit: Use LIBRARY_PATH to search for dynamically loaded libs.
>     
>     Transfer the NixOS "kdeinit-libpath" patch for kinit as of
>     2018-02-17.
>     
>     * gnu/packages/patches/kinit-kdeinit-libpath.patch: New file.
>     * gnu/local.mk (dist_patch_DATA): Add it.
>     * gnu/packages/kde-frameworks.scm (kinit)[source]: Use it.

[...]

> ++                // Try to load the library relative to the active profiles.
> ++                QByteArrayList profiles = qgetenv("LIBRARY_PATH").split(':');
> ++                for (const QByteArray &profile: profiles) {
> ++                    if (!profile.isEmpty()) {
> ++                        l.setFileName(QFile::decodeName(profile) + QStringLiteral("/") + libpath);
> ++                        if (l.load()) break;
> ++                    }

I find this use of LIBRARY_PATH questionable: it’s the variable normally
used by the compiler driver (gcc) at link time, not by the loader
(ld.so).  IOW, it’s really a compile-time variable, and one that has
nothing to do with Qt or KDE.

What about using a different variable for this purpose?  It could be
KDE_PLUGIN_PATH or something like this, and we could add it to
‘native-search-paths’ of kinit.  We could also avoid the “+ libpath” bit
I suppose.

Thoughts?

Apologies if this has been discussed elsewhere…

Thank you,
Ludo’.

Re: 07/09: gnu: kinit: Use LIBRARY_PATH to search for dynamically loaded libs.

[Hartmut Goebel]

Hallo Ludo,
Am 05.01.19 um 15:24 schrieb Ludovic Courtès:

>> ++                // Try to load the library relative to the active profiles.
>> ++                QByteArrayList profiles = qgetenv("LIBRARY_PATH").split(':');
>> ++                for (const QByteArray &profile: profiles) {
>> ++                    if (!profile.isEmpty()) {
>> ++                        l.setFileName(QFile::decodeName(profile) + QStringLiteral("/") + libpath);
>> ++                        if (l.load()) break;
>> ++                    }
> I find this use of LIBRARY_PATH questionable: it’s the variable normally
> used by the compiler driver (gcc) at link time, not by the loader
> (ld.so).  IOW, it’s really a compile-time variable, and one that has
> nothing to do with Qt or KDE.
>
> What about using a different variable for this purpose?  It could be
> KDE_PLUGIN_PATH or something like this, and we could add it to
> ‘native-search-paths’ of kinit. 

Thanks for discovering and pointing this out. Looks like I did a mistake
here. Oh, and when rethinking, I made even more mistakes here. Indeed I
was thinking about a native-search-path, too, but abandoned this since I
thought, LIBRARY_PATH would already be there.

The original KDE code searches for the shared lib in
CMAKE_INSTALL_PREFIX/LIB_INSTALL_DIR/ only.

Nixos loops over NIX_PROFILES, looking into $profile/lib/, which would match

             (search-path-specification
              (variable "KDEINIT5_LIBRARY_PATH")
              (files '("lib/")))

which would make all packages including a shared lib to be listed in the
environment variable. (This is why I came to the wrong idea of using the
already existing LIBRARY_PATH.) I'm not confident this is a good idea.

One possible solution would be: Since the name to be loaded will be
prefixed by "/libkdeinit5_" (see some lines above [1]), we could change
this into "/libkdeinit5/" which then could become KDEINIT5_LIBRARY_PATH.
This of course has the draw back of a larger patch and that *all*
packages providing plugins for kinit need to be patched, too.

WDYT? Any other  idea?


> We could also avoid the “+ libpath” bit I suppose.

The “+ libpath” bit can not be avoided, since this is the actual name to
be loaded.


[1] https://cgit.kde.org/kinit.git/tree/src/kdeinit/kinit.cpp?h=v5.53.0#n625
[2]
https://github.com/NixOS/nixpkgs/blob/18.09/pkgs/development/libraries/kde-frameworks/kinit/kinit-libpath.patch

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

Re: 07/09: gnu: kinit: Use LIBRARY_PATH to search for dynamically loaded libs.

[Ludovic Courtès]

Hello,

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

> Am 05.01.19 um 15:24 schrieb Ludovic Courtès:
>
>>> ++                // Try to load the library relative to the active profiles.
>>> ++                QByteArrayList profiles = qgetenv("LIBRARY_PATH").split(':');
>>> ++                for (const QByteArray &profile: profiles) {
>>> ++                    if (!profile.isEmpty()) {
>>> ++                        l.setFileName(QFile::decodeName(profile) + QStringLiteral("/") + libpath);
>>> ++                        if (l.load()) break;
>>> ++                    }
>> I find this use of LIBRARY_PATH questionable: it’s the variable normally
>> used by the compiler driver (gcc) at link time, not by the loader
>> (ld.so).  IOW, it’s really a compile-time variable, and one that has
>> nothing to do with Qt or KDE.
>>
>> What about using a different variable for this purpose?  It could be
>> KDE_PLUGIN_PATH or something like this, and we could add it to
>> ‘native-search-paths’ of kinit. 
>
> Thanks for discovering and pointing this out. Looks like I did a mistake
> here. Oh, and when rethinking, I made even more mistakes here. Indeed I
> was thinking about a native-search-path, too, but abandoned this since I
> thought, LIBRARY_PATH would already be there.
>
> The original KDE code searches for the shared lib in
> CMAKE_INSTALL_PREFIX/LIB_INSTALL_DIR/ only.
>
> Nixos loops over NIX_PROFILES, looking into $profile/lib/, which would match
>
>              (search-path-specification
>               (variable "KDEINIT5_LIBRARY_PATH")
>               (files '("lib/")))
>
> which would make all packages including a shared lib to be listed in the
> environment variable.

Well, only the lib/ directories themselves would be listed, which is
probably fine.

(It would of course be nicer if KDE would instead store these plugins in
lib/kdeinit or something.)

> One possible solution would be: Since the name to be loaded will be
> prefixed by "/libkdeinit5_" (see some lines above [1]), we could change
> this into "/libkdeinit5/" which then could become KDEINIT5_LIBRARY_PATH.
> This of course has the draw back of a larger patch and that *all*
> packages providing plugins for kinit need to be patched, too.

I think it’s a good idea, but this is something that upstream should
decide.

As downstreams, the most reasonable option is to honor the
KDEINIT5_LIBRARY_PATH as you defined it above IMO.

Thanks,
Ludo’.

Re: 07/09: gnu: kinit: Use LIBRARY_PATH to search for dynamically loaded libs.

[Hartmut Goebel]

Am 08.01.19 um 18:16 schrieb Ludovic Courtès:
> As downstreams, the most reasonable option is to honor the
> KDEINIT5_LIBRARY_PATH as you defined it above IMO.

I posted a respective patch:
<http://debbugs.gnu.org/cgi/bugreport.cgi?bug=34021>

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |