[bug#53878] [PATCH v5 03/22] gnu: racket: Use Git origins for Racket packages.

[Liliana Marie Prikler]

Hi,

Am Samstag, dem 26.02.2022 um 12:23 -0500 schrieb Philip McGrath:
> Hi,
> 
> On Saturday, February 26, 2022 9:26:29 AM EST Liliana Marie Prikler
> wrote:
> > I'm not talking about the generated file, but the procedure that
> > generates it.  Special characters like dots and slash are still
> > pretty special in Scheme, even if they're allowed as identifiers.
> 
> Do you mean A, B, C, and/or D of these:
> a
> >           (replace 'configure
> >             (lambda* (#:key inputs configure-flags #:allow-other-
> > keys)
> >               (let* ((racket (search-input-file inputs
> > "bin/racket")))
> >                 (apply invoke
> >                        racket
> >                        #$make-installation-layer.rkt ;; <--- A
> >                        `(,@configure-flags
> >                          ,(dirname (dirname racket))
> >                          ,#$output))
> >                 (invoke racket
> >                         "--config" (string-append #$output
> > "/etc/racket")
> >                         "-l" "raco" "setup"
> >                         "--no-user"))))
> > 
> > [...]
> > 
> > (define make-installation-layer.rkt ;; <--- B
> >   (scheme-file
> >    "make-installation-layer.rkt" ;; <--- C
> >    `(module
> >      make-installation-layer racket/base ;; <--- D
> >      (require racket/cmdline
> 
> or something else?
The name in B is confusing imo, because it implies a procedure when it
is in fact a file.  Of course, that'd affect A as well.  The file name
in C does not matter too much, but should probably be matched with B to
some extent (but importantly keeping the .rkt extension even if we drop
it from B).  I'm not sure if D is a function call or not, but if it's a
module name perhaps we should make it match B.

Given that we pass configure-flags to this thing, how about "configure-
racket-layer", "configure-layer.rkt" or something along those lines?

> In particular, not to be dense, but B is not a function. 
Yeah, that's my bad for only skimming these and not paying too close
attention to brackets.  The name itself implies a function, hence my
confusion.  Looking at things closely would make me see more typos like
";; workaroung Guile reader/printer:"
workaround.

> One of the reasons I particularly like being able to use `.` in
> identifiers is to refer to files by name, without some ad-hoc
> encoding.
I do admit that is helpful, but in this context, metaphors were mixed
in a way that conveyed the wrong message.

> > > > I think some way to shorten those origins would do wonders in
> > > > terms
> > > > of the number of lines this patch adds.
> > > 
> > > At one point, I had abbreviated the origins as something like:
> > > 
> > >   (REPO-SPEC SHA256 [COMMIT])
> > > 
> > > [...]
> > 
> > I wouldn't do this inside of extract-package-source, but define a
> > one or two liner for adding specifically packages hosted on
> > racket's github.  WDYT?
> > 
> > > I wasn't sure about the trade-off between being slightly more
> > > cryptic
> > > than explicit origins, but a lot shorter.
> > 
> > Note that the goal is not to code golf, but to be understandable. 
> > When adding a bunch of origins as inputs, understandability suffers
> > by induced scrolling.
> > 
> 
> I've done this, and it was a very good idea. In particular, it helped
> find some places where I hadn't switched to using `%racket-commit`
> once the release was tagged.
That is better, but can still be improved.  As hinted at in my comment
to your tree, origin snippets might be a better solution here unless
I'm missing something in the heat of the moment.

> Other than potentially doing something about "make-installation-
> layer.rkt", 
> I'd plan to squash the WIP/FIXUP commits in
> <
> https://gitlab.com/philip1/guix-patches/-/tree/racket-chez-refactor-hist-28
> >
> and send it as v6.
I did have a short look at it and apart from the above comment it
appears fine, but I'll have to reevaluate it when it's a full v6
anyway.

Cheers