Skip to content

Comments

Bukkit Build Fix#494

Merged
duplexsystem merged 5 commits intoPolyhedralDev:dev/7.0-2from
Ifiht:dev/7.0-2
Mar 22, 2025
Merged

Bukkit Build Fix#494
duplexsystem merged 5 commits intoPolyhedralDev:dev/7.0-2from
Ifiht:dev/7.0-2

Conversation

@Ifiht
Copy link
Contributor

@Ifiht Ifiht commented Mar 19, 2025

Pull Request

Description

The build was not working for 1.21.4, looks like a combination of things, first the paperBuild version was outdated, then paperweight was complaining about:

 Expected configuration ':platforms:bukkit:nms:v1_21_3:paperweightDevelopmentBundle' to contain exactly one file, however, it contains more than one file.

which according to the paper devs requires updating gradle to 8.13 and paperweight to the latest 2.0.0-beta (thank you hard fork..), however this breaks architectury so that then also needed upgraded. Details below.

Changelog

  1. Upgrade paperBuild version to latest 2025 release for 1.21.4 (fixes first build error)
  2. Remove gradle shasum from gradle-wrapper.properties (prevents upgrading gradle)
  3. Upgrade gradle to 8.12, and paperweight to 2.0.0-beta (fixes second build error)
  4. Upgrade architectury to 1.9.428 beta release, fixes build error caused by upgrading gradle from 10 -> 12
  5. Fix paperDevBundle configuration in Bukkit gradle build file (final build error gone!!!)

Checklist

Mandatory checks

  • The base branch of this PR is an unreleased version branch (that has a ver/ prefix)
    or is a branch that is intended to be merged into a version branch.
  • There are no already existing PRs that provide the same changes.
  • The PR is within the scope of Terra (i.e. is something a configurable terrain generator should be doing).
  • Changes follow the code style for this project.
  • I have read the CONTRIBUTING.md
    document in the root of the git repository.

Types of changes

  • Bug Fix
  • Build system
  • Documentation
  • New Feature
  • Performance
  • Refactoring
  • Repository
  • Revert
  • Style
  • Tests
  • Translation

Compatibility

  • Introduces a breaking change
  • Introduces new functionality in a backwards compatible way.
  • Introduces bug fixes

Documentation

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Testing

  • I have added tests to cover my changes.
  • All new and existing tests passed.

Licensing

  • I am the original author of this code, and I am willing to
    release it under GPLv3.
  • I am not the original author of this code, but it is in public domain or
    released under GPLv3 or a compatible license.

distributionPath=wrapper/dists
distributionSha256Sum=31c55713e40233a8303827ceb42ca48a47267a0ad4bab9177123121e71524c26
distributionUrl=https\://services.gradle.org/distributions/gradle-8.10.2-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.12-bin.zip
Copy link
Member

Choose a reason for hiding this comment

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

why was the wrapper & whatnot not updated? did you update gradle using the wrapper task?

Copy link
Contributor Author

@Ifiht Ifiht Mar 20, 2025

Choose a reason for hiding this comment

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

Sorry haven't done many gradle upgrades, I used ./gradlew wrapper --gradle-version 8.12 and my gradle version when executing ./gradlew --version shows 8.12? Was there something I missed?

Copy link
Member

Choose a reason for hiding this comment

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

hmm, my bad, I assumed that the wrapper jar had updated between 8.10.2 and 8.12. it seems that it has not.

is there a reason that 8.12 was used instead of 8.12.1 or 8.13?

also, I personally have a bash script called upgrade-gradle-wrapper which does the following

#!/bin/bash

WRAPPER_VERSION="$1"

WRAPPER_HASH=$(curl "https://services.gradle.org/distributions/gradle-$WRAPPER_VERSION-bin.zip.sha256" -L)

gradle wrapper --gradle-version="$WRAPPER_VERSION" --gradle-distribution-sha256-sum="$WRAPPER_HASH"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the script, I was wondering how to check that hash. The reason for 8.12 was that Architectury devs recommend 8.11, while Paper devs recommend 8.13, and praise Notch they both allowed their libs to work with 8.12, which is currently the only thing I can find that works with both. I didn't try the minor version 8.12.1, I literally worked my way up from 8.10.2 that we were on until I had a working gradle, then stopped. I can give 8.12.1 a shot if you think it matters?

Copy link
Member

Choose a reason for hiding this comment

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

give 8.13, a try. it should work, unless the architectury devs did smth incredibly dumb

Copy link
Contributor Author

@Ifiht Ifiht Mar 21, 2025

Choose a reason for hiding this comment

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

Sorry, should have mentioned I tried that one first & it doesn't work (due to architectury).. Only ones I didn't try were minor versions of 8.12
Screenshot_20250321-070542
^ this is why I had to use beta architectury (from their discord)

Copy link
Member

Choose a reason for hiding this comment

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

sigh. wtf is architectury doing that is breaking it.

const val paperDevBundle = paperBuild
const val runPaper = "2.3.1"
const val paperWeight = "1.7.2"
const val paperWeight = "2.0.0-beta.16"
Copy link
Member

Choose a reason for hiding this comment

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

we should probably wait for a non-beta release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I get that design decision, however from the Paper devs we're not going to be able to target 1.21.4 with paperDevBundle unless we're on this version and at least gradle 12.

Since bukkit wasn't building, and this isn't going to release yet I thought maybe that could justify using beta for paperDevBundle and Architectury, but if not I'll just keep this in my local fork to build & test for Paper.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I get that design decision, however from the Paper devs we're not going to be able to target 1.21.4 with paperDevBundle unless we're on this version and at least gradle 12.

Since bukkit wasn't building, and this isn't going to release yet I thought maybe that could justify using beta for paperDevBundle and Architectury, but if not I'll just keep this in my local fork to build & test for Paper.

Can you send the message here?
having to go to a discord server that I'm not in, requiring me to leave a separate discord server (because I'm at server limit), is incredibly inconvenient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You maxed out servers without adding PaperMC?? But here's jmp, other devs mention the specific version numbers if you want since this issue has been showing up for a lot of ppl/projects, I just consider jmp god of Paper issues so for me this is the authoritative msg:

image

@solonovamax
Copy link
Member

before merge, you should also squash all those commits into a single one

@Ifiht
Copy link
Contributor Author

Ifiht commented Mar 22, 2025

Rather than me adding upstreams and doing an interactive rebase against the PolyDev remote followed by a force push, couldn't we just squash and merge here when this gets approved?

I've already sunk a lot of time into this PR, kinda just want to get on with my testing.

@duplexsystem duplexsystem merged commit 52dc690 into PolyhedralDev:dev/7.0-2 Mar 22, 2025
1 check passed
duplexsystem pushed a commit that referenced this pull request Mar 25, 2025
* Bukkit Build Fix

* remove comments

* remove papermc repo from gradle settings

* add back gradle shasum

* fix formatting, update gradle hash
duplexsystem added a commit that referenced this pull request Jun 4, 2025
* Bukkit Build Fix (#494)

* Bukkit Build Fix

* remove comments

* remove papermc repo from gradle settings

* add back gradle shasum

* fix formatting, update gradle hash

* Initial Fabric 1.21.5

* Updated dependencies

* Updated SpawnerData with backwards compat

* Updated dependencies

* Updated setBlockState usage - needs verifying as flags are confusing

* Refactored Bukkit NMS packages

* Initial attempt at updating mixin-commons

* Continue fabric 1.21.5 WIP

* Some additional logging

* Update deps

* Build fixes and update allay

* Add oak to authors

---------

Co-authored-by: Mikal <Ifiht@users.noreply.github.com>
Co-authored-by: OakLoaf <oak@beaconstudios.org>
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