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

[JENKINS-27624][JENKINS-42709] Bump Jenkins baseline to Java 8 minimum #2802

Merged
merged 8 commits into from Apr 4, 2017

Conversation

@batmat
Member

batmat commented Mar 13, 2017

Description

Cf. article on jenkins.io: Jenkins Upgrades To Java 8

See also JENKINS-42709.

Details:

  • Fix generics issues
  • Fix infradna/bridge-method-injector to work with Jenkins@JDK8
  • Fix compilation error (in test module)
[INFO] 100 warnings
[INFO] -------------------------------------------------------------
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /home/tiste/dev/JENKINS/jenkins/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE

I guess some help from @kohsuke may be useful here. cc @jglick & @stephenc too.

Changelog entries

Proposed changelog entries:

  • JENKINS-42709: Make Jenkins buildable with JDK 8
  • ...

Submitter checklist

  • JIRA issue is well described
  • Link to JIRA ticket in description, if appropriate
  • Appropriate autotests or explanation to why this change has no tests
  • For new API and extension points: Link to the reference implementation in open-source (or example in Javadoc)

Desired reviewers

@mention

szpak and others added some commits Mar 13, 2017

[JENKINS-42709] Unable to build for Java 8
With Java 8 set as build target version.

@batmat batmat changed the title from [JENKINS-42709] Make Jenkins buildable with JDK8 to [WIP JENKINS-42709] Make Jenkins buildable with JDK8 Mar 13, 2017

Clarify PermissionGroup.owner is @nonnull
Permission.owner is already marked @nonnull, and
is built using PermissionGroup.owner.
@jglick

This comment has been minimized.

Show comment
Hide comment
@jglick

jglick Mar 29, 2017

Member
        // checks the class version
        if (readShort(off + 6) > Opcodes.V1_7) {
            throw new IllegalArgumentException();
        }

Did you try just upgrading asm-debug-all from 4.2, say to 5.2?

        // checks the class version
        if (readShort(off + 6) > Opcodes.V1_8) {
            throw new IllegalArgumentException();
        }
Member

jglick commented Mar 29, 2017

        // checks the class version
        if (readShort(off + 6) > Opcodes.V1_7) {
            throw new IllegalArgumentException();
        }

Did you try just upgrading asm-debug-all from 4.2, say to 5.2?

        // checks the class version
        if (readShort(off + 6) > Opcodes.V1_8) {
            throw new IllegalArgumentException();
        }
@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Mar 29, 2017

Member

@jglick Pretty sure I did try that, among other combinations. But I will retry to make sure.

Member

batmat commented Mar 29, 2017

@jglick Pretty sure I did try that, among other combinations. But I will retry to make sure.

@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Mar 29, 2017

Member

To sum up, the hint of Jesse above pushed me into the right direction. I think I had touched Jenkins' pom.xml instead of the plugin's when twiddling with it.

So, now I'm past it, the good news is that the WAR build succeeds (though I didn't smoke test it yet), only the test module fails because of a Javac error. I think this is coming from the inference fixes that have been added while introducing lambdas, and now the code is ambiguous. Looking into it.

AbstractBuild build = buildAndAssertSuccess(job);
does not compile, and shows:

[ERROR] /home/tiste/dev/JENKINS/jenkins/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
[INFO] 1 error
Member

batmat commented Mar 29, 2017

To sum up, the hint of Jesse above pushed me into the right direction. I think I had touched Jenkins' pom.xml instead of the plugin's when twiddling with it.

So, now I'm past it, the good news is that the WAR build succeeds (though I didn't smoke test it yet), only the test module fails because of a Javac error. I think this is coming from the inference fixes that have been added while introducing lambdas, and now the code is ambiguous. Looking into it.

AbstractBuild build = buildAndAssertSuccess(job);
does not compile, and shows:

[ERROR] /home/tiste/dev/JENKINS/jenkins/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
[INFO] 1 error
Fix bridge-method-injector IllegalArgumentException
```
[ERROR] Failed to execute goal com.infradna.tool:bridge-method-injector:1.13:process (default) on project jenkins-core: Failed to process @WithBridgeMethods: Failed to process /home/tiste/dev/tmp/2017-03-29T23h05m22+0200-jenkins-core/jenkins/jenkins/core/target/classes/hudson/model/AbstractItem.class: IllegalArgumentException -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal com.infradna.tool:bridge-method-injector:1.13:process (default) on project jenkins-core: Failed to process @WithBridgeMethods
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:212)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
        at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
        at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
        at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
        at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
        at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307)
        at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193)
        at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:106)
        at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863)
        at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288)
        at org.apache.maven.cli.MavenCli.main(MavenCli.java:199)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
        at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
        at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)
Caused by: org.apache.maven.plugin.MojoExecutionException: Failed to process @WithBridgeMethods
        at com.infradna.tool.bridge_method_injector.ProcessMojo.execute(ProcessMojo.java:68)
        at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134)
        at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:207)
        ... 20 more
Caused by: java.io.IOException: Failed to process /home/tiste/dev/tmp/2017-03-29T23h05m22+0200-jenkins-core/jenkins/jenkins/core/target/classes/hudson/model/AbstractItem.class
        at com.infradna.tool.bridge_method_injector.MethodInjector.handle(MethodInjector.java:106)
        at com.infradna.tool.bridge_method_injector.ProcessMojo.execute(ProcessMojo.java:65)
        ... 22 more
Caused by: java.lang.IllegalArgumentException
        at org.objectweb.asm.ClassReader.<init>(ClassReader.java:170)
        at org.objectweb.asm.ClassReader.<init>(ClassReader.java:153)
        at org.objectweb.asm.ClassReader.<init>(ClassReader.java:424)
        at com.infradna.tool.bridge_method_injector.MethodInjector.handle(MethodInjector.java:74)
        ... 23 more
[ERROR]

```
@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Mar 29, 2017

Member

So, now it fails with the following only in CI. And not locally with 1.8.0_121.
I can reproduce using 1.8.0_66, so I suspect it's a javac bug that got fixed between the two.

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java:[23,42] error: unreported exception T; must be caught or declared to be thrown
[INFO] 1 error
Member

batmat commented Mar 29, 2017

So, now it fails with the following only in CI. And not locally with 1.8.0_121.
I can reproduce using 1.8.0_66, so I suspect it's a javac bug that got fixed between the two.

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java:[23,42] error: unreported exception T; must be caught or declared to be thrown
[INFO] 1 error

@batmat batmat changed the title from [WIP JENKINS-42709] Make Jenkins buildable with JDK8 to [JENKINS-42709] Make Jenkins buildable with JDK8 Mar 29, 2017

@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Mar 29, 2017

Member

@jenkinsci/code-reviewers review welcome: this PR bumps Jenkins core to JDK8 minimum.

Member

batmat commented Mar 29, 2017

@jenkinsci/code-reviewers review welcome: this PR bumps Jenkins core to JDK8 minimum.

@batmat batmat changed the title from [JENKINS-42709] Make Jenkins buildable with JDK8 to [JENKINS-27624][JENKINS-42709] Bump Jenkins baseline to Java 8 minimum Mar 29, 2017

@aheritier

This comment has been minimized.

Show comment
Hide comment
Member

aheritier commented Mar 29, 2017

batmat added some commits Mar 29, 2017

Fix error with 1.8.0_77 in CI, does not happen with 1.8.0_121
Probably a bit ugly, but well for a test it should be acceptable.
Fix generics ambiguity
Inline the method to use concrete subtypes of AbstractBuild to remove ambiguity

```
[INFO] Jenkins war ........................................ SUCCESS [04:19 min]
[INFO] Tests for Jenkins core ............................. FAILURE [ 18.740 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 06:01 min
[INFO] Finished at: 2017-03-29T23:07:16+02:00
[INFO] Final Memory: 92M/504M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.0:testCompile (default-testCompile) on project test: Compilation failure
[ERROR] /home/tiste/dev/JENKINS/jenkins/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :test
```

@batmat batmat requested review from jglick and kohsuke Mar 30, 2017

@jglick

jglick approved these changes Mar 30, 2017

Show outdated Hide outdated pom.xml
@@ -483,6 +483,15 @@ THE SOFTWARE.
<groupId>com.infradna.tool</groupId>
<artifactId>bridge-method-injector</artifactId>
<version>1.13</version>
<!-- workaround waiting for a release with https://github.com/infradna/bridge-method-injector/pull/14 -->

This comment has been minimized.

@jglick

jglick Mar 30, 2017

Member

1.15 has been released acc. to @kohsuke.

@jglick

jglick Mar 30, 2017

Member

1.15 has been released acc. to @kohsuke.

