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

use new toolchain parent #6

Closed
wants to merge 3 commits into from
Closed

use new toolchain parent #6

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
c01ed29
use new toolchain parent
olamy Nov 6, 2023
f58be69
remove useless attributes
olamy Nov 7, 2023
59b7ae5
fix org name
olamy Nov 7, 2023
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
10 changes: 0 additions & 10 deletions header-template.txt
View file
Open in desktop

This file was deleted.

197 changes: 73 additions & 124 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,45 +1,42 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.eclipse.jetty.toolchain</groupId>
<artifactId>jetty-toolchain</artifactId>
<version>1.7</version>
<version>1.8-parent-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

The child repos should never depend on a SNAPSHOT parent.
Depending on a SNAPSHOT complicates the release process.

This will also allow dependabot to update the parent versions.
AND we'll see when a plugin version update breaks the build of the child repo (which has happened countless times before).
But now we will have to manually fix the incompatible plugin 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.

@joakime It's a draft/working in progress test ONLY branch.
c'mon I will not release the parent immediately...

</parent>
<modelVersion>4.0.0</modelVersion>
<groupId>org.eclipse.jetty.toolchain.setuid</groupId>
<artifactId>jetty-setuid-jna</artifactId>
<version>2.0.3-SNAPSHOT</version>
<name>Jetty :: SetUID JNA</name>
<packaging>jar</packaging>
<name>Jetty :: SetUID JNA</name>
<scm>
<connection>scm:git:https://github.com/jetty/${jetty.git.repo}.git</connection>
<developerConnection>scm:git:[email protected]:jetty/${jetty.git.repo}.git</developerConnection>
<tag>HEAD</tag>
<url>https://github.com/jetty/${jetty.git.repo}</url>
</scm>
<properties>
<compiler.source>17</compiler.source>
<compiler.target>17</compiler.target>
<compiler.release>17</compiler.release>
<jdk.version.minimum>17</jdk.version.minimum>
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did this property name come from?
Is it one of the bog standard Maven properties?
If not, then this will break some IDE users. (the IDE will not know what JDK version to use)

Copy link
Member Author

Choose a reason for hiding this comment

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

please look at the new parent pom: https://github.com/jetty/jetty.toolchain/blob/7ca85e650cb2b83c80530639685cd5e1255d78b8/pom.xml#L199

<jetty-test-helper.version>6.2</jetty-test-helper.version>
<jetty.git.repo>jetty.setuid</jetty.git.repo>
<jetty.version>12.0.2</jetty.version>
<jna.version>5.13.0</jna.version>
<jetty-test-helper.version>6.2</jetty-test-helper.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
<licenses>
<license>
<name>Eclipse Public License - Version 2.0</name>
<url>https://www.eclipse.org/legal/epl-2.0/</url>
</license>
<license>
<name>Apache Software License - Version 2.0</name>
<url>https://www.apache.org/licenses/LICENSE-2.0</url>
</license>
</licenses>

<dependencies>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-server</artifactId>
<version>${jetty.version}</version>
</dependency>
<dependency>
<groupId>net.java.dev.jna</groupId>
<artifactId>jna</artifactId>
<version>${jna.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-server</artifactId>
<version>${jetty.version}</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand All @@ -52,67 +49,80 @@
<scope>test</scope>
</dependency>
</dependencies>

<repositories>
<repository>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
<id>jetty.snapshots</id>
<url>https://oss.sonatype.org/content/repositories/jetty-snapshots</url>
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto (from below)

Copy link
Contributor

Choose a reason for hiding this comment

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

If you want these in the pom, then they should be behind a profile, not default.
This will, at least, keep dependabot from using them.

Copy link
Member Author

Choose a reason for hiding this comment

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

#6 (comment)

</repository>
</repositories>

<pluginRepositories>
<pluginRepository>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
<id>jetty.snapshots</id>
<url>https://oss.sonatype.org/content/repositories/jetty-snapshots</url>
Copy link
Contributor

Choose a reason for hiding this comment

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

No, we should not do this.
The maven local repo caching between builds doesn't handle this very well.

We used to do this in jetty-project/ repos, and would get nice updates for our staged jetty releases, and also updates for snapshots on jetty-test-helper.
But rerolls or staged releases and updated SNAPSHOTS both used the old maven versions (even if they were updated days / weeks later)

We've ripped out this section from our jetty-project/ repos because of the weirdness here.

We shouldn't be reintroducing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

AGAIN it's a temporary working branch as the parent is not released yet and so GHA build would have fail.

</pluginRepository>
</pluginRepositories>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<compilerArgument>-Xlint:all</compilerArgument>
</configuration>
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.11.0</version>
<configuration>
<source>${compiler.source}</source>
<target>${compiler.target}</target>
<release>${compiler.release}</release>
<showWarnings>true</showWarnings>
<compilerArgument>-Xlint:all</compilerArgument>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId>
<version>3.0.1</version>
<configuration>
<useReleaseProfile>true</useReleaseProfile>
<releaseProfiles>eclipse-release</releaseProfiles>
<autoVersionSubmodules>true</autoVersionSubmodules>
Copy link
Contributor

Choose a reason for hiding this comment

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

Last time we tried this, the parent <configuration> didn't merge with the child repo, if we wanted something specific in the child repo, we couldn't just add a single configuration difference, we had to copy/paste the whole <configuration> to the child repo and then make edits.

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 it will not work for 1 or 2 of 20 or more repos but it will be still a gain
do you have an example I'm curious to see what may not work to override something in child poms

</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-enforcer-plugin</artifactId>
<version>3.4.1</version>
<artifactId>maven-assembly-plugin</artifactId>
<dependencies>
<dependency>
<groupId>org.eclipse.jetty.toolchain</groupId>
<artifactId>jetty-assembly-descriptors</artifactId>
<version>1.1</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>enforce-build-env</id>
<id>assemble-config</id>
<goals>
<goal>enforce</goal>
<goal>single</goal>
</goals>
<phase>validate</phase>
<phase>package</phase>
<configuration>
<rules>
<requireMavenVersion>
<version>[3.8.4,)</version>
<message>[ERROR] OLD MAVEN [${maven.version}] in use, Jetty ${project.version} requires Maven 3.8.4 or newer</message>
Copy link
Contributor

Choose a reason for hiding this comment

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

This enforcer configuration NEEDS to be at the child repo level.
Not in a parent pom.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's using a property so you can override it

</requireMavenVersion>
<requireJavaVersion>
<version>[17,)</version>
<message>[ERROR] OLD JDK [${java.version}] in use. Jetty ${project.version} requires JDK 17 or newer</message>
</requireJavaVersion>
<requireUpperBoundDeps />
</rules>
<descriptors>
<descriptor>src/main/assembly/config.xml</descriptor>
</descriptors>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-resources-plugin</artifactId>
<version>3.3.1</version>
<artifactId>maven-resources-plugin</artifactId>
<executions>
<execution>
<id>copy-resources</id>
<phase>process-resources</phase>
<goals>
<goal>copy-resources</goal>
</goals>
<phase>process-resources</phase>
<configuration>
<encoding>UTF-8</encoding>
<useDefaultDelimiters>false</useDefaultDelimiters>
Expand All @@ -130,68 +140,7 @@
</execution>
</executions>
</plugin>
<plugin>
<groupId>com.mycila</groupId>
<artifactId>license-maven-plugin</artifactId>
<version>4.3</version>
<configuration>
<licenseSets>
<licenseSet>
<header>header-template.txt</header>
<includes>
<include>**/*.java</include>
</includes>
</licenseSet>
</licenseSets>
<failIfMissing>true</failIfMissing>
<aggregate>true</aggregate>
<strictCheck>true</strictCheck>
<mapping>
<java>DOUBLESLASH_STYLE</java>
</mapping>
</configuration>
<executions>
<execution>
<id>check-headers</id>
<phase>validate</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-assembly-plugin</artifactId>
<version>3.6.0</version>
<dependencies>
<dependency>
<groupId>org.eclipse.jetty.toolchain</groupId>
<artifactId>jetty-assembly-descriptors</artifactId>
<version>1.1</version>
</dependency>
</dependencies>
<executions>
<execution>
<id>assemble-config</id>
<phase>package</phase>
<goals>
<goal>single</goal>
</goals>
<configuration>
<descriptors>
<descriptor>src/main/assembly/config.xml</descriptor>
</descriptors>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
<scm>
<connection>scm:git:https://github.com/jetty/jetty.setuid.git</connection>
<developerConnection>scm:git:[email protected]:jetty/jetty.setuid.git</developerConnection>
<url>https://github.com/jetty/jetty.setuid/tree/master</url>
<tag>HEAD</tag>
</scm>

</project>