Skip to content

feat: allow for specifying ATs for upstream sources#377

Open
Toffikk wants to merge 38 commits intoPaperMC:mainfrom
CraftCanvasMC:feat/ats-for-upstream-sources
Open

feat: allow for specifying ATs for upstream sources#377
Toffikk wants to merge 38 commits intoPaperMC:mainfrom
CraftCanvasMC:feat/ats-for-upstream-sources

Conversation

@Toffikk
Copy link
Copy Markdown
Contributor

@Toffikk Toffikk commented Jan 25, 2026

These changes provide fork developers with a way to specify an AT file (or have it detected automatically) for upstream sources, such as paper-server and paper-api for a fork of Paper. This allows fork developers to benefit from using ATs while modifying these sources without needing to clutter patches, with lots of manual access transforms, and as such this greatly improves the overall comfort of making more involving changes, in the sources. It also allows for fork devs to be able to organize ATs in one place which makes it easier to review later.

The implementation proposed in this commit works as follows:

  1. Derive the location of the build-data dir from the output dir specified in the PatchRepo/Directory's config.
    • This sounds like it should be ideally derived from either the Project instance or Layout by injecting, however we cannot do it this way, as the upstream server projects are set up in the fork’s server project, not as part of the upstream setup like API is, meaning they either can’t access their ATs or other issues appear. The layout approach has even more downsides as it inherits the downsides of the aforementioned approach and the build-data dir would be forced to appear in the server project.
    • Therefore, I believe using the outputDir to calculate the possible root project is the best way to make it work, as it works on fork setups and doesn’t cause issues, and we can also safely assume that it will work for everyone as every fork and fork template is following the convention of specifying their outputDir’s in a subfolder of the root project, and regardless it can be overrode like the minecraft AT file currently can be so this is correct in practice.
  2. Search for an AT file, by the name of the patch repo/directory as specified.
  3. Apply them before applying source patches.

The locations in steps 1 and 2 can be overriden per patch repo/directory by just overriding the val.

In order to achieve this change, we introduce a new task, with its implementation sitting in SetupForkUpstreamSources which applies those ATs. Everything else not explained here or even explained should follow the behaviour of AT application for minecraft sources.

The change has been tested in production in an organisation fork of paperweight for over 6 months with both a nested fork project that uses them heavily and then fork projects depending on the nested fork project, and no issues have been reported or arose, so i'm confident that this implementation will work properly.

For additional reference:

  • weaver -> where this impl was implemented first and tested
  • Baguette -> an example fork using this feature

These changes provide fork developers with a way to specify an AT file (or have it detected automatically) for upstream sources,
such as `paper-server` and `paper-api` for a fork of Paper.
This allows fork developers to benefit from using ATs while modifying these sources without needing to clutter patches,
with lots of manual access transforms, and as such this greatly improves the overall comfort of making more involving changes,
in the sources. It also allows for fork devs to be able to organize ATs in one place which makes it easier to review later.

The implementation proposed in this commit works as follows:
1. Derive the location of the build-data dir from the output dir specified in the PatchRepo/Directory's config.
2. Search for an AT file, by the name of the patch repo/directory as specified.
3. Apply them before applying source patches.

The locations in steps 1 and 2 can be overriden per patch repo/directory by just overriding the val.

In order to achieve this change, we introduce a new task, with its implementation sitting in SetupForkUpstreamSources which applies those ATs.
Everything else not explained here or even explained should follow the behaviour of AT application for minecraft sources.

The change has been tested in production in an organisation fork of paperweight for over 6 months with both a nested fork project that uses them heavily and then fork projects depending on the nested fork project,
and no issues have been reported or arose, so i'm confident that this implementation will work properly.
@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from 342d27d to 3665104 Compare January 26, 2026 00:02
@Toffikk
Copy link
Copy Markdown
Contributor Author

Toffikk commented Jan 26, 2026

note: if #376 gets merged before this, this pr needs to be updated,
however if this gets merged first, then the other pr needs to be updated.
(i already have the proper changes on my local fork)

@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from 8cc10ea to 55d76c0 Compare January 26, 2026 13:03
@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from 310f23a to 4f1edf1 Compare February 3, 2026 17:27
@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from 9f01971 to 617e64b Compare February 3, 2026 17:31
This is needed for being fully able to represent types in API projects as they are resolved from the root project and not nested, meaning the compile classpath passed is null as its from the root project.
fixes more binary representation issues
…minecraft ATs

This fixes some issues with JST and forks as there are custom types from the API imported by paper, which arent contained in the mache libraries config and that causes JST to fail to create binary representations for some methods/classes thus not allowing to AT them, this fixes it
@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from 951a35a to 09cce04 Compare February 3, 2026 19:33
…ath for minecraft ATs"

This reverts commit 09cce04.

seems to fail the tests for some reason, a dev can take a look at this, although the output of the log file is correct locally
@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from 4f2f7c7 to bdf8721 Compare February 24, 2026 01:39
@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from bdf8721 to a20d4ac Compare February 24, 2026 01:46
jpenilla and others added 6 commits April 24, 2026 19:58
* Start Paper 26.1 development groundwork

  - remove legacy Spigot/reobf task pipeline from paperweight-core and
  paperweight-lib
  - simplify paperclip packaging to use the built server jar directly
  - update codebook task wiring and bump fallback toolchain target to Java 25
  - add userdev setup handler v7 and support dev bundle format 8 for 26.1+
  - add patchroulette cancel --force for recovery workflows
  - mark legacy reobf extraction path in old setup handler as follow-up work

* Allow selecting specific patches for roulette apply

* Add autoAccept flag to PatchRouletteApply

* Fix typo

* Update tests

* spotlessApply
@Toffikk
Copy link
Copy Markdown
Contributor Author

Toffikk commented Apr 24, 2026

I believe this should be ready for review now as the main concern i had with this impl has been addressed. This PR will also require small changes to the fork examples which i'll also take care of if interest in merging this is expressed

@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch 2 times, most recently from 197c12a to ac12a69 Compare April 25, 2026 23:15
@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from ac12a69 to 283ee51 Compare April 25, 2026 23:19
@Toffikk
Copy link
Copy Markdown
Contributor Author

Toffikk commented Apr 25, 2026

This PR will require Paper itself to also add the io.papermc.paperweight.dependency-bridge plugin to its API build file so that forks' builds succeed, even tho it will not use it.

edit: nvm this might not work, the issue is that the apply(upstreamName)SingleFilePatches task needs to run BEFORE all other tasks and cannot be a dependsOn in applyAllPatches even with some shenanigans as gradle will start evaluating other tasks connected to it too and it'll fail for new builds that declare the API project as a jstClasspath dependency in root. If anyone has any idea on how to make it work i'd REALLY appreciate it

One solution that I can think of would be making the apply${upstream.name.capitalized()}Patches a nested task like applyAllServerPatches under applyAllPatches, this would make it work but the task names for the API patches would change to include the fork's name inside of them. so applyPaperApiFilePatches -> applyForkPaperApiFilePatches

The above solution would have the obvious disadvantage of not matching the naming of the server related tasks which wouldn't have the prefix.

Another solution could be doing a non-trivial rework of the patcher tasks but it might be out of scope and i'm not sure how to about it; It could possibly consist of apply tasks always applying single file patches first for the entire fork chain, however i'm not too keen on this idea as it might not even work out. Maybe if we also immediately copied it downstream before applying the patch(?)

Or just telling people to run the single file apply task first before applyAllPatches 🤣 which would be the "easiest" way
or dropping the dep bridge altogether and making the api project behave like the server project instead of patching api sources from root.😕

We can eventually just say fuck it to JST and not provide it with the classpath for the API project, it will resolve most of the classes anyway, as I doubt people plan to AT Adventure related methods (this is how it was before the dep bridge). And it would be the cleanest solution to all this imho. Even more eventually we can go back to the subproject scaning but that's a gradle anti-practice

@jpenilla I would love your thoughts on this

@Toffikk Toffikk force-pushed the feat/ats-for-upstream-sources branch from a11ba7c to 752b4dc Compare April 25, 2026 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants