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

[JENKINS-41631, JENKINS-40088, JENKINS-43715, JENKINS-40979] - Update Stapler and Enforce upper bound deps #2956

Merged
merged 3 commits into from
Aug 11, 2017
Merged

[JENKINS-41631, JENKINS-40088, JENKINS-43715, JENKINS-40979] - Update Stapler and Enforce upper bound deps #2956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7c17eb
[JENKINS-41631] Enforce upper bound deps on Jenkins core.
jglick Jul 31, 2017
2fd8338
Merge branch 'master' into requireUpperBoundDeps-JENKINS-41631
jglick Aug 3, 2017
45f392c
stapler 1.252
jglick Aug 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ THE SOFTWARE.

<properties>
<staplerFork>true</staplerFork>
<stapler.version>1.250</stapler.version>
<stapler.version>1.252</stapler.version>
Copy link
Member Author

Choose a reason for hiding this comment

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

Changelog

Copy link
Member

@batmat batmat Aug 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Now I wonder why I didn't ask about this when reviewing: was this related to Enforcer Upper Bound Deps? Shouldn't it have been a dedicated PR?

Anyway, it revealed jenkinsci/stapler#128 for us, an internal plugin that does not compile anymore.

<spring.version>2.5.6.SEC03</spring.version>
<groovy.version>2.4.11</groovy.version>
<!-- TODO: Actually many issues are being filtered by src/findbugs/findbugs-excludes.xml -->
Expand Down Expand Up @@ -95,6 +95,12 @@ THE SOFTWARE.
<dependency>
<groupId>com.google.inject</groupId>
<artifactId>guice</artifactId>
<exclusions>
<exclusion> <!-- TODO it seems to want Guava 16; apparently it manages to run against 11 -->
Copy link
Member

Choose a reason for hiding this comment

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

What could possibly go wrong? :(

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, we are already doing it—this just makes it apparent.

<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency> <!-- for compatibility only; all new code should use JNR -->
Expand Down Expand Up @@ -602,6 +608,12 @@ THE SOFTWARE.
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<exclusions>
<exclusion> <!-- pick up from Stapler -->
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's safer to just define the newest version in this POM instead of adding exclusions

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe. Not used at runtime anyway, so probably does not matter.

</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
Expand Down
10 changes: 8 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ THE SOFTWARE.
<patch.tracker.serverId>jenkins-jira</patch.tracker.serverId>

<guavaVersion>11.0.1</guavaVersion>
<slf4jVersion>1.7.7</slf4jVersion> <!-- < 1.6.x version didn't specify the license (MIT) -->
<slf4jVersion>1.7.25</slf4jVersion>
Copy link
Member

Choose a reason for hiding this comment

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

👍 been hit by this recently and this improvement shows it's worth it to avoid JENKINS-41631 being too much of a nightmare when it starts reaching plugins (I think now it's still so recent that only very few ever tried, or reverted it seeing that new error).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes the intent here (together with the plugin-pom PR) was to allow a plugin using “newest everything” to build without warnings.

Copy link
Member

Choose a reason for hiding this comment

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

FYI @jtnord

<maven-plugin.version>2.14</maven-plugin.version>
<matrix-project.version>1.4.1</matrix-project.version>
<sorcerer.version>0.11</sorcerer.version>
Expand Down Expand Up @@ -207,7 +207,7 @@ THE SOFTWARE.
<dependency>
<groupId>commons-logging</groupId>
<artifactId>commons-logging</artifactId>
<version>1.1.3</version>
<version>1.2</version>
<scope>provided</scope><!-- by jcl-over-slf4j -->
</dependency>
<dependency>
Expand Down Expand Up @@ -558,6 +558,11 @@ THE SOFTWARE.
</dependency>
</dependencies>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<version>3.0.0-M1</version> <!-- TODO 3.0.0 when released -->
Copy link
Member Author

Choose a reason for hiding this comment

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

#2982

</plugin>
<!--This plugin's configuration is used to store Eclipse m2e settings only. It has no influence on the Maven build itself.-->
<plugin>
<groupId>org.eclipse.m2e</groupId>
Expand Down Expand Up @@ -690,6 +695,7 @@ THE SOFTWARE.
<enforceBytecodeVersion>
<maxJdkVersion>1.${java.level}</maxJdkVersion>
</enforceBytecodeVersion>
<requireUpperBoundDeps/>
</rules>
</configuration>
</execution>
Expand Down
47 changes: 44 additions & 3 deletions test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ THE SOFTWARE.
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>jenkins-test-harness</artifactId>
<version>2.20</version>
<version>2.23</version>
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this change, I'd guess

Copy link
Member

Choose a reason for hiding this comment

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

It is, to catch jenkinsci/jenkins-test-harness#62 which indeed got released as 2.23.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct.

Copy link
Member

Choose a reason for hiding this comment

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

ack

<scope>test</scope>
<exclusions>
<exclusion>
Expand Down Expand Up @@ -85,6 +85,14 @@ THE SOFTWARE.
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</exclusion>
<exclusion>
<groupId>com.google.inject</groupId>
<artifactId>guice</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.ant</groupId>
<artifactId>ant</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
Expand All @@ -106,7 +114,7 @@ THE SOFTWARE.
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>junit</artifactId>
<version>1.2-beta-4</version>
<version>1.6</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -119,6 +127,12 @@ THE SOFTWARE.
<groupId>org.jvnet.mock-javamail</groupId>
<artifactId>mock-javamail</artifactId>
<version>1.7</version>
<exclusions>
<exclusion>
<groupId>javax.mail</groupId>
<artifactId>mail</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
Expand All @@ -128,7 +142,7 @@ THE SOFTWARE.
<dependency><!-- we exclude this transient dependency from htmlunit, which we actually need in the test -->
<groupId>xalan</groupId>
<artifactId>xalan</artifactId>
<version>2.7.1</version>
<version>2.7.2</version>
Copy link
Member

Choose a reason for hiding this comment

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

So it's bundled into the core, right? Maybe requires a changelog entry as well

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this is the test POM.

<exclusions>
<exclusion>
<groupId>xml-apis</groupId>
Expand Down Expand Up @@ -156,6 +170,16 @@ THE SOFTWARE.
<groupId>org.reflections</groupId>
<artifactId>reflections</artifactId>
<version>0.9.9</version>
<exclusions>
<exclusion> <!-- TODO requests 15; apparently works well enough with the 11 we bundle -->
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
<exclusion> <!-- pick up from Stapler -->
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.codehaus.geb</groupId>
Expand Down Expand Up @@ -239,6 +263,23 @@ THE SOFTWARE.
<skip>true</skip>
</configuration>
</plugin>
<plugin> <!-- TODO pending JENKINS-45271 fix, would be best to finish moving MavenModuleSet-specific tests to maven-plugin and delete the test dep here -->
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, needs fix asap. OTOH, how many MavenPlugin-dependent tests do we have in the core? Maybe we should just move them elsewhere and remove the dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are a bunch. I do think it would be wise to move them to maven-plugin and delete the dep. I just did not want to take it on in this PR; would confuse the issue too much.

Copy link
Member

Choose a reason for hiding this comment

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

Could you please create a follow-up ticket for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

JENKINS-45952

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<configuration>
<rules>
<requireUpperBoundDeps>
<excludes combine.children="append">
<exclude>org.apache.maven:maven-embedder</exclude>
<exclude>org.codehaus.plexus:plexus-classworlds</exclude>
<exclude>org.apache.maven:maven-core</exclude>
<exclude>org.apache.maven:maven-aether-provider</exclude>
<exclude>org.codehaus.plexus:plexus-utils</exclude>
</excludes>
</requireUpperBoundDeps>
</rules>
</configuration>
</plugin>
</plugins>
</build>

Expand Down
1 change: 0 additions & 1 deletion war/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,6 @@ THE SOFTWARE.
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<version>1.3.1</version>
<executions>
<execution>
<id>enforce-versions</id>
Expand Down