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

Switch to command line tools xcode before running tests on mac #4809

Closed
wants to merge 1 commit into from

Conversation

Haroon-Khel
Copy link
Contributor

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

Switches xcode path to /Library/Developer/CommandLineTools before running tests. This should be the default xcode path anyway (when the machine is idle). Due to adoptium/temurin-build#3492, after a jdk8 build the machine will be stuck on /Applications/Xcode-11.7.app/Xcode.app/Contents/Developer which causes errors with gcc and make

14:06:42       [exec] gcc -D_JNI_IMPLEMENTATION_ -D_TRIVIAL_AGENT -O0 -g3 -pedantic -c -Wall -std=c99 -fPIC -fno-omit-frame-pointer -static-libgcc -o /Users/jenkins/workspace/Grinder/aqa-tests/system/aqa-systemtest/openjdk.test.modularity/bin/tests/com.test.jlink/native/lib/osx/JniTest.o -arch arm64 -I. -I/Users/jenkins/workspace/Grinder/aqa-tests/system/aqa-systemtest/openjdk.test.modularity/bin/tests/com.test.jlink/native/lib -I/Users/jenkins/workspace/Grinder/openjdkbinary/j2sdk-image/Contents/Home/include/darwin -I/Users/jenkins/workspace/Grinder/openjdkbinary/j2sdk-image/Contents/Home/include -I/usr/include JniTest.c
14:06:42       [exec] make[2]: Leaving directory '/Users/jenkins/workspace/Grinder/aqa-tests/system/aqa-systemtest/openjdk.test.modularity/src/tests/com.test.jlink/native'
14:06:42       [exec] makefile:192: JAVA_EXECUTABLE set to /Users/jenkins/workspace/Grinder/openjdkbinary/j2sdk-image/Contents/Home/bin/java/
14:06:42       [exec] xcrun: error: unable to load libxcrun (dlopen(/Applications/Xcode-11.7.app/Xcode.app/Contents/Developer/usr/lib/libxcrun.dylib, 0x0005): could not use '/Applications/Xcode-11.7.app/Xcode.app/Contents/Developer/usr/lib/libxcrun.dylib' because it is not a compatible arch).
14:06:42       [exec] make[2]: *** [makefile:223: /Users/jenkins/workspace/Grinder/aqa-tests/system/aqa-systemtest/openjdk.test.modularity/bin/tests/com.test.jlink/native/lib/osx/JniTest.o] Error 1
14:06:42  

see https://ci.adoptium.net/job/Grinder/7745/console

I've rebuilt that failing job, https://ci.adoptium.net/job/Grinder/7749/console, with this pr's changes. Should pass 👍🏻

@Haroon-Khel Haroon-Khel requested review from adoptium-bot, sophia-guo and sxa and removed request for adoptium-bot October 12, 2023 15:39
@Haroon-Khel
Copy link
Contributor Author

Runs as expected

16:30:48  Switching Xcode to command line tools
[Pipeline] sh
16:30:49  + sudo xcode-select --switch /
[Pipeline] dir

@sophia-guo
Copy link
Contributor

sophia-guo commented Oct 12, 2023

Would that affect mac x64? Rerun with mac x64 and failed https://ci.adoptium.net/job/Grinder/7750/ for other reasons.

Another one https://ci.adoptium.net/job/Grinder/7751/

@Haroon-Khel
Copy link
Contributor Author

It should affect x64 too. Not sure why 7750 failed, looks like it failed to download the binary. 7751 looks good

@Haroon-Khel
Copy link
Contributor Author

Incase 7750 failed because it ran on a 1014 box, ive kicked off https://ci.adoptium.net/job/Grinder/7752/console on test-macstadium-macos1014-x64-1. It ran the xcode switch without error

@sxa
Copy link
Member

sxa commented Oct 12, 2023

This makes me a little nervous as we have others using our test suite and it feels risky to assume the path.
I thought we were putting something in place to reset the default after the builds - did that not go in or is this a mitigation for any situation where it aborts before doing so?

@Haroon-Khel
Copy link
Contributor Author

I thought we were putting something in place to reset the default after the builds - did that not go in or is this a mitigation for any situation where it aborts before doing so?

A mitigation.

it feels risky to assume the path.

I think switching to / is safe but I am not certain. @sophia-guo Are there any variables/fields in the test scripts which are set/used when running specifically on Adoptium machines?

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.

A mitigation

What is the actual risk at the moment? A quick check suggests that neither of the two aarch64 build machines have a ci.role.test label so the testing should be immune to experiencing the problem unless a build is explicitly sent to one of them.

@smlambert
Copy link
Contributor

What is the impact of this change for other vendors, Alibaba, Huawei, OpenJ9/Semeru who use this same codebase with regards to this change.

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Oct 12, 2023

What is the actual risk at the moment? A quick check suggests that neither of the two aarch64 build machines have a ci.role.test label so the testing should be immune to experiencing the problem unless a build is explicitly sent to one of them.

Thats true. Similarly the jdk builds wont run on the test mac aarcn64 boxes so we're reassured there too. So not too urgent then, but I would like to (in some capacity) cover the case where a jdk build runs on a test machine or a test runs on a build machine.

What is the impact of this change for other vendors, Alibaba, Huawei, OpenJ9/Semeru who use this same codebase with regards to this change.

If they run test jobs on mac machines, xcode will try to reset to the 'default' toolset, found in /Library/Developer/CommandLineTools, during the setup stage of the test job. If this path is not available then I expect the test job will crash. So this would affect others.

Is there a feature in the test scripts which enable us as Adoptium to use variables that are specific to us? I could wedge this pr in there

@sxa
Copy link
Member

sxa commented Oct 12, 2023

Sounds like we're at effectively zero risk at the moment for most standard operations, and potential risk to community members utilising or opens scripts, so I suggest we postpone this until after the release where we can evaluate the potential impact in more detail.

@sophia-guo
Copy link
Contributor

Are there any variables/fields in the test scripts which are set/used when running specifically on Adoptium machines?

I don't think we have right now.

@smlambert
Copy link
Contributor

If they run test jobs on mac machines, xcode will try to reset to the 'default' toolset, found in /Library/Developer/CommandLineTools, during the setup stage of the test job. If this path is not available then I expect the test job will crash. So this would affect others.

More precisely, if they run system test jobs on mac. It would affect them if they are trying to compile system test code, so in the case of running system tests. An option would be to check if that path is available (during the environment check done by TKG), and if not, exit with a useful error.

Is this PR still in play or needed (since the switch to Orka machines)? Not sure I have seen system test failures on mac platforms recently.

Copy link
Contributor

@karianna karianna left a comment

Choose a reason for hiding this comment

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

Is using the xcodes utility to manage this an option?

@sxa
Copy link
Member

sxa commented Nov 22, 2024

If they run test jobs on mac machines, xcode will try to reset to the 'default' toolset, found in /Library/Developer/CommandLineTools, during the setup stage of the test job. If this path is not available then I expect the test job will crash. So this would affect others.

Is this PR still in play or needed (since the switch to Orka machines)? Not sure I have seen system test failures on mac platforms recently.

Almost certainly not. Also with the job restrictions plugin which now separates build/test systems there should be no risk even if we were to use non-orka systems as the problems described are only caused by builds changing the defaults on machines that tests run on.

@smlambert
Copy link
Contributor

Okay, given the recent information and infra updates, I believe we can close this PR.

@smlambert smlambert closed this Nov 22, 2024
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.

5 participants