Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gradle cleanup #645

Open
wants to merge 98 commits into
base: 1.21.4-pre1
Choose a base branch
from

Conversation

supersaiyansubtlety
Copy link
Contributor

@supersaiyansubtlety supersaiyansubtlety commented Sep 28, 2024

Ready!

Tasks

  • the thisening
  • the finalening
  • the line wrappening
  • the abstractening
  • the registerening
  • move all custom tasks to plugins
  • move (most) task configuration to plugins (makes it easier to see dependencies and makes tasks more reusable)
  • eliminate buildSrc referencing build.gradle things
  • eliminate project access in task actions
  • refine inputs/outputs and dependencies
  • move more things to QuiltMappingsExtension
  • separate QuiltMappingsPlugin's contents into several plugins
  • wait for add a test to make sure broken unpick definitions error #659 which fixes tests, make sure tests still pass
  • resolve a few remaining questions
  • replace TasksTest and depend on gradle test kit maybe in a followup PR
  • apply a checkstyle to buildSrc? maybe in a followup PR

Summary

renames MappingsPlugin/mappings-logic -> QuiltMappingsPlugin/quilt-mappings
renames MappingssExtension/mappings -> QuiltMappingsExtension/quiltMappings
splits QuiltMappingsPlugin into:

  • QuiltMappingsBasePlugin
  • MinecraftJarsPlugin
  • MapMinecraftJarsPlugin
  • MapV2Plugin
  • MapIntermediaryPlugin
  • ProcessMappingsPlugin
  • EnigmaMappingsPlugin
  • TargetDiffPlugin

all implement MappingsProjectPlugin and are applied by QuiltMappingsPlugin

eliminates FileConstants:

  • some common directories are in MappingsProjectPlugin
  • files that were the output of one task are now defined where that task's output is set and other tasks take that output as input

adds several extensions that expose objects created by their plugins
extensions are accessed via plugin instances which ensures extension have been created

no use of tasks.named(...) to access custom tasks:

  • instead they're accessed via the provider returned by tasks.register(...)
  • tasks used outside of the plugin they're defined in are accessed via Tasks objects obtained via extensions, obtained via plugin instances
  • this makes sure required plugins are applied and the tasks are registered

ensures all custom tasks have a group
eliminates MappingsTask and DefaultMappingsTask
(some MappingsTask methods are replaced with helper methods in more specific task interfaces and plugins)

moves Constants.MINECRAFT_VERSION to libs.versions.toml and passes it to QuiltMappingsExtension
moves Constants.MAPPINGS_VERSION to direct project.version assignment in build.gradle and passes it to QuiltMappingsExtension
moves dictionary file url to libs.versions.toml and downloads it with an ivy repository hack
eliminates downloadDictionaryFile and passes the dictionary file to mappingLint in build.gradle
saves unpickVersion in a QuiltMappingsEntension field and passes it to the constructors of tasks that require it
(it's required at configuration time for some tasks so it can't be a property)

extracts a bunch of magic strings to interfaces in constants package
moves all task names to their classes and renames all TASK_NAME fields <ACTUAL_NAME>_TASK_NAME
adds javadocs to all task names that link the plugin that registers them
adds javadocs to each plugin with any configurEaches listing what they do (these can be annoying to track down)
adds javadocs to each class that's configureEached and to all their subclasses, linking the plugin that does the configuring (these can be really annoying to track down)

