Package: guix-patches;
Reported by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Date: Thu, 27 Oct 2022 03:43:01 UTC
Severity: normal
Tags: patch
Merged with 59161, 59162, 59163, 59164
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Ludovic Courtès <ludo <at> gnu.org> Cc: 58812 <at> debbugs.gnu.org Subject: [bug#58812] [PATCH 0/5] Add --symlink option to 'guix shell'. Date: Thu, 10 Nov 2022 09:49:34 -0500
Hi Ludo! Ludovic Courtès <ludo <at> gnu.org> writes: > Hi Maxim! > > Maxim Cournoyer <maxim.cournoyer <at> 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.