Skip to content

refactor: replace plexus-archiver with JDK jar tool, use maven-archiver for manifest - #625

Draft
gnodet wants to merge 39 commits into
apache:maven-jar-plugin-4.xfrom
gnodet:feat/use-maven-archiver
Draft

gnodet wants to merge 39 commits into
apache:maven-jar-plugin-4.xfrom
gnodet:feat/use-maven-archiver

Conversation

@gnodet

@gnodet gnodet commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrates maven-jar-plugin away from plexus-archiver to the JDK built-in jar tool for archive creation, delegating manifest generation to maven-archiver (without plexus dependency).

This supersedes the approach in #618 where ManifestBuilder was inlined in the plugin. Following Hervé's feedback, manifest generation now lives in maven-archiver as shared code.

Changes

Kept from #618

  • ToolExecutor: JDK jar tool invocation via @argfile, staging directory, reproducible builds support
  • FileCollector: file collection from target/classes/
  • MetadataFiles: pom.xml/pom.properties temp file generation
  • Archive, DirectoryRole, ExcludedFiles, PomDerivation, TimestampCheck: unchanged

Replaced vs #618

  • ManifestBuilder → deleted, replaced by MavenArchiver.getManifest(session, project, archive) from maven-archiver:4.0.0-beta-7-SNAPSHOT
  • ArchiveConfiguration → deleted, replaced by MavenArchiveConfiguration from maven-archiver

pom.xml

Depends on

apache/maven-archiver#422 — which removes plexus-archiver from maven-archiver itself and changes getManifest() return type to java.util.jar.Manifest.

gnodet and others added 30 commits October 13, 2026 12:59
Replace MavenArchiveConfiguration/MavenArchiver with local
ArchiveConfiguration and ManifestBuilder classes.

ManifestBuilder inlines the manifest generation logic from
MavenArchiver.getManifest(), using:
- java.util.jar.Manifest directly (no plexus types)
- org.apache.maven.api.services.Interpolator for classpath layout
  interpolation (replaces plexus-interpolation)
- Inlined BuildHelper.discoverJavaRelease() (pure Maven API)

ArchiveConfiguration replaces MavenArchiveConfiguration with inner
ManifestConfig and ManifestSection classes matching the same plugin
parameter XML structure.
- Fix stale @link: ManifestConfig#isAddBuildEnvironmentEntries() does not
  exist (bare field, no getter); use field reference instead
- Add null/empty name guard in manifest section loop to prevent NPE at
  Manifest.getEntries().put(null, attrs) when <name> is missing
- Add ManifestBuilderTest covering: Build-Jdk-Spec presence/absence,
  extra entry precedence and sanitization, null/empty section name guard,
  Manifest-Version, and Created-By defaults
…sleading test, add classpath tests

- ManifestBuilder: move null/empty name guard before building Attributes
  object in manifest section loop (fail-fast, no needless allocation)
- ManifestBuilderTest: rename buildJdkSpecPresentByDefault to
  buildJdkSpecPresentWhenFieldDefaultIsTrue with a comment clarifying
  this tests the field-level default, not production behaviour (which
  AbstractJarMojo always overrides to false via addBuildEnvironmentEntries)
- ManifestBuilderTest: add classpath unit tests via SessionWithServices
  stub that vends inline DependencyResolver and Interpolator stubs:
  * classpathSimpleLayout — simple layout with useUniqueVersions=true
  * classpathRepositoryLayout — repository layout (groupId path)
  * classpathWithPrefix — classpathPrefix prepended to each entry
The maven-archiver dependency has been removed in this PR. Update all
site documentation that still linked to or described Maven Archiver:

- usage.md.vm: drop stale archiver link and ${mavenArchiverVersion}
  variable; update last-line executable JAR link to local example
- index.md: rename section to 'Archive Configuration'; replace Maven
  Archiver prose with ManifestBuilder/ArchiveConfiguration description
- faq.md: rewrite archive-parameter answer in terms of ArchiveConfiguration
- manifest-customization.md.vm: replace three Maven Archiver links with
  local ManifestBuilder description and plugin-info reference
- site.xml: remove two Maven Archiver sidebar entries
  ('Creating an Executable JAR File' and 'Using Your Own Manifest File')
Built-By and Build-Jdk-Spec are only included when
addBuildEnvironmentEntries is true (default: false).
Only Created-By (and Java-Version when discoverable) are present
by default.
The jar tool adds Manifest-Version automatically; no need to set it in
ManifestBuilder. Created-By is handled by ToolExecutor (removed for
non-reproducible builds, set to a JDK-independent value for reproducible
builds); ManifestBuilder setting it is redundant and the setCreatedBy()
method is removed along with the pluginVersion field it used.

Also make ManifestSection.manifestEntries final and rename test method
to reflect the eager ManifestConfig initialization.
…x redundant calls, update doc and stale comment
…ute loss

java.util.jar.Manifest.write() silently drops all main attributes when
Manifest-Version is absent from the manifest. This caused a regression
introduced in commit c2a3401: ManifestBuilder.buildManifest() creates a
new Manifest but never sets Manifest-Version, so the temp manifest file
written by MetadataFiles.addManifest() → manifest.write() contained only
\r\n (2 bytes), causing the jar tool to read an empty manifest.

