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