[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’.
- [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., Ludovic Courtès, 2022/11/09
- [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., Maxim Cournoyer, 2022/11/09
- [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'.,
Ludovic Courtès <=
- [bug#58812] [PATCH v3 1/4] Makefile.am: Sort EXTRA_DIST entries., Maxim Cournoyer, 2022/11/10
- [bug#58812] [PATCH v3 2/4] install: Validate symlink target in evaluate-populate-directive., Maxim Cournoyer, 2022/11/10
- [bug#58812] [PATCH v3 4/4] shell: Detect --symlink spec problems early., Maxim Cournoyer, 2022/11/10
- [bug#58812] [PATCH v3 3/4] guix: shell: Add '--symlink' option., Maxim Cournoyer, 2022/11/10
[bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'., zimoun, 2022/11/16