Re: RFC: new syntax for inline patches

[Liliana Marie Prikler]

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.

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.

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