moves almost all outputs to build/*
moves combineUnpickDefinitions's output to build/mappings/
eliminates custom clean task logic (everything is in build/ and gets cleaned)

names AbstractArchiveTask outputs using its name composition methods
adds ArtifactFileProducingTask which mimics AbstractArchiveTask name composition (implemented by CompressTinyTask)
eliminates explicit classifiers for published artifacts in build.gralde

mapped providers are the solution to every problem

moves unpick.json to unpick/ and unpick definitions to unpick/definitions/
moves enimga_profile.json and simple_type_field_names.json5 to enigma/

eliminates decompileClasspath configuration -> passes downloadMinecraftLibraries.librariesDir.fileTree
replaces CheckStuffExists/Matches tasks with mapped providers

populates unpickCli configuration using included build :unpick-holders
splits enigmaRuntime configuration into enigmaSwing and enigmaServer, populates them using :unpick-holders

replaces de.undercouch.gradle.tasks.download.DownloadAction usage with DownloadUtil methods (which use org.apache.commons.io.FileUtils::copyURLToFile)
eliminates de.undercouch:gradle-download-task dependency

adds generateDiff task (Windows users may have to put diff on their PATH)

Future

  • use generateDiff in gerenate-diff.yml (pass output path to avoid magic string)
  • move namedSource/ and namedTargetSource/once generateDiff is used in gerenate-diff.yml
  • update/replace licenser plugin: the last release is from 2021 and it's incompatible with gradle 9
  • @CachableTask everything
  • configuration cache

@supersaiyansubtlety supersaiyansubtlety added enhancement new feature or request t: toolchain changes to the quilt mappings toolchain outdated this pull request hasn't been updated to the latest version or has merge conflicts wip this is a work in progress labels Sep 28, 2024
@supersaiyansubtlety supersaiyansubtlety self-assigned this Sep 28, 2024
@supersaiyansubtlety
Copy link
Contributor Author

supersaiyansubtlety commented Sep 28, 2024

Early reviewers beware: I will probably be rewriting history so any review progress on github may be lost.
Ready -ish

@ix0rai ix0rai added the update-base used to notify github actions that the base branch should be updated label Oct 1, 2024
Copy link
Contributor

github-actions bot commented Oct 1, 2024

🚨 Target branch is already set to 24w39a

@github-actions github-actions bot removed the update-base used to notify github actions that the base branch should be updated label Oct 1, 2024
@ix0rai ix0rai added the update-base used to notify github actions that the base branch should be updated label Oct 3, 2024
@github-actions github-actions bot changed the base branch from 24w39a to 24w40a October 3, 2024 00:36
Copy link
Contributor

github-actions bot commented Oct 3, 2024

🚀 Target branch has been updated to 24w40a

Copy link
Contributor

github-actions bot commented Oct 3, 2024

🚨 Please fix merge conflicts before this can be merged

@github-actions github-actions bot added v: snapshot targets a snapshot version of minecraft and removed update-base used to notify github actions that the base branch should be updated labels Oct 3, 2024
Copy link

@lukebemish lukebemish left a comment

Choose a reason for hiding this comment

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

Looks pretty good! Unsure if the eventual aim is config cache here (and maybe it ought to be, as config cache will be on by default in gradle 9 which is on the horizon) -- regardless, if so there's a bit of work to do, since a lot of stuff is still kinda unidiomatic in that regard; all things told though a drastic improvement. Didn't comment on any remaining TODOs.

@supersaiyansubtlety supersaiyansubtlety removed the outdated this pull request hasn't been updated to the latest version or has merge conflicts label Oct 4, 2024
@supersaiyansubtlety
Copy link
Contributor Author

supersaiyansubtlety commented Oct 4, 2024

Note: Rather than marking this PR as ready for review, when it's ready I'll be closing it and creating a new one.
That will clean up the messy history here on GitHub caused by rewriting history.
Never mind, it cleaned itself up, I underestimated GitHub.

@supersaiyansubtlety supersaiyansubtlety added the s: large PRs with more than 700 lines label Oct 4, 2024
@supersaiyansubtlety supersaiyansubtlety force-pushed the procrastination+gradle-cleanup branch 2 times, most recently from 3e74240 to c6f2f85 Compare October 8, 2024 05:26
@ix0rai ix0rai added the update-base used to notify github actions that the base branch should be updated label Oct 9, 2024
Copy link
Contributor

github-actions bot commented Oct 9, 2024

🚀 Target branch has been updated to 1.21.2-pre1

@github-actions github-actions bot changed the base branch from 24w40a to 1.21.2-pre1 October 9, 2024 00:46
@github-actions github-actions bot removed the update-base used to notify github actions that the base branch should be updated label Oct 9, 2024
@ix0rai ix0rai added the update-base used to notify github actions that the base branch should be updated label Oct 10, 2024
@github-actions github-actions bot changed the base branch from 1.21.2-pre1 to 1.21.2-pre2 October 10, 2024 16:53
Copy link
Contributor

🚀 Target branch has been updated to 1.21.2-pre2

…ectPlugin with DefaultExtensionedMappingsProjectPlugin + Default/TaskedExtension

access Tasks objects and MapIntermediaryPlugin's intermediaryFile via extension instances obtained via plugin instances instead of directly via plugin instances
Copy link
Member

@OroArmor OroArmor left a comment

Choose a reason for hiding this comment

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

Honestly too big to properly review now. From my previous looks its going great. I trust that you did it correctly

- remove #getArtifact method as the classifier got stripped when publishing
- make #artifact method add the classifier
… from DownloadWantedVersionManifestTask#provideVersionParser

use netty version from libs.version.toml because minecraft's netty version doesn't have javadocs
@supersaiyansubtlety supersaiyansubtlety removed the wip this is a work in progress label Nov 14, 2024
@ix0rai ix0rai added the update-base used to notify github actions that the base branch should be updated label Nov 19, 2024
Copy link
Contributor

🚀 Target branch has been updated to 24w46a

@github-actions github-actions bot changed the base branch from 24w45a to 24w46a November 19, 2024 01:02
Copy link
Contributor

🚨 Please fix merge conflicts before this can be merged

@github-actions github-actions bot added outdated this pull request hasn't been updated to the latest version or has merge conflicts and removed update-base used to notify github actions that the base branch should be updated labels Nov 19, 2024
@supersaiyansubtlety supersaiyansubtlety removed the outdated this pull request hasn't been updated to the latest version or has merge conflicts label Nov 19, 2024
…raftJarsTask, and DownloadMinecraftLibrariesTask

closes QuiltMC#492
@supersaiyansubtlety supersaiyansubtlety linked an issue Nov 20, 2024 that may be closed by this pull request
@supersaiyansubtlety supersaiyansubtlety added the update-base used to notify github actions that the base branch should be updated label Nov 21, 2024
@github-actions github-actions bot changed the base branch from 24w46a to 1.21.4-pre1 November 21, 2024 02:25
Copy link
Contributor

🚀 Target branch has been updated to 1.21.4-pre1

Copy link
Contributor

🚨 Please fix merge conflicts before this can be merged

@github-actions github-actions bot added outdated this pull request hasn't been updated to the latest version or has merge conflicts and removed update-base used to notify github actions that the base branch should be updated labels Nov 21, 2024
@supersaiyansubtlety supersaiyansubtlety removed the outdated this pull request hasn't been updated to the latest version or has merge conflicts label Nov 21, 2024
@ix0rai ix0rai added the final-comment-period is approved and will soon be merged if no issues are raised label Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new feature or request final-comment-period is approved and will soon be merged if no issues are raised s: large PRs with more than 700 lines t: toolchain changes to the quilt mappings toolchain v: snapshot targets a snapshot version of minecraft
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use gradle home cache for Minecraft libraries
4 participants