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

Update x64 Mac build jobs to use macos11 label #825

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

Haroon-Khel
Copy link
Contributor

@Haroon-Khel Haroon-Khel commented Oct 10, 2023

In light of adoptium/infrastructure#2536 (comment) and adoptium/temurin-build#3354, and now that adoptium/temurin-build#3492 is in we can force the builds to run on Macos11 machines only.

I am positive I have updated the correct files, but feel free to correct me if I have not.

There is one problem. This line,

NODE_LABEL: "${additionalNodeLabels}&&${platformConfig.os}&&${archLabel}",
, will insist that the NODE_LABEL contains the architecture of the binary to build, in this case x64. So the final label will be "NODE_LABEL": "macos11&&build&&mac&&x64" which will prevent it from running on our arm mac build machines.

An easy solution is to add x64 labels to our build arm mac machines. A harder solution is to modify

NODE_LABEL: "${additionalNodeLabels}&&${platformConfig.os}&&${archLabel}",
to something more versatile which will not break the other build jobs, but at the moment I do not know this repo well enough for this to be a quick fix before the next release.

@sxa @andrew-m-leonard thoughts?

An easy solution is to add x64 labels to our build arm mac machines.

I'm in favour of this

@github-actions
Copy link

Thank you for creating a pull request!

Please check out the information below if you have not made a pull request here before (or if you need a reminder how things work).

Code Quality and Contributing Guidelines

If you have not done so already, please familiarise yourself with our Contributing Guidelines and Code Of Conduct, even if you have contributed before.

Tests

Github actions will run a set of jobs against your PR that will lint and unit test your changes. Keep an eye out for the results from these on the latest commit you submitted. For more information, please see our testing documentation.

In order to run the advanced pipeline tests (executing a set of mock pipelines), it requires an admin to post run tests on this PR.
If you are not an admin, please ask for one's attention in #infrastructure on Slack or ping one here.
To run full set of tests, use "run tests"; a subset of tests on specific jdk version, use "run tests quick 11,21"

@github-actions github-actions bot added jenkins-pipeline arm generation Issues that provide enhancements or fixes to the job generators mac x64 Issues that affect or relate to the x64/x32 LINUX OS labels Oct 10, 2023
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

I don't think this will work - it'll look for macos11&&build&&mac&&x64 (which we don't have any of) instead of selecting an aarch64 build machine

@sxa
Copy link
Member

sxa commented Oct 10, 2023

Take a look at the crossCompile sections in jdk11u for 32-bit arm and riscv64 for an example of how to make it run on a different machine. Hopefully something that that will work here too.

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Oct 12, 2023

Take a look at the crossCompile sections in jdk11u for 32-bit arm and riscv64 for an example of how to make it run on a different machine. Hopefully something that that will work here too.

I have added x64 labels to the build-arm-mac machines so they now show up in https://ci.adoptium.net/label/macos11&&build&&mac&&x64/. Let's merge this pr and if we see stable builds then I can implement a more versatile solution

@sxa @andrew-m-leonard

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

While it's not ideal having that label on a machine that is of a different architecture I'll approve this on the basis that this is an interim solution for next week's release and we will look at doing it properly for the next release. Let's also make sure that when we merge this we take the build label OFF the x64 ones (and log here which machines that's been done on) to ensure that we don't end up with a random selection of x64 or aarch64 systems used for builds which could affect reproducibility

@andrew-m-leonard
Copy link
Contributor

I think we might be able to use the crossCompile option, eg:


?

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Oct 12, 2023

@andrew-m-leonard Right now ive labelled our build-arm-mac machines with x64 so this will allow the arm mac build jobs to run on the appropriate machines. This is only a temp solution to see if we get a stable run of nightlies, and then I can implement a more long term solution using

(in a separate pr after the release)

You should know that with this pr merged only build-macstadium-macos11-arm64-1 and build-macstadium-macos11-arm64-2 will be used for building arm and x64 mac for the major jdk versions. I believe testing will still be done on both the x64 and arm64 test machines

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Oct 13, 2023

I just remembered, for cross compiling jdk11+ the --openjdk-target=x86_64-apple-darwin needs to be added to the configure args, so we cant merge this until changes are added for this

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

A block has been put on this Pull Request as this repository is temporarily under a code freeze due to an ongoing release cycle.

If this pull request needs to be merged during the release cycle then please comment /merge and a PMC member will be able to remove the block.

If the code freeze is over you can remove this block by commenting /thaw.

@sxa
Copy link
Member

sxa commented Oct 13, 2023

I just remembered, for cross compiling jdk11+ the --openjdk-target=x86_64-apple-darwin needs to be added to the configure args,

Oh https://github.com/adoptium/temurin-build/pull/3492/files was only doing the change for JDK8 - ok in that case I guess we'll need another temurin-build PR for 11+.

we can merge this until changes are added for this

Should that be "can't"?

@Haroon-Khel
Copy link
Contributor Author

Should that be "can't"?

Havent had my coffee this morning 😅

Oh https://github.com/adoptium/temurin-build/pull/3492/files was only doing the change for JDK8 - ok in that case I guess we'll bee need another temurin-build PR for 11+.

On it

@Haroon-Khel
Copy link
Contributor Author

adoptium/temurin-build#3502 is ready to review

@Haroon-Khel
Copy link
Contributor Author

/merge

@github-actions
Copy link

Approval to merge during the lockdown cycle

Please can two Adoptium PMC members comment /approve?

@Haroon-Khel Haroon-Khel enabled auto-merge (squash) October 13, 2023 15:08
@sxa
Copy link
Member

sxa commented Oct 13, 2023

/approve

1 similar comment
@andrew-m-leonard
Copy link
Contributor

/approve

@github-actions github-actions bot dismissed their stale review October 13, 2023 15:12

Thank you @sxa and @andrew-m-leonard for your approvals, this pull request is now approved to merge during release.

@Haroon-Khel Haroon-Khel merged commit 1f9dc24 into adoptium:master Oct 13, 2023
7 checks passed
Haroon-Khel added a commit that referenced this pull request Oct 16, 2023
* update x64 mac build jobs to use macos11 label

* typo
luhenry pushed a commit to luhenry/adoptium-ci-jenkins-pipelines that referenced this pull request Feb 3, 2024
* update x64 mac build jobs to use macos11 label

* typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm generation Issues that provide enhancements or fixes to the job generators jenkins-pipeline mac x64 Issues that affect or relate to the x64/x32 LINUX OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants