build: improve sed robustness by not using sed

[fanquake]

While using sed can be handy to use for a quick-fix, these instances accumulate, and can become unmaintainable. Not only that, but using sed isn't necessarily robust and it can fail silently. Most of our usage is also missing any documentation explaining why something is being done, when it should be updated/removed etc.

Rather than relying on sed going forward, where possible, I've converted our sed usage into patches. These are easier to maintain, contain documentation, and should fail loudly when they don't apply.

The remaining sed usage, (1 in miniupnpc, the rest in qt), are non-trivial to remove, as they are using build-time variables, or some input from the environment.

This also steals 2 related commits out of #19716.

Related to #16838.

[practicalswift]

Concept ACK

[theuni]

Concept ACK. This will make simple bumps more complicated, but arguably that's a good thing.

Also +1 for self-documentation.

[laanwj]

Lovely PR title 😄

Concept ACK. I remember having some discussions about this in the past (with @dongcarl ?). It has always bothered me that sed can fail silently. This is especially problematic because changes can be lost without noticing when bumping versions. This has in the past resulted in losing propagation of hardening flags and such.

[hebasto]

Concept ACK.

[dongcarl]

Concept ACK!
All the ways I've tried fixing this before weren't satisfactory, and this is a nice and simple approach.

One q: I often see -N being using in most patch invocations, would we want that?

[fanquake]

I'm glad everyone agrees with the approach. I've pushed a fix the docs in one patch file.

One q: I often see -N being using in most patch invocations, would we want that?

I've done some testing and think it could be a worthwhile addition. It seems if anything it causes patch to "fail faster". i.e chain applying the same patch with and without it:

# no -N
patch -p1 -i a.patch && patch -p1 -i a.patch && patch -p1 -i a.patch
patching file test.cpp
patching file test.cpp
Reversed (or previously applied) patch detected!  Assume -R? [n] n
Apply anyway? [n] n
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file test.cpp.rej

# reset
git stash
Saved working directory and index state WIP on master: cdb5127 init

# Using -N
patch -N -p1 -i a.patch && patch -N -p1 -i a.patch && patch -N -p1 -i a.patch
patching file test.cpp
patching file test.cpp
Reversed (or previously applied) patch detected!  Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file test.cpp.rej

So without -N you'll be asked for input, as opposed to just failing.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• build: lrelease requires xml if not cross-building #19785 (build: lrelease requires xml if not cross-building by hebasto)
• depends: Split boost into build/host packages + bump + cleanup #19764 (depends: Split boost into build/host packages + bump + cleanup by dongcarl)
• build: Qt 5.15.x #19716 (build: Qt 5.15.x by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[hebasto]

Reviewing commits:

• 5f98794 "build: use patch rather than sed in bdb package"
• 53afb71 "build: use patch rather than sed in Boost package"
• 45f3360 "build: use patch rather than sed in fontconfig package"
• b214067 "build: use patch rather than sed in native_cctools package"
• e995635 "build: use patch rather than sed in zeromq package"
• 6d42439 "build: remove no-longer needed qt configure workaround"
• 5b121cb "build: remove no-longer needed qt workaround"
• 5bdea1d "build: replace pwd sed in qt package with a patch"
• e2b18a4 "build: replace FreeType back-compat sed with a patch in qt package"
• 37d9898 "build: replace qtranslations ltranslate sed with a patch in qt package"
• 0b0f480 "build: replace wingenminiupnpcstrings sed with a patch in miniupnpc package"

[hebasto]

Approach ACK 0b0f480.

I've verified that patch changes are equal to sed changes.
I've verified that dropped workarounds are effectively noop.

[DrahtBot]

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}