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

Add better support for April Fools snapshots #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SizableShrimp
Copy link
Contributor

The biggest addition is adding 2 new fields to BranchSpec to make it easier to define April Fools snapshots from a config json. The demo branch_config.json file in the repository is not really useful, as it assumes that all April Fools versions should be diffed from each other. This makes no sense since each snapshot is forked from the snapshot cycle for that year.

Instead, I propose adding 2 fields: includeVersions and excludeVersions

  • includeVersions basically has the sole purpose of opting in to specific "special" versions as categorized by SRGUtils, but it's primarily meant for use w/ April Fools snapshots. It doesn't actually add any extra versions, but I'm not sure how to better name it to reflect this.
  • excludeVersions lets you exclude specific versions from the list of versions, rather than having to define all versions you want use. This allows us to have April Fools snapshots with the full diff history of all snapshots/versions before it, while accounting for the fact that the April Fools snapshot may have been forked from an earlier snapshot in the dev cycle than the snapshot released right before the April Fools one.
    • For example, this year's (2024) April Fools snapshot appears to be based on 24w12a according to coehlrich, but 24w13a also exists. Because SRGUtils orders the April Fools snapshots based on their year and week and not based on where it actually is forked from the dev cycle, it means that by default, Snowblower will try to diff from 24w12a -> 24w13a -> 24w14potato. This will cause the 24w14potato commit to be polluted by undoing all of the things added in 24w13a.

Besides the main addition, I also fixed/added some miscellaneous things:

  • Added settings.gradle file with the foojay resolver convention plugin for JDK resolution. This just makes it easier to import into an IDE if anyone does that for better inspection (like I did while testing my Snowblower changes). In this rare case, the JDK was bumped to 21 in the latest snapshot, and I didn't have it downloaded, so Gradle failed to import since the resolver plugin wasn't specified.
  • Fixed a bug when calculating the end version to generate to, if it is omitted from the CLI / config. See relevant discord messages here.
  • Changed the initial commit generation code to specifically copy gradle/wrapper instead of gradle, to account for the fact that this project might use libs.versions.toml (or some other file in gradle but not gradle/wrapper?) in the future, which shouldn't be included in the Snowblower jar nor the final output
  • Clarified the error messages for when the start/end version do not exist in the calculated version list
  • Fixed parsing of --cfg parameter on the CLI to actually support file: URIs
  • Update Gradle to 8.7

Here's a demo on how to test with the 2024 April Fools snapshot using this config.json:

  • We can generate the full history starting from 1.14.4 up until the 2024 April Fools snapshot using this command:
    --output output --cfg https://gist.githubusercontent.com/SizableShrimp/ab8a7882a91978bbd8eb96f2a467f2bc/raw/b8e5f4ed5a61f71b51c66f7094468d111319aa41/config.json --branch april-fools/2024 --start-over-if-required
  • For the purposes of easy testing, you can specify the start version as 24w12a to only have to decompile 2 MC versions:
    --output output --cfg https://gist.githubusercontent.com/SizableShrimp/ab8a7882a91978bbd8eb96f2a467f2bc/raw/b8e5f4ed5a61f71b51c66f7094468d111319aa41/config.json --branch april-fools/2024 --start-over-if-required --start-ver 24w12a

Assuming that we do go ahead with this idea, I would suggest we maintain a config.json URL somewhere. Every time a new April Fools snapshot comes out, we can add on to this config.json. Then, we can change the TeamCity configuration to specify this config json in every run. We would have a template (or whatever the TC equivalent is) that lets us do one-off builds running the standard Snowblower template and has a parameter to specify the branch name. This template thing would be run after having added the relevant configuration for that branch to the config.json. The branch name would follow some consistent format for each April Fools snapshot. I propose april-fools/XXXX where XXXX is the year.

We could also retroactively do this for every older April Fools snapshot that has mojmaps just for fun to be able to poke at the code whenever.

This acts as an addition/subtraction combo that is only used if the versions field is not set. Useful for April Fools snapshots.
Add settings.gradle for Gradle JDK resolution. Fixes case when user does not have the necessary JDK downloaded (like me not having J21 for 24w14a) and tries to import with Gradle.
Fix bug calculating target version if it is omitted
Change initial commit generation to only copy gradle/wrapper instead of gradle to account for a potential future use of libs.versions.toml by excluding it
Fix parsing of --cfg parameter when provided URI uses the file scheme
Update Gradle to 8.7
@neoforged-pr-publishing
Copy link

neoforged-pr-publishing bot commented Apr 6, 2024

  • Publish PR to GitHub Packages

Last commit published: d85c7b73af8943a66953180decf31fe6f17b16e8.

PR Publishing

The artifacts published by this PR:

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #4' // https://github.com/neoforged/snowblower/pull/4
        url 'https://prmaven.neoforged.net/snowblower/pr4'
        content {
            includeModule('net.neoforged', 'snowblower')
        }
    }
}

@sciwhiz12 sciwhiz12 added the enhancement New feature or request label Apr 6, 2024
Comment on lines +28 to +30
List<MinecraftVersion> versions,
List<MinecraftVersion> includeVersions,
List<MinecraftVersion> excludeVersions
Copy link
Member

Choose a reason for hiding this comment

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

We really should work on adding proper @Nullable annotations everywhere, but alas that would mean also adding @NotNull annotations because JetBrains' annotations have no non-null-by-default annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not even sure what is nullable vs. not in the project. Do you want me to go through and try to add that to everything? I think notnull is a bit excessive if we document everything explicitly nullable.

Choose a reason for hiding this comment

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

Just use of @Nullable is fine imo, @NotNull is noise and would be littered on virtually everything

Copy link
Member

Choose a reason for hiding this comment

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

I think, for the sake of keeping this PR's scope limited, @Nullable should be added to these components, so we can gradually start marking things as @Nullable (with the intention that anything that isn't annotated should be taken as @NotNull, at least by developers/reviewers).

Choose a reason for hiding this comment

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

Yeah that's fine. We don't need to attempt a full-scale @Nullable rollout for this PR, just for the components it adds/changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants