GDLauncher icon indicating copy to clipboard operation
GDLauncher copied to clipboard

Allow for Local ARM64 Linux Building

Open theofficialgman opened this issue 1 year ago • 2 comments

Purpose

Allow for building GDLauncher on ARM64 Linux and use custom arm64 mojang meta repo https://github.com/theofficialgman/piston-meta-arm64

Approach

Makes necessary changes to the build infrastructure where necessay and provides arm64 binaires in the repo where x64 ones exist (note about the binaries, I did not build them and do not know how to do so, they were built by @nicholasaiello) .

build commands used (with nodejs 16 and npm 8)

npm i --legacy-peer-deps
npm run release:setup

If this PR https://github.com/gorilla-devs/GDLauncher/pull/1446 gets merged, then this PR will conflict. I will rework this PR where necessary if that gets merged.

Learning

issue https://github.com/gorilla-devs/GDLauncher/issues/727

Notes

This PR is functional for ARM64 linux (builds produced on arm64 bionic here: https://github.com/Pi-Apps-Coders/files/releases/tag/large-files ) but will remain as a draft until a better solution for replacing the MC_MANIFEST_URL is made, ideally through a build argument instead of source modification.

System FPM is necessary for building on anything other than x64, so it has been defined in the .env. The easiest way to install system FPM is sudo apt-get install ruby-dev build-essential -y && sudo gem i fpm -f

Incorrect and duplicate options have been removed in the .env so that gdlauncher can be built.

~~Finally, the user is expected to provide their own java. It would be nice if arm64 linux java binaries could be added and the code modified to download them~~ edit: looks like GDLauncher provides its own temurin json files for java 8 and 17 so the info is here. However it was hardcoded to download x86_64 only and didn't do runtime detection... that has also been fixed in this PR

theofficialgman avatar Sep 07 '22 16:09 theofficialgman

no code changes besides removing my custom manifest

if the plan is to merge https://github.com/gorilla-devs/GDLauncher/pull/1446 then this will need to be reworked slightly (it actually would make this a little simpler)

theofficialgman avatar Sep 08 '22 23:09 theofficialgman

I have rebased on another branch on top of @Eskaan 's changes in another branch. They greatly simplify this PR, I would like those changes first. I will push those changes to this branch once their PR merges

theofficialgman avatar Sep 18 '22 14:09 theofficialgman

@theofficialgman I know it was some time, and I now took over some of the maintenance myself. I guess you'll be fairly happy seeing #1446 merged (20148d6bfa1178bee65ba9c93591ffc8fb082062) I am really seeing forward to this! Guess it's time to see what your code did back then and go rebase it ;)

Eskaan avatar Jan 16 '23 21:01 Eskaan

@Eskaan luckily I had already done the rebase, so just did a couple of quick edits.

I can drop the java memory commit if you want. I was running on systems with less than 4GB of ram and if the user doesn't know to change it then it just refuses to start java

theofficialgman avatar Jan 16 '23 21:01 theofficialgman

I guess most users that run on a low memory system will know that they have to decrease their java memory usage. It is a sensible default as most systems nowadays have at least 8G.

Eskaan avatar Jan 16 '23 21:01 Eskaan

alright then, I have dropped it

theofficialgman avatar Jan 16 '23 21:01 theofficialgman

...so he removed the commit anyways. Whatever. Continuing my review.

Eskaan avatar Jan 16 '23 21:01 Eskaan

LGTM. Maybe you want to fiddle with a solution to run the linux CI for ARM too in some future pr? Although the github sctions runner does not support arm64 natively, there are some actions for it that simulate arm64.

possibly though not right now I am still recovering from getting QEMU powered docker container runners in MuseScore's github actions.

plus that wouldn't be any use unless you also accept my meta repo changes for arm64/arm32. https://github.com/theofficialgman/GDLauncher/commit/bb5c7e2ccdefaf680df4ac7215164add50a1d833 those changes would need to be done on a per architecture basis

theofficialgman avatar Jan 16 '23 21:01 theofficialgman

So, does this code work on ARM without the metadata changes? Edit: It surely compiles, but would it work with the current metadata?

Eskaan avatar Jan 16 '23 21:01 Eskaan

So, does this code work on ARM without the metadata changes? Edit: It surely compiles, but would it work with the current metadata?

it compiles and the launcher works. minecraft itself will not. hence the name of this PR: Allow for Local ARM64 Linux Building. it is made to allow someone to build and have the launcher itself work.

theofficialgman avatar Jan 16 '23 21:01 theofficialgman

Yeah, I think I will tell @killpowa to include those files in the web backend and link them later. Thanks for your efforts in making the launcher ARM compatible!

Eskaan avatar Jan 16 '23 21:01 Eskaan