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

[Maxim Cournoyer]

Hi Ludo!

Ludovic Courtès <ludo@gnu.org> writes:

> 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: […]

Yes, and I don't get it :-)

--8<---------------cut here---------------start------------->8---
   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:

     Sea surges are described in @ref{Hurricanes}.

looks ok in the printed output:

     Sea surges are described in Section 6.7 [Hurricanes], page 72.

but is awkward to read in Info, "note" being a verb:

     Sea surges are described in *note Hurricanes::.
--8<---------------cut here---------------end--------------->8---

I don't see a "note" in the final sentence that should make it awkward?
It's lacking a "see " prefix though, which could help to make things a
bit clearer, I guess.

It looks the same in info as in the pxref example given above:

--8<---------------cut here---------------start------------->8---
For example,

     For more information, @pxref{This}, and @ref{That}.

produces in Info:

     For more information, *note This::, and *note That::.
--8<---------------cut here---------------end--------------->8---

I'm also unsure where the "see" comes before That:: above.  Is it a
mistake in the manual?

>>>> +(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).

OK, I see you point about relative-file-name not being needed.  I'll make
the change.

>>>> +++ 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.

Perhaps there could be a strict execution mode where it is assumed that
side effects are not used when modules run?  That seems a seldom used
feature anyway, and could enable making lazy loading of modules the
default.

>> 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’.

Thank you for the explanations and review!  I'll send a v3 shortly.

--
Maxim