Fix compatibility with current Pumpkin nightlies#18
Conversation
|
This actually has a lot of changes that should be implemented. Maybe not everything should, but implementation instead of compileOnly is definitely a good idea. Also having protoc find the vendor is a good idea. And making the build script run cargo update is a smart idea. Some of the other changes do seem to be unneeded. |
|
I gave codex the instruction to directly use and test patchbukkit with an pumpkin server, and test changes directly. Additionally I told him to downlod a test plugin from spigot to directly test changes. What would be things you don't like from the changes? |
| format!("{PATCHBUKKIT_PERMISSION_NAMESPACE}:command.{plugin_segment}.{command_segment}") | ||
| } | ||
|
|
||
| fn sanitize_permission_segment(value: &str) -> String { |
There was a problem hiding this comment.
Why are we sanitizing permission segments? Is this something paper does?
| "Allows running the Bukkit command `{cmd_name}` from `{}`", | ||
| plugin.name | ||
| ), | ||
| PermissionDefault::Allow, |
There was a problem hiding this comment.
Seems unsafe to default to allow.
| }) | ||
| } | ||
|
|
||
| pub fn setup_pumpkin_build_info(base: &Path) { |
There was a problem hiding this comment.
I don't understand why this exists. And if it is for a good reason why not just use a toml parsing crate.
|
Any more corrections? |
|
Looks good to me. @yunuservices can you review and merge this? |
|
Lemme look rq |
yunuservices
left a comment
There was a problem hiding this comment.
- used protobuf dependencies a lot than expected. (compile-time only)
- Fat JAR usage. (acceptable)
Overall this PR looks good, approved.
Note
Me (rotstein007) discovered, that PatchBukkit currently doesn't work. So I quickly made an fix with codex. I didn't read through the code, so please be careful with merging. Hopefully it's still helpful. The past section is also generated by codex:
Summary
This updates PatchBukkit so it works again with current Pumpkin nightlies.
What changed
patchbukkit.jar, so the JVM can startreliably without resolving Paper/Bukkit classes at runtime
protocto make local builds and CI more reliablepatchbukkit/folder againResult
PatchBukkit starts successfully on current Pumpkin nightlies again.
I also verified this with a real Spigot plugin:
simple_spawn-1.3.jarloads successfullyVerification
Tested locally with:
0.1.0-dev+26.1)simple_spawn-1.3.jar)Additional checks:
cargo +nightly-2026-03-05 testcargo +nightly-2026-03-05 clippy --all-targets -- -D warningscargo +nightly-2026-03-05 build --releaseKnown limitation
Pumpkin still logs the native plugin as
Loaded ().PatchBukkit itself works in the tested scenario, but that remaining metadata issue appears to be on the
Pumpkin native plugin loader / ABI side rather than in PatchBukkit's startup path.