Hi Tomas,

Tomas Volf <[email protected]> writes:

> Hello,
>
> Ludovic Courtès <[email protected]> writes:
>
>> How about this instead?
>>
>> diff --git a/guix/scripts/pack.scm b/guix/scripts/pack.scm
>> index 1536e913bf6..de0ec88c0f8 100644
>> --- a/guix/scripts/pack.scm
>> +++ b/guix/scripts/pack.scm
>> @@ -1348,6 +1348,13 @@ (define* (wrapped-package package
>>                               (elf-loader-compile-flags program)))
>>                (delete-file "run.c")))
>>  
>> +          (define (executable? file)
>> +            ;; Return #t if FILE is a regular file and is executable.
>> +            (let ((s (stat file #f)))
>> +              (and s
>> +                   (eq? 'regular (stat:type s))
>> +                   (not (zero? (logand (stat:mode s) #o100))))))
>> +
>>            (setvbuf (current-output-port) 'line)
>>  
>>            ;; Link the top-level files of PACKAGE so that search paths are
>> @@ -1360,7 +1367,7 @@ (define* (wrapped-package package
>>                      (scandir input))
>>  
>>            (receive (executables others)
>> -              (partition executable-file?
>> +              (partition executable?
>>                       ;; Note: Trailing slash in case these are symlinks.
>>                           (append (find-files (string-append input "/bin/"))
>>                                   (find-files (string-append input "/sbin/"))
>>
>>
>> That would avoid wrapping directories (!) and everything should work
>> much better afterwards.
>
> I have tested your patch and can confirm it works.  I still prefer my
> approach.  The resulting code is simpler, and since nothing in libexec
> should be executed directly anyway, wrapping it should not be needed in
> the first place (even your version just skips the wrapping in favor of
> making a symlink; "proper" way would be to replace the symlink with an
> actual directory with wrapped content).

I think it's safer to wrap binaries under libexec; these are sometimes
used in D-Bus services for example, and wrapping is useful on them.  So

It reminded me of some change I submitted to the Jami daemon in 2021 to
fix an issue where the 'jamid' (then dring) binary used by the D-Bus
services was not wrapped because it appeared under and this would cause
issues.  See commit 2afa99fd1 [0].

  Executables are typically installed under bin/, sbin/ or libexec,
  rather than lib/.  The problem was noticed when trying to use Jami
  with a Guix relocatable pack, which only wraps executables found under
  the above locations.  Since dring is a daemon and should not be run
  directly by users under normal circumstances, the convention is to use
  the libexec directory, according to the File Hierchy Standard 3.0.

[0]  
https://git.jami.net/savoirfairelinux/jami-daemon/-/commit/2afa99fd126979441e399cbc145e0f4d81e72c31

-- 
Thanks,
Maxim



Reply via email to