[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: RFC: new syntax for inline patches
From: |
Efraim Flashner |
Subject: |
Re: RFC: new syntax for inline patches |
Date: |
Wed, 12 Jan 2022 19:56:57 +0200 |
On Tue, Jan 04, 2022 at 05:50:31PM +0100, Ricardo Wurmus wrote:
> 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”.
>
> Or maybe it’s a CMakeLists.txt file that inexplicably wants to download
> stuff? I should patch that file but it’s a multi-line change. So I’m
> trying to do the same as above with several different anchor strings to
> comment out lines.
>
> 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.
>
> 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.
It also feels wrong to add a 30 line patch, taking into account the
header bits, to make a 3 line change.
> 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 believe you can leave out the modules line and use a gexp in the
snippet (without the "'(begin" portion )
> I feel that the first point is perhaps a little overvalued. I have
> often felt annoyed that I had to manually apply all this build phase
> patching to source code obtained with “guix build -S”, but I never felt
> that source code I got from “guix build -S” was too far removed from
> upstream.
>
> 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?")))))
I have on at least one occasion stopped myself from trying to use ed (it
IS the standard editor) to apply something that SHOULD BE trivial to
change.
--
Efraim Flashner <efraim@flashner.co.il> רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
signature.asc
Description: PGP signature
Re: RFC: new syntax for inline patches, Ludovic Courtès, 2022/01/08
Re: RFC: new syntax for inline patches,
Efraim Flashner <=