guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'.


From: Ludovic Courtès
Subject: [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'.
Date: Thu, 10 Nov 2022 15:17:17 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)

Hi Maxim!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

>> We should refrain from using @ref in sentences (info "(texinfo) @ref").
>> Instead, I’d write:
>>
>>   documented for @command{guix pack} (@pxref{pack-symlink-option}).
>
> I've heard that from you before, but is there a reason against?  I like
> to know the rationale for doing things a certain way, lest I forget :-).
> From info '(texinfo) @ref':

It’s right below the bit you quoted:

     The '@ref' command can tempt writers to express themselves in a
  manner that is suitable for a printed manual but looks awkward in the
  Info format.  Bear in mind that your audience could be using both the
  printed and the Info format.  For example: […]

>>> +(define (make-symlink->directives directory)
>>> +  "Return a procedure that turn symlinks specs into directives that target
>>> +DIRECTORY."
>>> +  (match-lambda
>>> +    ((source '-> target)
>>> +     (let ((target (string-append directory "/" target))
>>> +           (parent (dirname source)))
>>> +       ;; Never add a 'directory' directive for "/" so as to preserve its
>>> +       ;; ownership and avoid adding the same entries multiple times.
>>> +       `(,@(if (string=? parent "/")
>>> +               '()
>>> +               `((directory ,parent)))
>>> +         ;; Note: a relative file name is used for compatibility with
>>> +         ;; relocatable packs.
>>> +         (,source -> ,(relative-file-name parent target)))))))
>>
>> I think it’s a case where I would refrain from factorizing because this
>> procedure, as shown by the comments and the use of ‘relative-file-name’,
>> is specifically tailored for the needs to ‘guix pack -f tarball’.
>>
>> I’d prefer to have a similar but independently maintained variant of
>> this procedure in (guix scripts environment) to avoid difficulties down
>> the road.
>
> I considered to duplicate it, but I opted to reuse it in the end because
> I care that the behavior is exactly the same between the two actions
> (guix shell --symlink vs guix pack --symlink).  If the way we handle
> this is to be changed in the future, I'd want both to be changed at
> once, so they remain consistent.  Does this make sense?

They don’t have to be consistent.  Use of ‘relative-file-name’ here for
example is dictated by the needs of relocatable packs.  It doesn’t have
to be this way here.

I think it’s best to keep separate copies here (they likely won’t be
exactly the same).

>>> +++ b/guix/scripts/environment.scm
>>> @@ -33,8 +33,10 @@ (define-module (guix scripts environment)
>>>    #:use-module ((guix gexp) #:select (lower-object))
>>>    #:use-module (guix scripts)
>>>    #:use-module (guix scripts build)
>>> +  #:use-module ((guix scripts pack) #:select (symlink-spec-option-parser))
>>
>> You can turn this into #:autoload so we don’t pay the price when not
>> using ‘--symlink’.
>
> Done!  Could Guile simply always use lazy loading (autoload by default)?

#:select could be synonymous with #:autoload, if that’s what you mean,
but in general Guile cannot know whether autoloading is semantically
equivalent to eagerly loading: there might be side-effects happening
when the top-level of the module runs.

> Otherwise, when is it OK to use autoload and when is it not?

#:autoload exists as a way to amortize initialization costs and make
sure only necessary functionality gets loaded, thereby reducing CPU and
memory usage.

Only the module user can tell whether #:autoload is appropriate.  In
general you’d use it for optional functionality that has a
non-negligible memory footprint or that would noticeably degrade startup
time.

Ludo’.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]