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

8318039: GHA: Bump macOS and Xcode versions #544

Closed
wants to merge 1 commit into from

Conversation

zzambers
Copy link
Contributor

@zzambers zzambers commented Jul 9, 2024

This backport is written from scratch, as GHA workflows in jdk8u differ significantly from newer jdk versions.

Notes:

  • additionally installed gawk to prevent build error on newer macos (see my comment)
  • interestingly, unlike for newer jdks (11, 17), additional backports do not seem necessary to pass the build, probably build system of jdk8 is more relaxed, when it comes to warnings as errors
  • fixes problem of stuck macos jobs in GHA (caused by dropping of macos-11 by GitHub)

Testing:

  • macos builds pass
  • there are some test failures on updated macos, probably problem in test environment, seem to be hostname related -> to be fixed (either here, or in separate PR)

Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • JDK-8318039 needs maintainer approval

Issue

  • JDK-8318039: GHA: Bump macOS and Xcode versions (Enhancement - P4 - Approved)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/544/head:pull/544
$ git checkout pull/544

Update a local copy of the PR:
$ git checkout pull/544
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/544/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 544

View PR using the GUI difftool:
$ git pr show -t 544

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/544.diff

Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Jul 9, 2024

👋 Welcome back zzambers! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Jul 9, 2024

@zzambers This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8318039: GHA: Bump macOS and Xcode versions

Reviewed-by: sgehwolf, andrew

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 3 new commits pushed to the master branch:

  • f231e27: 8330415: Update system property for Java SE specification maintenance version
  • 21e4278: 8320964: sun/tools/native2ascii/Native2AsciiTests.sh fails on Japanese
  • fc15e99: 8305931: jdk/jfr/jcmd/TestJcmdDumpPathToGCRoots.java failed with "Expected chains but found none"

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot changed the title Backport 605c9767291ddf1c409c3e805ffb3182899d06c2 8318039: GHA: Bump macOS and Xcode versions Jul 9, 2024
@openjdk
Copy link

openjdk bot commented Jul 9, 2024

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added backport rfr Pull request is ready for review labels Jul 9, 2024
@mlbridge
Copy link

mlbridge bot commented Jul 9, 2024

Webrevs

Copy link
Contributor

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

Looks fine to me. The warnings are still there and it would probably be good to fix them. In separate PRs though.

@openjdk
Copy link

openjdk bot commented Jul 9, 2024

⚠️ @zzambers This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

@zzambers
Copy link
Contributor Author

zzambers commented Jul 9, 2024

/approval request Fixes MacOS testing in GHA by updating MacOS/XCode to newer versions, affects GHA testing only

@openjdk
Copy link

openjdk bot commented Jul 9, 2024

@zzambers
8318039: The approval request has been created successfully.

@openjdk openjdk bot added the approval label Jul 9, 2024
@zzambers
Copy link
Contributor Author

zzambers commented Jul 9, 2024

Most of macos test failures seem to be caused by machine hostname not being resolved to IP, as expected. When I tried to add entry to /etc/hosts assigning 127.0.0.1 to hostname of the machine, in separate branch, it fixes most of the failures. (there are few remaining failures though)

@jerboaa Should I include fix here or rather create separate followup issue?

@jerboaa
Copy link
Contributor

jerboaa commented Jul 9, 2024

@jerboaa Should I include fix here or rather create separate followup issue?

We need to include any necessary fixes before we can include this one. So it would either be a) backport separately as preparation for this backport b) include with this. Leaning towards a).

@zzambers
Copy link
Contributor Author

zzambers commented Jul 10, 2024

I have done some additional testing and macos-13 machines indeed do not resolve their own hostname to IP address. I see this infra/environment issue by GitHub.

This causes many tier1 test failures on jdk8 (many com/sun/jdi/* and runtime/7158988/FieldMonitor.java). It does not cause tier1 test failures on openjdk/jdk, jdk21u-dev, even without workaround as it seems. It does, however, cause failures on jdk11 (checked on jdk11 with some not-yet-integrated fixes cherry-picked and disabled-warning-as-errors on macos-13). So failures are not jdk8 specific as I previously thought...

Ideally this would be fixed by GitHub in their infra/images, I have found there was the same macos issue for runner-images in the past. I commented there (I can open new issue, if it does not get their attention). Maybe I should wait for their reaction, before doing workaround and backporting it.

@vieiro
Copy link

vieiro commented Jul 12, 2024

Hi @zzambers @jerboaa ,

In the latest 11 builds on macos-13 the tests that are failing [1] are all related to JDI/JDWP, and report connection failures, and the tests fail only on this platform.

An example stack trace (attached): stack-trace.txt shows (note the empty localAddress):

Caused by: java.lang.InternalError: Failed remote listen: java.util.concurrent.ExecutionException: com.sun.jdi.connect.TransportTimeoutException: timeout waiting for connection @ com.sun.jdi.SocketListen (defaults: timeout=, port=, localAddress=) -- {timeout=timeout=5000, port=port=50478, localAddress=localAddress=}

And then times out:

2024-07-12T08:44:34.5430720Z Caused by: com.sun.jdi.connect.TransportTimeoutException: timeout waiting for connection
2024-07-12T08:44:34.5432330Z 	at jdk.jdi/com.sun.tools.jdi.SocketTransportService.accept(SocketTransportService.java:379)
2024-07-12T08:44:34.5460550Z 	at jdk.jdi/com.sun.tools.jdi.GenericListeningConnector.accept(GenericListeningConnector.java:155)

I was wondering if this is something related to -Djava.net.preferIPv4Stack=true perhaps?

[1]

jdk/jshell/ExceptionMessageTest.java: Test exception().getMessage() in events returned by eval()
jdk/jshell/JdiFailingLaunchExecutionControlTest.java: Tests for JDI connector failure
jdk/jshell/JdiFailingListenExecutionControlTest.java: Tests for JDI connector failure
jdk/jshell/JdiHangingLaunchExecutionControlTest.java: Tests for JDI connector timeout failure 

com/sun/jdi/AccessSpecifierTest.java: Test fix for JDI: methods Accessible.is...() lie about array types
com/sun/jdi/AfterThreadDeathTest.java: Creating a StepRequest on a nonexistant thread fails
com/sun/jdi/ArrayRangeTest.java: Test access to ranges within ArrayReferences
com/sun/jdi/ConstantPoolInfo.java: Test ReferenceType.majorVersion(), minorVersion, constantPoolCount and ConstantPool apis.
com/sun/jdi/CountFilterTest.java: Check correct processing of filters after a count filter
com/sun/jdi/EarlyReturnNegativeTest.java: Unexpected ClassCastException in ThreadReference.forceEarlyReturn
com/sun/jdi/EarlyReturnTest.java: Need a way to create JDI VoidValue for use in ThreadReference.forceEarlyReturn
com/sun/jdi/FieldWatchpoints.java: Test fix for: JDWP: WatchpointEvents outside of class filtered out
com/sun/jdi/FramesTest.java: Test ThreadReference.frames(int,int)
com/sun/jdi/InstanceFilter.java: Verify that an instance filter on a MethodEntryRequest works properly.
com/sun/jdi/InterfaceMethodsTest.java: JDI: Add support for static, private and default methods in interfaces
com/sun/jdi/InvokeTest.java: Test argument types for invoke
com/sun/jdi/LocalVariableEqual.java: Test
com/sun/jdi/LocationTest.java: Test that Method.location() returns the right values
com/sun/jdi/ModificationWatchpoints.java: Test all info returned by modification watchpoints
com/sun/jdi/MonitorEventTest.java: Simple basic test of jdi Monitor request and event.
com/sun/jdi/MonitorFrameInfo.java: MonitorInfo objects aren't invalidated when the owning thread is resumed
com/sun/jdi/NullThreadGroupNameTest.java: Ensure that JDWP doesn't crash with a null thread group name
com/sun/jdi/PopAndStepTest.java: Hin says that doing a step over after a popframe acts like a resume.
com/sun/jdi/PopAsynchronousTest.java: Test the popping of frames in an asynchronous context (that is, when suspended by the debugger at random points)
com/sun/jdi/ReferrersTest.java: Add support for backtracking reference graph.
com/sun/jdi/RequestReflectionTest.java: RequestReflectionTest checks to see that reflective accessors on EventRequests return what they are given.
com/sun/jdi/ResumeOneThreadTest.java: Thread resume invalidates all stack frames, even from other threads
com/sun/jdi/SourceNameFilterTest.java: JDI add addSourceNameFilter to ClassPrepareRequest
com/sun/jdi/VarargsTest.java: JPDA: Add support for RFE 4856541 - varargs
com/sun/jdi/Vars.java: Test Method.variables() and the like.
com/sun/jdi/redefineMethod/RedefineTest.java: Test class redefinition - method data line numbers and local vars,
com/sun/jdi/sde/MangleTest.java: Test the new SourceDebugExtension facility
com/sun/jdi/sde/TemperatureTableTest.java: Test the new SourceDebugExtension facility

@zzambers
Copy link
Contributor Author

@vieiro problem, which causes these failures on jdk8 is, that macos-13 does not correctly resolve it's own hostname (see my comment higher). I have tried to work-around this issue by modifying /etc/hosts ( see: zzambers@53cac79 ), and it fixed most of the tests. You can try equivalent on jdk11 to see, to verify, if you face the same issue.
However it is something, which would better be fixed in GitHub images/infra.

@vieiro
Copy link

vieiro commented Jul 14, 2024

Hi @zzambers , @jerboaa
The /etc/hosts fix does indeed work. I think this is related to actions/runner-images#8649

@gnu-andrew
Copy link
Member

gnu-andrew commented Jul 19, 2024

@jerboaa Should I include fix here or rather create separate followup issue?

We need to include any necessary fixes before we can include this one. So it would either be a) backport separately as preparation for this backport b) include with this. Leaning towards a).

I don't see why. Having a Mac OS build is better than none at all, even if it fails a few tests (which is already true on 11u where this has been merged). Having this change in will also enable others to look at the Mac OS issues, without having to depend on this PR.

I'd go for c) get this in and then fix the JDI tests separately.

Copy link
Member

@gnu-andrew gnu-andrew left a comment

Choose a reason for hiding this comment

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

Backport looks good to me.

@gnu-andrew
Copy link
Member

/approve yes

@openjdk
Copy link

openjdk bot commented Jul 19, 2024

@gnu-andrew
8318039: The approval request has been approved.

@openjdk openjdk bot added ready Pull request is ready to be integrated and removed approval labels Jul 19, 2024
@vieiro
Copy link

vieiro commented Jul 20, 2024

For 11 I opened JDK-8336451. Also the PRs below openjdk/jdk11u-dev#2861 should bring the macos-build back online in 11.

It's getaddrinfo that is having trouble resolving (here's a reproducer https://github.com/vieiro/gha-macos-resolve-hostname)

@zzambers
Copy link
Contributor Author

Considering latest comments, I am going forward with this one. Once workaround for hostname resolution is in 11, it can be backported to 8.

/integrate

@openjdk
Copy link

openjdk bot commented Jul 22, 2024

Going to push as commit 765bd89.
Since your change was applied there have been 3 commits pushed to the master branch:

  • f231e27: 8330415: Update system property for Java SE specification maintenance version
  • 21e4278: 8320964: sun/tools/native2ascii/Native2AsciiTests.sh fails on Japanese
  • fc15e99: 8305931: jdk/jfr/jcmd/TestJcmdDumpPathToGCRoots.java failed with "Expected chains but found none"

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Jul 22, 2024
@openjdk openjdk bot closed this Jul 22, 2024
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jul 22, 2024
@openjdk
Copy link

openjdk bot commented Jul 22, 2024

@zzambers Pushed as commit 765bd89.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@zzambers
Copy link
Contributor Author

zzambers commented Jul 25, 2024

So there are 2 problems after macos update:

  • Hostname resolve issue (to be fixed by backport of JDK-8336451, after 11)
runtime/7158988/FieldMonitor.java
com/sun/jdi/AccessSpecifierTest.java 
com/sun/jdi/AfterThreadDeathTest.java 
com/sun/jdi/AllLineLocations.java 
com/sun/jdi/ArrayRangeTest.java 
com/sun/jdi/BacktraceFieldTest.java 
com/sun/jdi/BreakpointTest.java 
com/sun/jdi/ClassLoaderClassesTest.java 
com/sun/jdi/ClassesByName.java 
com/sun/jdi/ClassesByName2Test.java 
com/sun/jdi/ConnectedVMs.java 
com/sun/jdi/ConstantPoolInfo.java 
com/sun/jdi/CountEvent.java 
com/sun/jdi/CountFilterTest.java 
com/sun/jdi/DebuggerThreadTest.java 
com/sun/jdi/DeleteAllBkptsTest.java 
com/sun/jdi/DeleteEventRequestsTest.java 
com/sun/jdi/EarlyReturnNegativeTest.java 
com/sun/jdi/EarlyReturnTest.java 
com/sun/jdi/EnumTest.java 
com/sun/jdi/EventQueueDisconnectTest.java 
com/sun/jdi/ExceptionEvents.java 
com/sun/jdi/ExpiredRequestDeletionTest.java 
com/sun/jdi/FieldWatchpoints.java 
com/sun/jdi/FilterMatch.java 
com/sun/jdi/FilterNoMatch.java 
com/sun/jdi/FinalLocalsTest.java 
com/sun/jdi/FinalizerTest.java 
com/sun/jdi/FramesTest.java 
com/sun/jdi/GenericsTest.java 
com/sun/jdi/GetLocalVariables2Test.java 
com/sun/jdi/GetUninitializedStringValue.java 
com/sun/jdi/HomeTest.java 
com/sun/jdi/InstanceFilter.java 
com/sun/jdi/InstancesTest.java 
com/sun/jdi/InterfaceMethodsTest.java 
com/sun/jdi/InterruptHangTest.java 
com/sun/jdi/InvokeHangTest.java 
com/sun/jdi/InvokeTest.java 
com/sun/jdi/InvokeVarArgs.java 
com/sun/jdi/Java_gTest.java 
com/sun/jdi/LambdaBreakpointTest.java 
com/sun/jdi/LambdaStepTest.java 
com/sun/jdi/LaunchCommandLine.java 
com/sun/jdi/LineNumberInfo.java 
com/sun/jdi/LocalVariableEqual.java 
com/sun/jdi/LocationTest.java 
com/sun/jdi/MethodEntryExitEvents.java 
com/sun/jdi/MethodExitReturnValuesTest.java 
com/sun/jdi/MethodInvokeWithTraceOnTest.java 
com/sun/jdi/ModificationWatchpoints.java 
com/sun/jdi/MonitorEventTest.java 
com/sun/jdi/MonitorFrameInfo.java 
com/sun/jdi/MultiBreakpointsTest.java 
com/sun/jdi/NativeInstanceFilter.java 
com/sun/jdi/NewInstanceTest.java 
com/sun/jdi/NoLocInfoTest.java 
com/sun/jdi/NullThreadGroupNameTest.java 
com/sun/jdi/PopAndStepTest.java 
com/sun/jdi/PopAsynchronousTest.java 
com/sun/jdi/PopSynchronousTest.java 
com/sun/jdi/PrivateTransportTest.sh 
com/sun/jdi/RedefineCrossEvent.java 
com/sun/jdi/RedefineCrossStart.java 
com/sun/jdi/ReferrersTest.java 
com/sun/jdi/RepStep.java 
com/sun/jdi/RequestReflectionTest.java 
com/sun/jdi/ResumeOneThreadTest.java 
com/sun/jdi/SDENullTest.java 
com/sun/jdi/SimulResumerTest.java 
com/sun/jdi/SourceNameFilterTest.java 
com/sun/jdi/StepTest.java 
com/sun/jdi/SuspendThreadTest.java 
com/sun/jdi/TemplateTest.java 
com/sun/jdi/ThreadGroupTest.java 
com/sun/jdi/TwoThreadsTest.java 
com/sun/jdi/UTF8Test.java 
com/sun/jdi/UnpreparedByName.java 
com/sun/jdi/UnpreparedClasses.java 
com/sun/jdi/VMDeathLastTest.java 
com/sun/jdi/VMDeathRequestTest.java 
com/sun/jdi/VarargsTest.java 
com/sun/jdi/Vars.java 
com/sun/jdi/VisibleMethods.java 
com/sun/jdi/connect/spi/DebugUsingCustomConnector.java 
com/sun/jdi/oom/OomDebugTest.java 
com/sun/jdi/redefine/RedefineTest.java 
com/sun/jdi/redefineMethod/RedefineTest.java 
com/sun/jdi/sde/FilterMangleTest.java 
com/sun/jdi/sde/MangleStepTest.java 
com/sun/jdi/sde/MangleTest.java 
com/sun/jdi/sde/SourceDebugExtensionTest.java 
com/sun/jdi/sde/TemperatureTableTest.java  
compiler/unsafe/OpaqueAccesses.java
sun/misc/CopyMemory.java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

4 participants