From c6233d8bcd28cfbd367ff9a518f5ca762ef32391 Mon Sep 17 00:00:00 2001 From: Nassim Jahnke Date: Sat, 7 Dec 2024 17:41:32 +0100 Subject: [PATCH] Start updating contributing docs --- CONTRIBUTING.md | 130 +++++++++++++++----------------------- LICENSE.md | 4 +- README.md | 2 +- scripts/checkoutpr.sh | 54 ---------------- scripts/upstreamCommit.sh | 38 ----------- scripts/upstreamMerge.sh | 41 ------------ 6 files changed, 55 insertions(+), 214 deletions(-) delete mode 100755 scripts/checkoutpr.sh delete mode 100755 scripts/upstreamCommit.sh delete mode 100755 scripts/upstreamMerge.sh diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 475d77bbb8..9e48879b0e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,9 +17,6 @@ closing the PR instead of marking it as merged. We much prefer to have PRs show as merged, so please do not use repositories on organizations for PRs. -See for more information on the -issue. - ## Requirements To get started with PRing changes, you'll need the following software, most of @@ -51,14 +48,15 @@ javac 21.0.3 ## Understanding Patches -Paper is mostly patches and extensions to Spigot. These patches/extensions are -split into different directories which target certain parts of the code. These -directories are: +Unlike the API and its implementation, modifications to Vanilla source files +are done through patches. These patches/extensions are split into different +three different sets, which are: -- `Paper-API` - Modifications to `Spigot-API`/`Bukkit`; -- `Paper-Server` - Modifications to `Spigot`/`CraftBukkit`. +- `sources` - Per-file patches to individual Minecraft classes; +- `resources` - Per-file patches to Minecraft data files; +- `features` - Larger feature patches that modify multiple Minecraft classes. -Because the entire structure is based on patches and git, a basic understanding +Because this entire structure is based on patches and git, a basic understanding of how to use git is required. A basic tutorial can be found here: . @@ -67,22 +65,32 @@ Assuming you have already forked the repository: 1. Clone your fork to your local machine; 2. Type `./gradlew applyPatches` in a terminal to apply the changes from upstream. On Windows, replace the `./` with `.\` at the beginning for all `gradlew` commands; -3. cd into `Paper-Server` for server changes, and `Paper-API` for API changes. - +3. cd into `paper-server` for server changes, and `paper-api` for API changes. +**Only changes made in `paper-server/src/vanilla` have to deal with the patch system.** -`Paper-Server` and `Paper-API` aren't git repositories in the traditional sense: +`paper-server/src/vanilla` is not a git repositories in the traditional sense. Its +initial commits are the decompiled and deobfuscated Vanilla source files. The per-file +patches are applied on top of these files as a single, large commit, which is then followed +by the individual feature-patch commits. -- `base` points to the unmodified source before Paper patches have been applied. -- Each commit after `base` is a patch. +### Modifying (per-file) Vanilla patches -## Adding Patches +This is generally what you need to do when editing Vanilla files. Updating our +per-file patches is as easy as making your changes and then running +# TODO +in the root directory. If nothing went wrong, you can rebuild patches with +`./gradlew rebuildPatches` and finally commit and PR the patch changes. -Adding patches to Paper is very simple: +### Adding larger feature patches -1. Modify `Paper-Server` and/or `Paper-API` with the appropriate changes; +Feature patches are exclusively used for large-scale changes that are hard to +track and maintain, and that can be optionally dropped, such as the more involved +optimizations we have. This makes it easier to update Paper during Minecraft updates, +since we can temporarily drop these patches and reapply them later. + +Adding such patches is very simple: + +1. Modify `paper-server/src/vanilla` with the appropriate changes; 1. Type `git add .` inside these directories to add your changes; 1. Run `git commit` with the desired patch message; 1. Run `./gradlew rebuildPatches` in the main directory to convert your commit into a new @@ -94,13 +102,11 @@ Your commit will be converted into a patch that you can then PR into Paper. > ❗ Please note that if you have some specific implementation detail you'd like > to document, you should do so in the patch message *or* in comments. -## Modifying Patches +### Modifying larger feature patches -Modifying previous patches is a bit more complex. -Similar to adding patches, the methods to modify a patch are applied inside -the `Paper-Server` and/or `Paper-API` folders. +Modifying existing feature patches is slightly more complex. -### Method 1 +#### Method 1 This method works by temporarily resetting your `HEAD` to the desired commit to edit it using `git rebase`. @@ -135,7 +141,7 @@ later; - This will modify the appropriate patches based on your commits. 1. PR your modified patch file(s) back to this repository. -### Method 2 - Fixup commits +#### Method 2 - Fixup commits If you are simply editing a more recent commit or your change is small, simply making the change at HEAD and then moving the commit after you have tested it @@ -144,7 +150,7 @@ may be easier. This method has the benefit of being able to compile to test your change without messing with your HEADs. -#### Manual method +##### Manual method 1. Make your change while at HEAD; 1. Make a temporary commit. You don't need to make a message for this; @@ -159,7 +165,7 @@ move it under the line of the patch you wish to modify; - This will modify the appropriate patches based on your commits. 1. PR your modified patch file(s) back to this repository. -#### Automatic method +##### Automatic method 1. Make your change while at HEAD; 1. Make a fixup commit. `git commit -a --fixup `; @@ -181,13 +187,11 @@ need to "save" the changes. Steps to rebase a PR to include the latest changes from `master`. These steps assume the `origin` remote is your fork of this repository and `upstream` is the official PaperMC repository. -1. Pull the latest changes from upstreams master: `git checkout master && git pull upstream master`. -1. Checkout feature/fix branch and rebase on master: `git checkout patch-branch && git rebase master`. +1. Pull the latest changes from upstreams master: `git switch main && git pull upstream main`. +1. Checkout feature/fix branch and rebase on master: `git checkout patch-branch && git rebase main`. 1. Apply updated patches: `./gradlew applyPatches`. 1. If there are conflicts, fix them. - * If there are conflicts within `Paper-API`, after fixing them run `./gradlew rebuildApiPatches` before re-running `./gradlew applyPatches`. - * The API patches are applied first, so if there are conflicts in the API patches, the server patches will not be applied. -1. If your PR creates new patches instead of modifying existing ones, in both the `Paper-Server` and `Paper-API` directories, ensure your newly-created patch is the last commit by either: +1. If your PR creates new feature patches instead of modifying existing ones, ensure your newly-created patch is the last commit by either: * Renaming the patch file with a large 4-digit number in front (e.g. 9999-Patch-to-add-some-new-stuff.patch), and re-applying patches. * Running `git rebase --interactive base` and moving the commits to the end. 1. Rebuild patches: `./gradlew rebuildPatches`. @@ -207,8 +211,7 @@ when making and submitting changes. ## Formatting -All modifications to non-Paper files should be marked. The one exception to this is -when modifying javadoc comments, which should not have these markers. +All modifications to Vanilla files should be marked. - You need to add a comment with a short and identifiable description of the patch: `// Paper start - ` @@ -246,11 +249,20 @@ The only exception to this is if a line would otherwise be way too long/filled w hard to parse generics in a case where the base type itself is already obvious ### Imports -When adding new imports to a class in a file not created by the current patch, use the fully qualified class name +When adding new imports to a Vanilla class (or if you're editing feature patches), use the fully qualified class name instead of adding a new import to the top of the file. If you are using a type a significant number of times, you can add an import with a comment. However, if its only used a couple of times, the FQN is preferred to prevent future patch conflicts in the import section of the file. +```java +import net.minecraft.server.MinecraftServer; +// don't add import here, use FQN like below + +public class SomeVanillaClass { + public final org.bukkit.Location newLocation; // Paper - add location +} +``` + ### Nullability annotations We are in the process of switching nullability annotation libraries, so you might need to use one or the other: @@ -262,17 +274,8 @@ are assumed to be non-null by default. For less obvious placing such as on gener **For classes added by upstream**: Keep using both `@Nullable` and `@NotNull` from `org.jetbrains.annotations`. These will be replaced later. -```java -import org.bukkit.event.Event; -// don't add import here, use FQN like below - -public class SomeEvent extends Event { - public final org.bukkit.Location newLocation; // Paper - add location -} -``` - ## Access Transformers -Sometimes, vanilla or CraftBukkit code already contains a field, method, or type you want to access +Sometimes, Vanilla code already contains a field, method, or type you want to access but the visibility is too low (e.g. a private field in an entity class). Paper can use access transformers to change the visibility or remove the final modifier from fields, methods, and classes. Inside the `build-data/paper.at` file, you can add ATs that are applied when you `./gradlew applyPatches`. You can read about the format of ATs @@ -304,7 +307,7 @@ diff --git a/build.gradle.kts b/build.gradle.kts ## Patch Notes -When submitting patches to Paper, we may ask you to add notes to the patch +When submitting feature patches to Paper, we may ask you to add notes to the patch header. While we do not require it for all changes, you should add patch notes when the changes you're making are technical, complex, or require an explanation of some kind. It is very likely that your patch will remain long after we've all @@ -315,8 +318,8 @@ These notes should express the intent of your patch, as well as any pertinent technical details we should keep in mind long-term. Ultimately, they exist to make it easier for us to maintain the patch across major version changes. -If you add a message to your commit in the `Paper-Server`/`Paper-API` -directories, the rebuild patches script will handle these patch notes +If you add a message to your commit in the Vanilla source directory, +the rebuild patches script will handle these patch notes automatically as part of generating the patch file. If you are not extremely careful, you should always just `squash` or `amend` a patch (see the above sections on modifying patches) and rebuild. @@ -455,33 +458,6 @@ If you use Maven to build your plugin: ## Frequently Asked Questions -### I can't find the NMS file I need! - -By default, Paper (and upstream) only import files we make changes to. If you -would like to make changes to a file that isn't present in `Paper-Server`'s -source directory, you just need to add it to our import script ran during the -patching process. - -1. Save (rebuild) any patches you are in the middle of working on! Their -progress will be lost if you do not; -1. Identify the name(s) of the file(s) you want to import. - - A complete list of all possible file names can be found at - `./Paper-Server/.gradle/caches/paperweight/mc-dev-sources/net/minecraft/`. You might find - [MappingViewer] useful if you need to translate between Mojang and Spigot mapped names. -1. Open the file at `./build-data/dev-imports.txt` and add the name of your file to -the script. Follow the instructions there; -1. Re-patch the server `./gradlew applyPatches`; -1. Edit away! - -> ❗ This change is temporary! **DO NOT COMMIT CHANGES TO THIS FILE!** -> Once you have made your changes to the new file, and rebuilt patches, you may -> undo your changes to `dev-imports.txt`. - -Any file modified in a patch file gets automatically imported, so you only need -this temporarily to import it to create the first patch. - -To undo your changes to the file, type `git checkout build-data/dev-imports.txt`. - ### My commit doesn't need a build, what do I do? Well, quite simple: You add `[ci skip]` to the start of your commit subject. @@ -512,5 +488,3 @@ everything like usual. > ❗ Do not use the `/mnt/` directory in WSL! Instead, mount the WSL directories > in Windows like described here: > - -[MappingViewer]: https://mappings.cephx.dev/ diff --git a/LICENSE.md b/LICENSE.md index 3a25fbbf25..c910b95922 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -1,8 +1,8 @@ -Paper inherits its licensing from upstream projects. +Paper inherits its licensing from the included upstream projects. As such, Paper is licensed under the [GNU General Public License version 3](licenses/GPL.md); as it inherits it from Spigot, -who in turn inherits it from the original Bukkit and Craftbukkit projects. +who in turn inherits it from the original Bukkit and CraftBukkit projects. Any author who is _not_ listed below should be presumed to have released their work under the original [GPL](licenses/GPL.md) license. diff --git a/README.md b/README.md index db2db9ddd1..567114a550 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Run the Paperclip jar directly from your server. Just like old times How To (Plugin Developers) ------ -* See our API patches [here](patches/api) +* See our API [here](paper-api) * See upcoming, pending, and recently added API [here](https://github.com/orgs/PaperMC/projects/2/views/4) * Paper API javadocs here: [papermc.io/javadocs](https://papermc.io/javadocs/) #### Repository (for paper-api) diff --git a/scripts/checkoutpr.sh b/scripts/checkoutpr.sh deleted file mode 100755 index 204fb0ed9f..0000000000 --- a/scripts/checkoutpr.sh +++ /dev/null @@ -1,54 +0,0 @@ -#!/usr/bin/env bash -if [ -z "$1" ]; then - echo "$0 " - exit 1; -fi -repo=$(git remote get-url origin | sed -E 's/(.*@)?github.com(:|\/)//g' | sed 's/.git$//g') -data=$(curl -q https://api.github.com/repos/$repo/pulls/$1 2>/dev/null) -url=$(echo -e "$data" | grep --color=none ssh_url | head -n 1 |awk '{print $2}' | sed 's/"//g' | sed 's/,//g') -ref=$(echo -e "$data" | grep --color=none '"head":' -A 3 | grep ref | head -n 1 |awk '{print $2}' | sed 's/"//g' | sed 's/,//g') -prevbranch=$(\git branch --no-color 2> /dev/null | sed -e '/^[^*]/d' -e 's/* \(.*\)/\1/') -branch="pr/$1" -up="pr-$1" -git remote remove $up 2>&1 1>/dev/null -git remote add -f $up $url -git branch -D $branch 2>/dev/null 1>&2 -git checkout -b $branch $up/$ref 2>/dev/null|| true -echo "Merging $prevbranch into $branch" -git fetch origin -read -p "Press 'm' to merge, 'r' to rebase, or 'n' for nothing" -n 1 -r >&2 -echo -if [[ "$REPLY" =~ ^[Mm]$ ]]; then - git merge origin/$prevbranch -elif [[ "$REPLY" =~ ^[Rr]$ ]]; then - git rebase master -fi -echo "Dropping to new shell, exit to delete the refs" -"${SHELL:-bash}" -i - -read -p "Press 'p' to push. " -n 1 -r >&2 -echo -pushed=0 -if [[ "$REPLY" =~ ^[Pp]$ ]]; then - git push $up $branch:$ref -f - pushed=1 - echo "Pushed" >&2 -fi - -echo "Deleting branch/upstream" -git checkout $prevbranch -if [[ "$pushed" == "1" ]]; then - read -p "Press 'm' to merge or 'r' to rebase merge " -n 1 -r >&2 - if [[ "$REPLY" =~ ^[Mm]$ ]]; then - git merge $branch - fi - if [[ "$REPLY" =~ ^[Rr]$ ]]; then - git merge --ff-only $branch - fi -fi - -git branch -D $branch -git remote remove $up -git gc -#git branch -u $up/$ref $branch -#git checkout $branch diff --git a/scripts/upstreamCommit.sh b/scripts/upstreamCommit.sh deleted file mode 100755 index e994d90a5f..0000000000 --- a/scripts/upstreamCommit.sh +++ /dev/null @@ -1,38 +0,0 @@ -#!/usr/bin/env bash -( -set -e -PS1="$" - -function changelog() { - base=$(git ls-tree HEAD $1 | cut -d' ' -f3 | cut -f1) - cd $1 && git log --oneline ${base}...HEAD | sed -E 's/(^[0-9a-f]{8,}( (SPIGOT-[0-9]{1,4}|MC-[0-9]{1,6}),?)* |Revert ")#([0-9]+)/\1PR-\4/' -} -bukkit=$(changelog work/Bukkit) -cb=$(changelog work/CraftBukkit) -spigot=$(changelog work/Spigot) - -updated="" -logsuffix="" -if [ ! -z "$bukkit" ]; then - logsuffix="$logsuffix\n\nBukkit Changes:\n$bukkit" - updated="Bukkit" -fi -if [ ! -z "$cb" ]; then - logsuffix="$logsuffix\n\nCraftBukkit Changes:\n$cb" - if [ -z "$updated" ]; then updated="CraftBukkit"; else updated="$updated/CraftBukkit"; fi -fi -if [ ! -z "$spigot" ]; then - logsuffix="$logsuffix\n\nSpigot Changes:\n$spigot" - if [ -z "$updated" ]; then updated="Spigot"; else updated="$updated/Spigot"; fi -fi -disclaimer="Upstream has released updates that appear to apply and compile correctly.\nThis update has not been tested by PaperMC and as with ANY update, please do your own testing" - -if [ ! -z "$1" ]; then - disclaimer="$@" -fi - -log="${UP_LOG_PREFIX}Updated Upstream ($updated)\n\n${disclaimer}${logsuffix}" - -echo -e "$log" | git commit -F - - -) || exit 1 diff --git a/scripts/upstreamMerge.sh b/scripts/upstreamMerge.sh deleted file mode 100755 index 319a71695d..0000000000 --- a/scripts/upstreamMerge.sh +++ /dev/null @@ -1,41 +0,0 @@ -#!/usr/bin/env bash - -( -set -e -PS1="$" -basedir="$(cd "$1" && pwd -P)" -workdir="$basedir/work" -gitcmd="git -c commit.gpgsign=false" - -updated="0" -function getRef { - git ls-tree $1 $2 | cut -d' ' -f3 | cut -f1 -} -function update { - cd "$workdir/$1" - $gitcmd fetch && $gitcmd clean -fd && $gitcmd reset --hard origin/master - refRemote=$(git rev-parse HEAD) - cd ../ - $gitcmd add --force $1 - refHEAD=$(getRef HEAD "$workdir/$1") - echo "$1 $refHEAD - $refRemote" - if [ "$refHEAD" != "$refRemote" ]; then - export updated="1" - fi -} - -update Bukkit -update CraftBukkit -update Spigot - -if [[ "$2" = "all" || "$2" = "a" ]] ; then - update BuildData -fi -if [ "$updated" == "1" ]; then - echo "Rebuilding patches without filtering to improve apply ability" - cd "$basedir" - ./gradlew cleanCache || exit 1 # todo: Figure out why this is necessary - ./gradlew applyPatches -Dpaperweight.debug=true || exit 1 - ./gradlew rebuildPatches || exit 1 -fi -)