Re: RFC: new syntax for inline patches

[Jelle Licht <jlicht@fsfe.org>]

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> Hi Ricardo,
>
> Am Dienstag, dem 04.01.2022 um 17:50 +0100 schrieb Ricardo Wurmus:
>> Hi Guix,
>> 
>> does this pattern look familiar to you?
>> 
>>   (arguments
>>     (list
>>     #:phases
>>     '(modify-phases %standard-phases
>>       (add-after 'unpack 'i-dont-care
>>         (lambda _
>>           (substitute* "this-file"
>>             (("^# some unique string, oh, careful! gotta \\(escape\\)
>> this\\." m)
>>              (string-append m "\nI ONLY WANTED TO ADD THIS
>> LINE!\n"))))))))
>> 
>> This is a lot of boilerplate just to add a line to a file.  I’m using
>> “substitute*” but actually I don’t want to substitute anything.  I
>> just know that I need to add a line somewhere in “this-file”.
> Now many of these substitute*s are actually sane.  E.g. those which
> match the beginning of a defun in Emacs terms.  However, there for sure
> are cases in which I think "when all you have is a substitute*, every
> problem you have starts to look like... oh shit, I can't match this
> multi-line string, back to square 0".
>
>> CMakeLists.txt
> I feel your pain.
>
>> We have a lot of packages like that.  And while this boilerplate
>> pattern looks familiar to most of us now, it is really unclear.  It
>> is imperative and abuses regular expression matching when really it
>> should have been a patch.
> Now imperative is not really the bad thing here, but I'll get to that a
> little bit later when discussing your mockup.  I do however agree that
> it's too limited for its task.
>
>> There are a few reasons why we don’t use patches as often:
>> 
>> 1. the source code is precious and we prefer to modify the original
>> sources as little as necessary, so that users can get the source code
>> as upstream intended with “guix build -S foo”.  We patch the sources
>> primarily to get rid of bundled source code, pre-built binaries, or
>> code that encroaches on users’ freedom.
>> 
>> 2. the (patches …) field uses patch files.  These are annoying and
>> inflexible.  They have to be added to dist_patch_DATA in
>> gnu/local.mk, and they cannot contain computed store locations.  They
>> are separate from the package definition, which is inconvenient.
>> 
>> 3. snippets feel like less convenient build phases.  Snippets are not
>> thunked, so we can’t do some things that we would do in a build phase
>> substitution.  We also can’t access %build-inputs or %outputs.  (I
>> don’t know if we can use Gexps there.)
> Both patches and snippets serve the functions you've outlined in 1. 
> Now 2. is indeed an issue, but it's still an issue if you include patch
> as native input as well as the actual patch file you want to apply
> (assuming it is free of store locations, which can be and has been
> worked around).
>
>> It may not be possible to apply patches with computed store locations
>> — because when we compute the source derivation (which is an input to
>> the package derivation) we don’t yet know the outputs of the package
>> derivation.  But perhaps we can still agree on a more declarative way
>> to express patches that are to be applied before the build starts;
>> syntax that would be more declarative than a serious of brittle
>> substitute* expressions that latch onto hopefully unique strings in
>> the target files.
>> 
>> (We have something remotely related in etc/committer.scm.in, where we
>> define a record describing a diff hunk.)
>> 
>> Here’s a colour sample for the new bikeshed:
>> 
>>   (arguments
>>     (list
>>       #:patches
>>       #~(patch "the-file"
>>          ((line 10)
>>           (+ "I ONLY WANTED TO ADD THIS LINE"))
>>          ((line 3010)
>>           (- "maybe that’s better")
>>           (+ (string-append #$guix " is better"))
>>           (+ "but what do you think?")))))
> Now this thing is again just as brittle as the patch it encodes and if
> I know something about context-less patches then it's that they're
> super not trustworthy.

What do you mean here, with 'brittle' and 'trustworthy'? Is it the fact
that line numbers are hardcoded, compared to the substitute* approach?

> However, have you considered that something similar has been lying
> around in our codebase all this time and 99% of the packages ignore it
> because it's so obscure and well hidden?  Why yes, of course I'm
> talking about emacs-batch-edit-file.
>
> Now of course there are some problems when it comes to invoking Emacs
> inside Guix build.  For one, not every package has emacs-minimal
> available at build time and honestly, that's a shame.  Even then, you'd
> have to dance around and add emacs-utils to enable it.  And after that?
> You code Emacs Lisp in the middle of Guile code!  Now certainly, this
> UI can be improved.

> Particularly, I'd propose a set of monadic functions that operate on a
> buffer in much the same way Emacs does.  Now we wouldn't need to
> implement all of Emacs in that way (though we certainly could if given
> enough time).
>
> Basic primitives would be the following to name a few: search-forward,
> re-search-forward, goto-char, forward-line, point-min, insert,
> beginning-of-line, end-of-line, delete-region, point.  Of course we
> could rename them to be more guily, but that's a minor concern imo. 
> Notice how I omitted find-file and save-buffer, because that should be
> taken care of by the monad.

I'd prefer something that is declarative in the sense that the 'how' is
abstracted away, with a focus on the 'what'; both Ricardo's proposal and
emacs-batch-edit-file do this, and to a much lesser extent the current
substitute* mess. 'Navigation primitives' as you propose here are
perhaps abstract in the technical sense of the phrase, but still
indicate to me a focus on the 'how' instead of the 'what'.

Another nice property of keeping these concerns separated is that it
should be easy (or at least possible) to later reimplement
e.g. Ricardo's proposal on top of such monadic primitives.  Doing it the
other way around seems much less fun, if even feasible at all :-).

> WDYT, should we pursue that?  Or should we just add an Emacs to our
> native inputs?  :P

At the risk of escalating this perhaps-not-totally-serious proposal, I
wrote some snippets that should allow one to write elisp in
g-expressions using guile's built-in elisp reader + some hacks to
serialize guile's internal representation of parsed elisp to valid elisp
again.

- Jelle