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 support for building on AArch64 #231

Closed
wants to merge 2 commits into from
Closed

Conversation

nick-arm
Copy link
Member

@nick-arm nick-arm commented Oct 31, 2019

I'm working with OpenJDK on Arm machines so I want to be able to run
`git webrev' etc. there. At the moment we can't build the Skara tools on
AArch64 due to an implicit assumption that Linux means X86. I tried to
refactor the build scripts support multiple architectures on Linux. Only
X86 and AArch64 for now but should be trivial to add others.

Using the AdoptOpenJDK AArch64 binary build as there are no prebuilt
binaries for AArch64 on java.net.

Currently there are several Gradle sub-projects that use the image
plugin to build Java images, and each specifies the URL and SHA265 of
the binary JDK to use. Changed the top-level Gradle script to parse
deps.env and select the JDK source based on the OS and architecture
system properties. This gets rid of the duplicated URLs but now each
image { .. } block has a redundant section like the following for each
OS:

    jdk {
        url = project.jdkUrl
        sha256 = project.jdkSha256
    }

So maybe the plugin should be tweaked to move this up into the image {}
block itself?

Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Oct 31, 2019

👋 Welcome back ngasson! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request (refresh this page to view it).

@mlbridge
Copy link

@mlbridge mlbridge bot commented Oct 31, 2019

Webrevs

@mlbridge
Copy link

@mlbridge mlbridge bot commented Oct 31, 2019

Mailing list message from Nick Gasson nick.gasson@arm.com

Hi,

What's the workflow for make changes to the PR? (the Mac build broke.)
Should I amend the commit and force-push or add another commit to that
branch?

Webrev: https://webrevs.openjdk.java.net/skara/231/webrev.00

This 404s for me - is that a known issue?

Thanks,
Nick

@mlbridge
Copy link

@mlbridge mlbridge bot commented Oct 31, 2019

Mailing list message from Per Liden per.liden@oracle.com

On 10/31/19 12:01 PM, Nick Gasson wrote:

Hi,

What's the workflow for make changes to the PR? (the Mac build broke.)
Should I amend the commit and force-push or add another commit to that
branch?

? Webrev: https://webrevs.openjdk.java.net/skara/231/webrev.00

This 404s for me - is that a known issue?

Hmm, works fine for me.

cheers,
Per

@mlbridge
Copy link

@mlbridge mlbridge bot commented Oct 31, 2019

Mailing list message from Robin Westberg robin.westberg@oracle.com

Hi Nick,

On 31 Oct 2019, at 12:01, Nick Gasson <Nick.Gasson at arm.com> wrote:

Hi,

What's the workflow for make changes to the PR? (the Mac build broke.) Should I amend the commit and force-push or add another commit to that branch?

Usually adding another commit is the preferred choice, this will enable automatic generation of incremental webrevs. But either works fine, when integrating the branch all commits will be squashed.

Webrev: https://webrevs.openjdk.java.net/skara/231/webrev.00

This 404s for me - is that a known issue?

Sometimes it takes a little while to update I think, but usually it is quite fast. Looks like it is working now at least.

Best regards,
Robin

Thanks,
Nick

@mlbridge
Copy link

@mlbridge mlbridge bot commented Nov 2, 2019

Mailing list message from Nick Gasson nick.gasson@arm.com

Hi Robin,

Webrev: https://webrevs.openjdk.java.net/skara/231/webrev.00

This 404s for me - is that a known issue?

Sometimes it takes a little while to update I think, but usually it is quite fast. Looks like it is working now at least.

Yeah it works for me now, guess I was just impatient :-)

Thanks,
Nick

Copy link
Member

@edvbld edvbld left a comment

Hi Nick,

first of all, thanks for contributing! We definitely want to support building the Skara tooling on AArch64, but please see my inline comments, it seems that you may have misinterpreted how the images target is meant to work.

Thanks!
Erik

url = 'https://download.java.net/java/GA/jdk12/GPL/openjdk-12_osx-x64_bin.tar.gz'
sha256 = '52164a04db4d3fdfe128cfc7b868bc4dae52d969f03d53ae9d4239fe783e1a3a'
url = project.jdkUrl
sha256 = project.jdkSha256
Copy link
Member

@edvbld edvbld Nov 4, 2019

Choose a reason for hiding this comment

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

This is unfortunately not how the jdk property of the images target is meant to be used.

The JDK fully supports cross-platform linking, something we make use of in Skara. This means if you build the :cli:images target on a Linux x64 machine then you will actually get fully working Skara distributions for Windows, macOS and Linux (all x64), since we are downloading the JDKs for macOS, Windows and Linux. With this patch we only get Linux x64 distributions.

Fortunately the JDK not only supports cross-jlink across operating system, it also supports cross-linking across CPU architectures. So I would prefer to be able to write:

linux_x64 {
    modules = ext.modules
    launchers = ext.launchers
    man = 'cli/resources/man'
    bundles = ['zip', 'tar.gz']
    jdk {
        url = 'https://download.java.net/java/GA/jdk12/GPL/openjdk-12_linux-x64_bin.tar.gz'
        sha256 = 'b43bc15f4934f6d321170419f2c24451486bc848a2179af5e49d10721438dd56'
    }
}

linux_aarch64 {
    modules = ext.modules
    launchers = ext.launchers
    man = 'cli/resources/man'
    bundles = ['zip', 'tar.gz']
    jdk {
        url = 'https://github.com/AdoptOpenJDK/openjdk12-binaries/releases/download/jdk-12.0.2%2B10/OpenJDK12U-jdk_aarch64_linux_hotspot_12.0.2_10.tar.gz'
        sha256 = ''855f046afc5a5230ad6da45a5c811194267acd1748f16b648bfe5710703fe8c'
    }
}

This way we will always produce Linux AArch64 images on Linux x64 hosts. To get this to work you need to do some minor changes to ImagesPlugin.java (and you might have to do some adjustments to LinkTask.java). Do you want to take a stab at that? Or do you want me to try and hack something together?

Copy link
Member Author

@nick-arm nick-arm Nov 4, 2019

Choose a reason for hiding this comment

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

Ah ok, I see that now. I'm happy to have a go at redoing it.

echo "error: unsupported Linux architecture ${ARCH}"
exit 1
;;
esac
Copy link
Member

@edvbld edvbld Nov 4, 2019

Choose a reason for hiding this comment

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

This part looks good 👍

@edvbld
Copy link
Member

@edvbld edvbld commented Nov 5, 2019

@nick-arm I thought some more about the build today and ended up implementing something a bit more scalable and future-proof than just adding ARM support on Linux. Please have a look PR #237 and see what you think.

If #237 is fine with you, can I go ahead and close this pull request?

Thanks,
Erik

@nick-arm
Copy link
Member Author

@nick-arm nick-arm commented Nov 7, 2019

@edvbld yes I just tried #237 and it works fine modulo the few small comments I left. Thanks!

@nick-arm nick-arm closed this Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants