Skip to content

add a flag to control the hash checking#43

Merged
shartte merged 2 commits intoneoforged:mainfrom
Trial97:main
May 6, 2025
Merged

add a flag to control the hash checking#43
shartte merged 2 commits intoneoforged:mainfrom
Trial97:main

Conversation

@Trial97
Copy link
Contributor

@Trial97 Trial97 commented May 4, 2025

Address the compatibility issue with zlib-ng-compat, which is now shipped in place of zlib in various Linux distros. This change introduces an option to disable hash mismatches reported on processors.
I plan to integrate it with the ForgeWrapper.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@shartte
Copy link
Contributor

shartte commented May 4, 2025

How realistic is shipping zlib-ng?

@Trial97
Copy link
Contributor Author

Trial97 commented May 4, 2025

Right now, I know that cachyos defaults to zlib-ng instead of baseline zlib.
Other than that, I do not know of other distributions that do that, but I would expect either the installer to behave the same regardless of the system libraries or have a way to bypass the check.

@shartte
Copy link
Contributor

shartte commented May 4, 2025

Right now, I know that cachyos defaults to zlib-ng instead of baseline zlib. Other than that, I do not know of other distributions that do that, but I would expect either the installer to behave the same regardless of the system libraries or have a way to bypass the check.

I actually mean via JNI bindings we ship ourselves to use the same zlib everywhere.

note: It is however true that we should not necessarily rely on reproducible compression to begin with, but I wonder if disabling the hash check is actually to our benefit.

@Trial97
Copy link
Contributor Author

Trial97 commented May 4, 2025

I want to have the users who encounter this issue have an alternative to installing the loader.
Yeah maybe it is not the best alternative, maybe there is a way to set some settings for compression that would ensure reproducibility on all cases. But that part is to advanced for me.
Here is the original discussion on Neoforge Discord: https://discord.com/channels/313125603924639766/915304642668290119/1346007687221219369

@shartte
Copy link
Contributor

shartte commented May 4, 2025

I want to have the users who encounter this issue have an alternative to installing the loader. Yeah maybe it is not the best alternative, maybe there is a way to set some settings for compression that would ensure reproducibility on all cases. But that part is to advanced for me. Here is the original discussion on Neoforge Discord: https://discord.com/channels/313125603924639766/915304642668290119/1346007687221219369

While doing a reference check might work I am worried that zlib-ng only compresses differently in some, but not all cases. Then a reference check might look like the zlib implementation used by the JDK is what we expect, but it still produces differences in some cases.

Regarding your change: I'd not merge this flag as-is, instead I'd want the flag to continue to print that there was a hash mismatch, but simply not delete the resulting file (and continue).

Signed-off-by: Trial97 <alexandru.tripon97@gmail.com>
@shartte
Copy link
Contributor

shartte commented May 4, 2025

I'll try to take a closer look tomorrow

@shartte shartte merged commit c984a90 into neoforged:main May 6, 2025
2 checks passed
@neoforged-releases
Copy link

🚀 This PR has been released as LegacyInstaller version 3.0.39.

@Trial97
Copy link
Contributor Author

Trial97 commented May 7, 2025

Thanks for merging this.
Are there any additional steps for the newer version to use this installer version?
And I presume there will be no regeneration of old files, right?

@shartte
Copy link
Contributor

shartte commented May 7, 2025

Thanks for merging this. Are there any additional steps for the newer version to use this installer version? And I presume there will be no regeneration of old files, right?

Usually not, no. But NF needs to bump the installer version to make use of this (gradle.properties over there).

edit: we have backported a shit ton to older versions so it's possible to backport this too, actually.

@SolsticeClouds
Copy link

Hi, so sorry to bother but is there any chance this could be backported to 47.1.106 or similar? i want to play a 1.20.1 modpack but i'm having this issue with the hash check due to having to use an older version of NeoForge.

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