Skip to content

Allow for declaring an oldForkCommit to bring in objects from the active fork for 3 way apply.#386

Open
Toffikk wants to merge 3 commits intoPaperMC:mainfrom
CraftCanvasMC:feat/old-fork-commit-fetching
Open

Allow for declaring an oldForkCommit to bring in objects from the active fork for 3 way apply.#386
Toffikk wants to merge 3 commits intoPaperMC:mainfrom
CraftCanvasMC:feat/old-fork-commit-fetching

Conversation

@Toffikk
Copy link
Copy Markdown
Contributor

@Toffikk Toffikk commented Apr 24, 2026

This PR extends the updatingMinecraft {} extension by introducing a new oldForkCommit property. When configured, oldForkCommit behaves similarly to oldPaperCommit, but it targets the active fork defined in the build script rather than Paper.

The main motivation for this change is that the existing oldPaperCommit mechanism often doesn't bring in enough objects, particularly in nested fork chains (e.g., fork-of-a-fork scenarios), but sometimes in simple Paper forks too, which causes 3-way patch application to fail.

It also enables patch roulette for minecraft source patches for forks, so the ones that host their own instance of it can also have some fun with it and use it to ease their updates.

Toffikk added 3 commits April 24, 2026 21:17
…ive fork for 3 way apply.

This PR extends the updatingMinecraft {} extension by introducing a new oldForkCommit property.
When configured, oldForkCommit behaves similarly to oldPaperCommit, but it targets the active fork defined in the build script rather than Paper.

The main motivation for this change is that the existing oldPaperCommit mechanism often doesn't bring in enough objects,
particularly in nested fork chains (e.g., fork-of-a-fork scenarios), but sometimes in simple Paper forks too, which causes 3-way patch application to fail.

It also enables patch roulette for minecraft source patches for forks, so the ones that host their own instance of it can also have some fun with it and use it to ease their updates.

PatchRouletteTasks(
target,
coreExt.activeFork.get().name.lowercase(), // TODO: make this lazy
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this call get() here to get the actual fork name, or should we default to fork?
I assume this get() call is cheap enough for it to be okay

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.

1 participant