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

Allow for Local ARM64 Linux Building #1451

Merged
merged 4 commits into from
Jan 16, 2023

Conversation

theofficialgman
Copy link
Contributor

@theofficialgman theofficialgman commented Sep 7, 2022

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 #1446 gets merged, then this PR will conflict. I will rework this PR where necessary if that gets merged.

Learning

issue #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 theofficialgman marked this pull request as ready for review September 8, 2022 23:54
@theofficialgman
Copy link
Contributor Author

theofficialgman commented Sep 8, 2022

no code changes besides removing my custom manifest

if the plan is to merge #1446 then this will need to be reworked slightly (it actually would make this a little simpler)

@Eskaan Eskaan added Priority: High Status: On Hold Waiting for something else... labels Sep 9, 2022
@Eskaan Eskaan linked an issue Sep 9, 2022 that may be closed by this pull request
@theofficialgman
Copy link
Contributor Author

theofficialgman commented Sep 18, 2022

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

@Eskaan
Copy link
Collaborator

Eskaan commented Jan 16, 2023

@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 (20148d6)
I am really seeing forward to this!
Guess it's time to see what your code did back then and go rebase it ;)

@theofficialgman
Copy link
Contributor Author

@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

@Eskaan
Copy link
Collaborator

Eskaan commented Jan 16, 2023

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.

@theofficialgman
Copy link
Contributor Author

alright then, I have dropped it

@Eskaan
Copy link
Collaborator

Eskaan commented Jan 16, 2023

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

Copy link
Collaborator

@Eskaan Eskaan left a comment

Choose a reason for hiding this comment

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

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.

@Eskaan Eskaan added Status: Completed and removed Status: On Hold Waiting for something else... labels Jan 16, 2023
@theofficialgman
Copy link
Contributor Author

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. theofficialgman@bb5c7e2
those changes would need to be done on a per architecture basis

@Eskaan
Copy link
Collaborator

Eskaan commented Jan 16, 2023

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

@theofficialgman
Copy link
Contributor Author

theofficialgman commented Jan 16, 2023

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.

@Eskaan
Copy link
Collaborator

Eskaan commented Jan 16, 2023

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 Eskaan merged commit 4fd9a47 into gorilla-devs:master Jan 16, 2023
@Eskaan Eskaan mentioned this pull request Jan 17, 2023
Eskaan pushed a commit to Eskaan/GDLauncher that referenced this pull request Nov 15, 2023
* update project/deploy configs

* add system architecture detection to java jre/jvm downloader

* Update package.json

* update electron-builder and electron-updater

arm is added as an alias for armv7l
makes building easier
Eskaan pushed a commit to Eskaan/GDLauncher that referenced this pull request Nov 17, 2023
* update project/deploy configs

* add system architecture detection to java jre/jvm downloader

* Update package.json

* update electron-builder and electron-updater

arm is added as an alias for armv7l
makes building easier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Trouble Building on Arm64
2 participants