This comment has been minimized.

@batmat

batmat Mar 30, 2017

Member

Fixed with 496d574

@batmat

batmat Mar 30, 2017

Member

Fixed with 496d574

@@ -655,7 +655,7 @@ private boolean relatedTo(@Nonnull AbstractBuild<?,?> b) {
@SuppressWarnings("unchecked")
@WithBridgeMethods(List.class)
public @Nonnull RunList getBuilds() {
return RunList.fromJobs(Jenkins.getInstance().allItems(Job.class)).filter(new Predicate<Run<?,?>>() {
return RunList.fromJobs((Iterable)Jenkins.getInstance().allItems(Job.class)).filter(new Predicate<Run<?,?>>() {

This comment has been minimized.

@jglick

jglick Mar 30, 2017

Member

Might suffice to just replace Run<?,?> with Run (untested). Or to add a qualifier to filter like

return RunList.fromJobs(Jenkins.getInstance().allItems(Job.class)).<Something<Here>>filter(new Predicate<Run<?,?>>() {
@jglick

jglick Mar 30, 2017

Member

Might suffice to just replace Run<?,?> with Run (untested). Or to add a qualifier to filter like

return RunList.fromJobs(Jenkins.getInstance().allItems(Job.class)).<Something<Here>>filter(new Predicate<Run<?,?>>() {

This comment has been minimized.

@batmat

batmat Mar 30, 2017

Member

I had tried quite a few combinations before finding that one, for instance with Run only:

image

So I'm ready to change to something else, but I thought the fix I have was readable/simple enough anyway (?).

@batmat

batmat Mar 30, 2017

Member

I had tried quite a few combinations before finding that one, for instance with Run only:

image

So I'm ready to change to something else, but I thought the fix I have was readable/simple enough anyway (?).

@@ -778,7 +778,7 @@ public String getUrl() {
}
public RunList getBuilds() {
return RunList.fromJobs(Jenkins.getInstance().allItems(Job.class)).node(getNode());
return RunList.fromJobs((Iterable)Jenkins.getInstance().allItems(Job.class)).node(getNode());

This comment has been minimized.

@jglick

jglick Mar 30, 2017

Member

Ditto.

@jglick

jglick Mar 30, 2017

Member

Ditto.

@@ -17,7 +17,7 @@
public static JenkinsRule j = new JenkinsRule();
@Test
public void isJenkinsJVM() throws Exception {
public void isJenkinsJVM() throws Throwable {

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Mar 30, 2017

Member

Hm? Why would you need such change?

@oleg-nenashev

oleg-nenashev Mar 30, 2017

Member

Hm? Why would you need such change?

This comment has been minimized.

@batmat

batmat Mar 30, 2017

Member

@oleg-nenashev I tried to separate each "fix" in a dedicated commit. This is 77dfa64
IOW, this is a weird javac bug, that was apparently fixed somewhere between 1.8.0_77 and _121. As this is a test, I thought it was worth changing the code a bit to avoid having some people unable to build Jenkins because they use a JDK8 that is not totally up to date.
Alternative would have been to add an enforcer rule to require some working javac version.

@batmat

batmat Mar 30, 2017

Member

@oleg-nenashev I tried to separate each "fix" in a dedicated commit. This is 77dfa64
IOW, this is a weird javac bug, that was apparently fixed somewhere between 1.8.0_77 and _121. As this is a test, I thought it was worth changing the code a bit to avoid having some people unable to build Jenkins because they use a JDK8 that is not totally up to date.
Alternative would have been to add an enforcer rule to require some working javac version.

This comment has been minimized.

@batmat

batmat Mar 30, 2017

Member

The build logs exhibiting the issue: https://ci.jenkins.io/job/Core/job/jenkins/job/PR-2802/2/execution/node/28/log/

Copying the end for future ref when the build has been wiped out:

proprietary API and may be removed in a future release
21:18:06 [INFO] 100 warnings 
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [ERROR] COMPILATION ERROR : 
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java:[23,42] error: unreported exception T; must be caught or declared to be thrown
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
21:18:06 [INFO] 2 errors 
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [INFO] Reactor Summary:
21:18:06 [INFO] 
21:18:06 [INFO] Jenkins main module ................................ SUCCESS [ 45.970 s]
21:18:06 [INFO] Jenkins cli ........................................ SUCCESS [ 42.146 s]
21:18:06 [INFO] Jenkins core ....................................... SUCCESS [03:12 min]
21:18:06 [INFO] Jenkins war ........................................ SUCCESS [01:23 min]
21:18:06 [INFO] Tests for Jenkins core ............................. FAILURE [ 56.812 s]
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [INFO] BUILD FAILURE
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [INFO] Total time: 07:18 min
21:18:06 [INFO] Finished at: 2017-03-29T21:18:05+00:00
21:18:06 [INFO] Final Memory: 100M/594M
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.0:testCompile (default-testCompile) on project test: Compilation failure: Compilation failure:
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java:[23,42] error: unreported exception T; must be caught or declared to be thrown
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
21:18:06 [ERROR] -> [Help 1]
21:18:06 [ERROR] 
21:18:06 [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
21:18:06 [ERROR] Re-run Maven using the -X switch to enable full debug logging.
21:18:06 [ERROR] 
21:18:06 [ERROR] For more information about the errors and possible solutions, please read the following articles:
21:18:06 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
21:18:06 [ERROR] 
21:18:06 [ERROR] After correcting the problems, you can resume the build with the command
21:18:06 [ERROR]   mvn <goals> -rf :test
@batmat

batmat Mar 30, 2017

Member

The build logs exhibiting the issue: https://ci.jenkins.io/job/Core/job/jenkins/job/PR-2802/2/execution/node/28/log/

Copying the end for future ref when the build has been wiped out:

proprietary API and may be removed in a future release
21:18:06 [INFO] 100 warnings 
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [ERROR] COMPILATION ERROR : 
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java:[23,42] error: unreported exception T; must be caught or declared to be thrown
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
21:18:06 [INFO] 2 errors 
21:18:06 [INFO] -------------------------------------------------------------
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [INFO] Reactor Summary:
21:18:06 [INFO] 
21:18:06 [INFO] Jenkins main module ................................ SUCCESS [ 45.970 s]
21:18:06 [INFO] Jenkins cli ........................................ SUCCESS [ 42.146 s]
21:18:06 [INFO] Jenkins core ....................................... SUCCESS [03:12 min]
21:18:06 [INFO] Jenkins war ........................................ SUCCESS [01:23 min]
21:18:06 [INFO] Tests for Jenkins core ............................. FAILURE [ 56.812 s]
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [INFO] BUILD FAILURE
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [INFO] Total time: 07:18 min
21:18:06 [INFO] Finished at: 2017-03-29T21:18:05+00:00
21:18:06 [INFO] Final Memory: 100M/594M
21:18:06 [INFO] ------------------------------------------------------------------------
21:18:06 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.0:testCompile (default-testCompile) on project test: Compilation failure: Compilation failure:
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/jenkins/util/JenkinsJVMRealTest.java:[23,42] error: unreported exception T; must be caught or declared to be thrown
21:18:06 [ERROR] /home/jenkins/workspace/Core_jenkins_PR-2802-7MEOUWG2NJTLSSLTVDVDIXOKU5BJ5UJ5PCL2QYQPQXAHRMFCDSMA/test/src/test/java/hudson/model/GetEnvironmentOutsideBuildTest.java:[89,30] error: no suitable method found for buildAndAssertSuccess(AbstractProject)
21:18:06 [ERROR] -> [Help 1]
21:18:06 [ERROR] 
21:18:06 [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
21:18:06 [ERROR] Re-run Maven using the -X switch to enable full debug logging.
21:18:06 [ERROR] 
21:18:06 [ERROR] For more information about the errors and possible solutions, please read the following articles:
21:18:06 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
21:18:06 [ERROR] 
21:18:06 [ERROR] After correcting the problems, you can resume the build with the command
21:18:06 [ERROR]   mvn <goals> -rf :test

This comment has been minimized.

@batmat

batmat Apr 2, 2017

Member

@oleg-nenashev does this address your question/concern? Thanks

@batmat

batmat Apr 2, 2017

Member

@oleg-nenashev does this address your question/concern? Thanks

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Apr 2, 2017

Member

it does

@oleg-nenashev
Use released bridge-method-injector:1.15
And suppress associated workaround
@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Apr 2, 2017

Member

@olivergondza I would really appreciate your review, especially with your LTS Officer cap, as this will possibly propagate to next one (or next-next) if we merge it soonish as initially planned.

Member

batmat commented Apr 2, 2017

@olivergondza I would really appreciate your review, especially with your LTS Officer cap, as this will possibly propagate to next one (or next-next) if we merge it soonish as initially planned.

@oleg-nenashev

I am fine with the PR itself, but maybe it makes sense to postpone it till the next weekly. #2826 is a serious regression, and I would rather allow Java 7 users to pick it. Not a blocker, just a quite biased IMHO

@@ -17,7 +17,7 @@
public static JenkinsRule j = new JenkinsRule();
@Test
public void isJenkinsJVM() throws Exception {
public void isJenkinsJVM() throws Throwable {

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Apr 2, 2017

Member

it does

@oleg-nenashev
@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck Apr 3, 2017

Member

Could we get rid of occurrences of MaxPermSize then? It means nothing on JDK 8 but spams the logs.

Member

daniel-beck commented Apr 3, 2017

Could we get rid of occurrences of MaxPermSize then? It means nothing on JDK 8 but spams the logs.

Remove MaxPermSize usage
Obsolete with Java 8. Permanent generation was removed.
@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Apr 3, 2017

Member

@daniel-beck done. Arnaud had already asked for it, and I said I would do it in a followup PR (to keep that PR a bit shorter). As you were now two to request it, I went ahead and did it here.

$ git grep -i permsize | wc -l
0
Member

batmat commented Apr 3, 2017

@daniel-beck done. Arnaud had already asked for it, and I said I would do it in a followup PR (to keep that PR a bit shorter). As you were now two to request it, I went ahead and did it here.

$ git grep -i permsize | wc -l
0
@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Apr 3, 2017

Member

I believe all concerns have been addressed and that this pull request is ready to merge.

If nobody objects in the meantime, or does it before me, I plan to merge this PR before the end of the week.

Which means that the next weekly will then require Java 8 to run.

Member

batmat commented Apr 3, 2017

I believe all concerns have been addressed and that this pull request is ready to merge.

If nobody objects in the meantime, or does it before me, I plan to merge this PR before the end of the week.

Which means that the next weekly will then require Java 8 to run.

@batmat batmat added ready-for-merge and removed needs-review labels Apr 3, 2017

@szpak

szpak approved these changes Apr 3, 2017

LGTM

@jglick

jglick approved these changes Apr 4, 2017

@jglick

This comment has been minimized.

Show comment
Hide comment
@jglick

jglick Apr 4, 2017

Member

#2826 is a serious regression, and I would rather allow Java 7 users to pick it.

@oleg-nenashev perhaps you missed our policy update when getting rid of the rc branch, but in summary: if you find a serious regression and fix it, you can ask @kohsuke to cut an out-of-order trunk release. No need to wait a week.

Member

jglick commented Apr 4, 2017

#2826 is a serious regression, and I would rather allow Java 7 users to pick it.

@oleg-nenashev perhaps you missed our policy update when getting rid of the rc branch, but in summary: if you find a serious regression and fix it, you can ask @kohsuke to cut an out-of-order trunk release. No need to wait a week.

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Apr 4, 2017

Member

@jglick Asn a requester of several out-of-order releases on this year, I definitely know about this opportunity. And I also know about obstacles like KK's reachability. In the case of this fix it has landed too late (late Friday) for making out-of-order release meaningful.

BTW I think we should just go forward with this PR

Member

oleg-nenashev commented Apr 4, 2017

@jglick Asn a requester of several out-of-order releases on this year, I definitely know about this opportunity. And I also know about obstacles like KK's reachability. In the case of this fix it has landed too late (late Friday) for making out-of-order release meaningful.

BTW I think we should just go forward with this PR

@batmat batmat merged commit 09cfe3b into jenkinsci:master Apr 4, 2017

2 checks passed

Jenkins This pull request looks good
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@batmat batmat deleted the batmat:JENKINS-42709 branch Apr 4, 2017

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Apr 4, 2017

Member

Merging without squash since there are 2 authors && the change is not a backporting candidate

Member

oleg-nenashev commented Apr 4, 2017

Merging without squash since there are 2 authors && the change is not a backporting candidate

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Apr 4, 2017

Member

Uh oh, the merge button dissapeared :D

Member

oleg-nenashev commented Apr 4, 2017

Uh oh, the merge button dissapeared :D

@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Apr 4, 2017

Member

Thank you everyone! \o/

Member

batmat commented Apr 4, 2017

Thank you everyone! \o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment