Re: RFC: new syntax for inline patches

[Ludovic Courtès]

Hi!

Ricardo Wurmus <rekado@elephly.net> skribis:

>   (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"))))))))

[...]

> 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.)

I agree that #1 is overrated.

As for #3, we could make ‘snippet’ thunked (a snippet can be a gexp
already).  We cannot refer to build inputs there, but that’s on purpose:
snippets, like patches, are supposed to be architecture-independent and
unable to insert store file names.

[...]

> (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?")))))

Like Attila my first reaction was skepticism.

… but thinking about it, we could have a <computed-patch> record,
similar to the <diff-hunk> record you mention; it would be a file-like
object that, when lowered, would give an actual patch.

So you could write:

  (origin
    ;; …
    (patches (list (computed-patch
                     (hunk (line 10) (+ "new line") (- "old line"))))))

The good thing is that the implementation of <computed-patch> would be
entirely orthogonal, separate from the package machinery.

OTOH, if we do that, we might as well write the actual patch right away.

I wonder how frequent the pattern we’re discussing is.  I know I’ve used
it a few times, but I wonder if it warrants sophisticated tooling.

Thoughts?

Ludo’.