Fix: set Manifest-Version: 1.0 unconditionally in buildManifest(), as
MavenArchiver.getManifest() always did.

Also add a regression test that verifies Manifest-Version is present in
the built manifest and that manifest.write() produces non-empty output.
Rename single-letter and abbreviated local variable names to
their full, descriptive counterparts as requested by @elharo:

- m -> manifest (Manifest local in buildManifest)
- org -> organization (Organization local in addSpecificationEntries
  and addImplementationEntries)
- p -> plugin (Plugin local in getPlugin)
- mgmt -> pluginManagement (PluginManagement local in getPlugin)
- conf -> configuration (XmlNode local in getPluginParameter)

Also add missing java.nio.file.Files import and replace the
fully-qualified java.nio.file.Files.isRegularFile() call with
the short form.
… buildJdkSpecDefaultEntry flag

The flag was always set to the same value as addBuildEnvironmentEntries
by AbstractJarMojo. Moving Build-Jdk-Spec directly into
addBuildEnvironmentEntries() removes the indirection and simplifies the
code. Tests updated accordingly.
Maven XML/Plexus binding can inject null for an empty <classpathPrefix/>
element. StringBuilder.append(null) silently appends the literal string
"null", corrupting classpath entries. Add an explicit null-and-empty
check before appending.
The null check alone is sufficient. Appending an empty string is a no-op,
so the !prefix.isEmpty() check is unnecessary.

Suggested by @desruisseaux.
…ustomization

Since maven-archiver is removed as a dependency, all configuration options
must be documented here rather than delegating to the maven-archiver shared docs.

Adds:
- Full table of <archive> top-level options (compress, addMavenDescriptor, forced,
  manifestFile, manifest, manifestEntries, manifestSections)
- Full table of <archive><manifest> options with types, defaults, and descriptions
- Classpath layout expression reference (artifact.* interpolation expressions)
- Worked examples: executable JAR, Class-Path generation, specification/implementation
  entries, manifest sections, and using an existing manifest file

Addresses: apache#618 (review)
Guillaume Nodet added 8 commits October 18, 2026 08:50
…ion.md.vm

The .vm file is processed by the Velocity template engine during
site generation. Literal ${artifact.*} references in the documentation
tables and layout examples were being interpreted as Velocity variable
references, causing a ParseErrorException at line 61.

Escape all literal ${...} expressions with \$ so Velocity treats them
as plain text, while leaving the intentional ${project.version}
substitutions untouched.
…ier expression

- manifestFile type was documented as File; the actual field is Path
- pomPropertiesFile (Path) was missing from the archive options table
- Normalize ${artifact.dashClassifier} to ${dashClassifier?} in the
  expressions table to match the layout-equivalent examples below it
…plate

The ${artifact.*} and ${dashClassifier?} expressions in
manifest-customization.md.vm were written as \${...} which caused a
Velocity ParseErrorException during site generation: Velocity attempted
to evaluate ${artifact.*} as a reference (invalid Velocity syntax).

Replace all occurrences with ${esc.d}{...} which is the established
pattern already used in this file (see line with project.url) and renders
as ${...} in the generated HTML output.
…rmalization, specificationVersion, packageName/addExtensions

Addresses issues raised in Copilot review:

1. addBuildEnvironmentEntries: restore Build-Tool (Apache Maven <version>),
   Build-Os (os.name, version, arch), and the full Build-Jdk format
   (java.version + vendor in parentheses), matching maven-archiver behaviour.

2. classpathPrefix normalization: replace backslashes with forward slashes
   and add a trailing '/' automatically when absent, preventing
   '<classpathPrefix>lib</classpathPrefix>' from producing 'liblib-a-1.0.jar'
   instead of 'lib/lib-a-1.0.jar'.

3. specificationVersion(): avoid producing an invalid trailing-dot string
   (e.g. '1.' for version '1.foo'); only append a dot if the next character
   is also a digit.

4. ArchiveConfiguration.ManifestConfig: add 'packageName' field (generates
   the Package manifest attribute) and 'addExtensions' field (accepted for
   compatibility with maven-archiver; no-op in this implementation).

Documentation and unit tests updated accordingly.
CLASSPATH_LAYOUT_TYPE_REPOSITORY and CLASSPATH_LAYOUT_TYPE_CUSTOM were
only used internally in resolveLayout() and were not more readable than
the literal strings. Inline them as per @elharo's suggestion.
CLASSPATH_LAYOUT_TYPE_SIMPLE is kept as it serves as the default value
in ArchiveConfiguration.
@gnodet
gnodet changed the base branch from master to maven-jar-plugin-4.x October 19, 2026 09:08
…ed ManifestBuilder

Replace the inlined ManifestBuilder and ArchiveConfiguration classes
with maven-archiver 4.0.0-beta-7-SNAPSHOT (MavenArchiver + MavenArchiveConfiguration).
This keeps the JDK jar tool approach from pr-618 while delegating manifest
generation back to the shared maven-archiver library.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant