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.

Sign up for GitHub

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

Jump to bottom

Fix build for JDK 18 >= EA b21 #1249

Merged
merged 1 commit into from Nov 17, 2021
Merged

Fix build for JDK 18 >= EA b21 #1249

merged 1 commit into from Nov 17, 2021

Conversation

Godin
Copy link
Member

@Godin Godin commented Nov 17, 2021

Using

openjdk version "18-ea" 2022-03-15
OpenJDK Runtime Environment (build 18-ea+23-1525)
OpenJDK 64-Bit Server VM (build 18-ea+23-1525, mixed mode, sharing)

currently execution of

mvn verify -Dbytecode.version=18 -Dsurefire.useFile=false

fails with

testReportWithSourceDirButNoDebug [src\/org\/jacoco\/ant\/ReportTaskTest.xml](org.jacoco.ant.ReportTaskTest)  Time elapsed: 0.014 sec  <<< ERROR!
/Users/godin/projects/jacoco/jacoco/org.jacoco.ant.test/src/org/jacoco/ant/ReportTaskTest.xml:92: java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
        at org.apache.tools.ant.taskdefs.ExecuteJava.execute(ExecuteJava.java:198)
        at org.apache.tools.ant.taskdefs.Java.run(Java.java:832)
        at org.apache.tools.ant.taskdefs.Java.executeJava(Java.java:226)
        at org.apache.tools.ant.taskdefs.Java.executeJava(Java.java:135)
        at org.apache.tools.ant.taskdefs.Java.execute(Java.java:108)
        at org.apache.tools.ant.UnknownElement.execute(UnknownElement.java:293)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
        at java.base/java.lang.reflect.Method.invoke(Method.java:577)
        at org.apache.tools.ant.dispatch.DispatchUtils.execute(DispatchUtils.java:106)
        at org.apache.tools.ant.Task.perform(Task.java:352)
        at org.apache.tools.ant.Target.execute(Target.java:437)
        at org.apache.tools.ant.Target.performTasks(Target.java:458)
        at org.apache.tools.ant.Project.executeSortedTargets(Project.java:1406)
        at org.apache.tools.ant.Project.executeTarget(Project.java:1377)
        at org.apache.tools.ant.helper.DefaultExecutor.executeTargets(DefaultExecutor.java:41)
        at org.apache.tools.ant.Project.executeTargets(Project.java:1261)
        at org.apache.ant.antunit.AntUnitScriptRunner.runTarget(AntUnitScriptRunner.java:226)
        at org.apache.ant.antunit.AntUnitScriptRunner.runSuite(AntUnitScriptRunner.java:305)
        at org.apache.ant.antunit.junit3.AntUnitSuite.runInContainer(AntUnitSuite.java:159)
        at org.apache.ant.antunit.junit4.AntUnitSuiteRunner.run(AntUnitSuiteRunner.java:185)
        at org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:53)
        at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:123)
        at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:104)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
        at java.base/java.lang.reflect.Method.invoke(Method.java:577)
        at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:164)
        at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:110)
        at org.apache.maven.surefire.booter.SurefireStarter.invokeProvider(SurefireStarter.java:172)
        at org.apache.maven.surefire.booter.SurefireStarter.runSuitesInProcessWhenForked(SurefireStarter.java:104)
        at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:70)
Caused by: java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
        at java.base/java.lang.System.setSecurityManager(System.java:411)
        at org.apache.tools.ant.types.Permissions.setSecurityManager(Permissions.java:103)
        at org.apache.tools.ant.taskdefs.ExecuteJava.run(ExecuteJava.java:219)
        at org.apache.tools.ant.taskdefs.ExecuteJava.execute(ExecuteJava.java:154)
        ... 29 more

Results :

Tests in error:
  testInclBootstrapClasses [src\/org\/jacoco\/ant\/CoverageTaskTest.xml](org.jacoco.ant.CoverageTaskTest): java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
  testReportWithSourceButNoDebug [src\/org\/jacoco\/ant\/ReportTaskTest.xml](org.jacoco.ant.ReportTaskTest): java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release
  testReportWithSourceDirButNoDebug [src\/org\/jacoco\/ant\/ReportTaskTest.xml](org.jacoco.ant.ReportTaskTest): java.lang.UnsupportedOperationException: The Security Manager is deprecated and will be removed in a future release

Tests run: 85, Failures: 0, Errors: 3, Skipped: 0

which is related to https://bugs.openjdk.java.net/browse/JDK-8270380

For the time being I pinned version of JDK 18 EA to b20 in Azure Pipelines to unblock processing of PRs such as #1247

@Godin Godin mentioned this pull request Nov 16, 2021
Open
@marchof
Copy link
Member

marchof commented Nov 16, 2021

@Godin Any preferences how to fix this? We probably can set a system property for those tests:

java.security.manager=allow

Are you already on it or should I try this?

I haven't seen an Ant option not to use a security manager.

@Godin
Copy link
Member Author

Godin commented Nov 17, 2021

Are you already on it or should I try this?

@marchof I'm on this.

Any preferences how to fix this? We probably can set a system property

And seems that I have better option 😉 will open PR after lunch.

@Godin Godin self-assigned this Nov 17, 2021
@Godin Godin added this to the 0.8.8 milestone Nov 17, 2021
@Godin Godin added this to Candidates in Current work items via automation Nov 17, 2021
@Godin Godin moved this from Candidates to Implementation in Current work items Nov 17, 2021
@Godin
Copy link
Member Author

Godin commented Nov 17, 2021 •
edited

Any preferences how to fix this? We probably can set a system property for those tests

java.security.manager=allow

You're right that execution of

mvn verify -Dbytecode.version=18 -DargLine=java.security.manager=allow

succeeds, however addition/modification of argLine in pom.xml IMO is a bet with IDEs whether it will be taken into account or not during launch of tests from IDE - see for example #785 (comment)

I haven't seen an Ant option not to use a security manager.

I was curious about this too. While was far more curious why some tests pass and some fail 😉 careful reading of failure message and test gave me a hint:

CoverageTaskTest.xml:155: java.lang.UnsupportedOperationException

line 155 is a java task, which is just below another java task at line 146 and which is successfully executed

jacoco/org.jacoco.ant.test/src/org/jacoco/ant/CoverageTaskTest.xml

Lines 145 to 157 in 70d5b98

<jacoco:coverage destfile="${exec.file}" inclbootstrapclasses="true" includes="java/sql/*">
<java classname="org.jacoco.ant.TestTarget" fork="true" failonerror="true">
<classpath path="${org.jacoco.ant.coverageTaskTest.classes.dir}"/>
</java>
</jacoco:coverage>
<au:assertLogContains text="Enhancing java with coverage"/>
<au:assertFileExists file="${exec.file}"/>
<au:assertLogContains text="Target executed"/>
<java classname="org.jacoco.ant.DumpExecClassNames" classpath="${java.class.path}" failonerror="true">
<arg value="${exec.file}" />
</java>

the difference between these two tasks is value of fork.

Ticket https://bz.apache.org/bugzilla/show_bug.cgi?id=65381#c0 in Ant bug tracker describes the same.

So my proposal is to use fork=true.

@Godin Godin moved this from Implementation to Review in Current work items Nov 17, 2021
@Godin Godin requested a review from marchof November 17, 2021 15:00
@marchof
Copy link
Member

marchof commented Nov 17, 2021

Thanks Evgeny, as this avoids the security manager at all for me this is the best solution. Thanks!

@marchof
Copy link
Member

marchof commented Nov 17, 2021

I assume you tested this locally? The CI is still on build 18-ea+20-1248

@Godin
Fix build for JDK 18 >= EA b21
7e207e6 
Ant task `java` with parameter `fork="false"` calls
`java.lang.System.setSecurityManager`,
however tests should not call it,
because as part of work on JEP 411
in JDK 17 it was marked as deprecated
(see https://bugs.openjdk.java.net/browse/JDK-8264713)
and in JDK 18 throws UnsupportedOperationException
unless system property `java.security.manager` set to `allow`
(see https://bugs.openjdk.java.net/browse/JDK-8270380).
@Godin
Copy link
Member Author

Godin commented Nov 17, 2021

I assume you tested this locally? The CI is still on build 18-ea+20-1248

yep, tested locally,
I unpinned version of JDK 18 EA in Azure Pipelines
and triggered rerun for this PR

@marchof marchof merged commit 024de66 into master Nov 17, 2021
Current work items automation moved this from Review to Done Nov 17, 2021
@marchof marchof deleted the issue-1249 branch November 17, 2021 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marchof marchof marchof approved these changes

Assignees

@Godin Godin

Labels
component: build component: test
Projects
Current work items
  
Done
Milestone
0.8.8
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Godin @marchof