build: Implement patching for the project-local Meson installation

[lhearachel]

This implements a basic patching-mechanism for the project-local checkout of Meson. For details on the Why and the How, refer to docs/meson/patching.md.

I want lots of eyes on this PR, because this is a relatively high-impact change. I especially want some extra scrutiny on the documentation; this process will probably always be a bit gray, but I think that that is fine so long as we are adamantly selective in what we choose to inherit as a maintained patch.

[Kuruyia]

question: A couple of questions about this process:

1. Do we want to introduce some in-repo way to keep track of the upstream PR linked to each patch? (could be some markdown file or a comment directly in each patch)
   As I understand, it is expected to have the upstream PR link in the related pokeplatinum PR description message. Do we want to rely on GitHub to hold this information?
2. At what point do we consider that a PR can be opened here to introduce a new patch? (once the first upstream review has been done? once the upstream PR has been accepted and a version milestone added?)

thought: For patches that already have an upstream PR open, we could have a script that automatically downloads the diff from e.g. https://patch-diff.githubusercontent.com/raw/mesonbuild/meson/pull/15683.diff and sets up the .patch file here (which could also be used to sync a local .patch with its PR).

(just thinking out loud here; it's so easy to get a patch/diff from a GitHub PR that I feel like we should at least think about doing something with this!)

[lhearachel]

1. Do we want to introduce some in-repo way to keep track of the upstream PR linked to each patch? (could be some markdown file or a comment directly in each patch) As I understand, it is expected to have the upstream PR link in the related pokeplatinum PR description message. Do we want to rely on GitHub to hold this information?

I want to keep the links in the commit-text, so it's tracked in git itself. But leaving a comment in the patch file is also a good thought.

2. At what point do we consider that a PR can be opened here to introduce a new patch? (once the first upstream review has been done? once the upstream PR has been accepted and a version milestone added?)

I think this is the point where it can be gray. For big features (e.g., syntax-extensions to the interpreter, like the feat patch included in this PR), I wouldn't want to support it unless there is also acceptance from the upstream. For smaller features or fixes (e.g., the fix patch included in this PR), I think that we can be more lenient.

Maybe it is good to have a formal process here, but I'm not so sure. The idea behind this setup existing at all is to loosen our shackles a bit and permit us to make use of changes that we feel are necessary or important to us. In that vein, it's all vibes, so I think that it's okay to leave the process relatively informal.

thought: For patches that already have an upstream PR open, we could have a script that automatically downloads the diff from e.g. https://patch-diff.githubusercontent.com/raw/mesonbuild/meson/pull/15683.diff and sets up the .patch file here (which could also be used to sync a local .patch with its PR).

(just thinking out loud here; it's so easy to get a patch/diff from a GitHub PR that I feel like we should at least think about doing something with this!)

Mmh, no, I think this is a good thought! I can whip something up here that would be useful, I think.

Along that thread, do you think stripping these patches down to their bare essentials is the right approach? Should we leave in changes to documentation, etc.? That feels like a bit of unnecessary bloat in our repo.

[lhearachel]

I've added two commits that are made possible by these patches. As a demo for the warnings, I made the following changes to Bulbasaur:

diff --git a/res/pokemon/bulbasaur/data.json b/res/pokemon/bulbasaur/data.json
index 0242d8e60..e392f839e 100644
--- a/res/pokemon/bulbasaur/data.json
+++ b/res/pokemon/bulbasaur/data.json
@@ -85,7 +85,8 @@
             "MOVE_SNORE",
             "MOVE_SYNTHESIS",
             "MOVE_SEED_BOMB",
-            "MOVE_KNOCK_OFF"
+            "MOVE_KNOCK_OFF",
+            "MOVE_GIGA_DRAIN"
         ],
         "egg_moves": [
             "MOVE_LIGHT_SCREEN",
@@ -116,7 +117,9 @@
         "type": "FOOTPRINT_TYPE_CUTE"
     },
     "pokedex_data": {
+        "height_meters": 0.7,
         "height_inches": 28,
+        "weight_kilograms": 6.9,
         "weight_pounds": 15.2,
         "body_shape": "SHAPE_QUADRUPED",
         "trainer_scale_f": 272,

The log of the resulting build looks like this:

> make
subprojects/meson-1.10.0/meson.py configure build -Dgdb_debugging=false -Dlogging_enabled=false
ninja -C build data
ninja: Entering directory `build'
[  3% 1/27] Generating res/pokemon/species_data with a custom command (wrapped by meson to set env)
/home/rachel/code/git/pokeplatinum/meson-patching/res/pokemon/bulbasaur/data.json:119:21: warning: .pokedex_data: conflicting unit-systems given for species height; metric units will be used
  119 |     "pokedex_data": {
      |                     ^
/home/rachel/code/git/pokeplatinum/meson-patching/res/pokemon/bulbasaur/data.json:120:26: note: .pokedex_data.height_meters: metric units defined here
  120 |         "height_meters": 0.7,
      |                          ^^^
/home/rachel/code/git/pokeplatinum/meson-patching/res/pokemon/bulbasaur/data.json:121:26: note: .pokedex_data.height_inches: imperial units defined here
  121 |         "height_inches": 28,
      |                          ^^
/home/rachel/code/git/pokeplatinum/meson-patching/res/pokemon/bulbasaur/data.json:119:21: warning: .pokedex_data: conflicting unit-systems given for species weight; metric units will be used
  119 |     "pokedex_data": {
      |                     ^
/home/rachel/code/git/pokeplatinum/meson-patching/res/pokemon/bulbasaur/data.json:122:29: note: .pokedex_data.weight_kilograms: metric units defined here
  122 |         "weight_kilograms": 6.9,
      |                             ^^^
/home/rachel/code/git/pokeplatinum/meson-patching/res/pokemon/bulbasaur/data.json:123:26: note: .pokedex_data.weight_pounds: imperial units defined here
  123 |         "weight_pounds": 15.2,
      |                          ^^^^
/home/rachel/code/git/pokeplatinum/meson-patching/res/pokemon/bulbasaur/data.json:89:13: warning: .learnset.by_tutor[6]: 'MOVE_GIGA_DRAIN' is not available from any move tutors and will be skipped
   89 |             "MOVE_GIGA_DRAIN"
      |             ^^^^^^^^^^^^^^^^^

[100% 27/27] Generating res/text/pl_msg.narc with a custom command
ninja -C build pokeplatinum.us.nds
ninja: Entering directory `build'
[100% 7/7] Generating pokeplatinum.us.nds with a custom command
subprojects/meson-1.10.0/meson.py test -C build
ninja: Entering directory `/home/rachel/code/git/pokeplatinum/meson-patching/build'
ninja: no work to do.
1/4 pokeplatinum:SBIN Checksums - Shared              OK              0.02s
2/4 pokeplatinum:SBIN Checksums - For Revision        OK              0.02s
3/4 pokeplatinum:Filesystem Checksums                 OK              0.06s
4/4 pokeplatinum:ROM Checksum                         OK              0.09s

Ok:                4
Fail:              0

Full log written to /home/rachel/code/git/pokeplatinum/meson-patching/build/meson-logs/testlog.txt

Note that it still produces a match. The extraneous move in its by-tutor learnset (which is unavailable from any tutor) is ignored, and I used the canonical metric values for its height and weight.

[Kuruyia]

I want to keep the links in the commit-text, so it's tracked in git itself.

Sounds good to me.

I'm simply worried this is going to be forgotten. Looking at the commit history on main, the current norm seems to be to not carry over the PR description when squash-merging.

I think this is the point where it can be gray. For big features (e.g., syntax-extensions to the interpreter, like the feat patch included in this PR), I wouldn't want to support it unless there is also acceptance from the upstream. For smaller features or fixes (e.g., the fix patch included in this PR), I think that we can be more lenient.

Maybe it is good to have a formal process here, but I'm not so sure. The idea behind this setup existing at all is to loosen our shackles a bit and permit us to make use of changes that we feel are necessary or important to us. In that vein, it's all vibes, so I think that it's okay to leave the process relatively informal.

Also sounds good to me. We can leave that to the appreciation of the contributor and reviewers on a case-by-case basis.

Along that thread, do you think stripping these patches down to their bare essentials is the right approach? Should we leave in changes to documentation, etc.? That feels like a bit of unnecessary bloat in our repo.

I think that makes perfect sense. If we expect reviewers here to check patches done to Meson, I think it's best to remove what can be considered noise to reduce churn.

As I understand from your Meson PRs, it's only a matter of removing the diffs of files under docs/ and test cases/?

[lhearachel]

I'm simply worried this is going to be forgotten. Looking at the commit history on main, the current norm seems to be to not carry over the PR description when squash-merging.

Yeah, it is, and that it behooves us to be thorough. A comment-line at the top of the diff sounds like a good addition to me.

As I understand from your Meson PRs, it's only a matter of removing the diffs of files under docs/ and test cases/?

Really, it's anything that isn't under mesonbuild/, which is the actual package-folder. That arguably makes it easier. 🙂

[lhearachel]

Commit 86b05b2 contains some additional changes:

1. I've decorated the existing patch-files in the commit with comments linking to their upstream PRs.
2. I added a script that can be used to fetch the content of a PR on GitHub and store it as a local patch.
3. The documentation now mentions (1) as a general requirement and (2) as a way to set up a patch.

[Kuruyia]

praise: Really nice seeing a shell script!

^{I would have done the lazy Python route >_<}

[lhearachel]

I didn't want to go through the effort of grabbing requests or running curl through a subprocess. 😓

[lhearachel]

Changes applied. I'd also like to get some other maintainers' eyes on this, if only to test the functionality (and also the developer experience).

[ravepossum]

question/nitpick: What's the rationale behind staging here? And would there be a benefit to actually committing the changes? I guess it would be kind of weird to commit on others' behalves, but it could make it easier to test any changes in-repo by having a totally clean working directory by default.

[lhearachel]

I think there's actually a larger problem here: applying new patches from an upstream pull is either awkward or broken.

[lhearachel]

Slipping this back to draft until I have more time to work on it; the PRs referenced in Meson have both been accepted, so there is additional thought to be made here about the approach.