[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 20:06:06 +0200 |
On Sat, Jan 08, 2022 at 10:34:15PM +0100, Ludovic Courtès wrote:
> 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?
I'm OK with needing to change the exact line needed if it moves (EXACTLY
line 10, not 8 or 12 or 25).
It comes up a lot when glibc headers move or split, suddenly we're
looking at the sources, trying to find somewhere to stuff in an extra
include statement. Or qt-5.11, I think we came up with 3 different ways
of dealing with the missing header over the 10 patches.
--
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 <=
Re: RFC: new syntax for inline patches, Efraim Flashner, 2022/01/12