feat: use hard links for artifact installation by default by gnodet · Pull Request #2198 · apache/maven-resolver

gnodet · 2026-10-10T12:31:15Z

Summary

Add aether.installer.hardLink configuration property (default true) to make DefaultInstaller create a hard link from the build output file to its destination in the local repository, instead of copying it.

This saves significant disk I/O for large JARs when building locally: instead of reading and writing the full file, the OS simply adds a new directory entry pointing to the same inode.

Motivation

This was raised as a follow-up to apache/maven-install-plugin#468. The install plugin only assembles the ArtifactInstallerRequest — the right place to implement hard-link installation is here in DefaultInstaller, which owns the file-copy step.

The feature is analogous to FileTransporter.WriteOp.HARDLINK which already exists for file:// remote repositories.

Behaviour

Default: aether.installer.hardLink=true — hard link attempted first.
Fallback to copy — when Files.createLink throws UnsupportedOperationException (unsupported filesystem) or IOException (cross-device link, EXDEV), the installer silently falls back to a regular file copy. The build is never broken.
Opt out: aether.installer.hardLink=false — always copy, same as the previous behaviour.

Design notes

Property defined on ConfigurationProperties with full @configurationSource / @configurationType / @configurationDefaultValue / @configurationRepoIdSuffix annotations following the established pattern.
PREFIX_INSTALLER = "aether.installer." added alongside the existing prefix constants.
Hard links share an inode. Any tool that overwrites build output in-place (not via atomic rename) would silently corrupt the cached artifact. Standard Maven tooling always uses atomic rename for target/, so this is safe in practice.
When destination exists as a directory, the code explicitly checks and falls through to copyWithTimestamp (which will fail as before), preserving existing error semantics.

Testing

testHardLinkInstallCreatesHardLink — verifies that install succeeds and (on same-device filesystems) produces a hard link (same inode). Gracefully skips the inode assertion on cross-device environments (common in CI containers).
testHardLinkDisabledFallsThroughToCopy — sets aether.installer.hardLink=false and verifies install still succeeds with correct content.
All 585 existing tests in maven-resolver-impl pass unchanged.

Configuration

To opt out globally, add to ~/.m2/settings.xml:

<!-- in Maven 4: via project properties or system property -->

Or pass on the command line:

mvn install -Daether.installer.hardLink=false

Hermès Agent (Claude Sonnet 4.6) on behalf of Guillaume Nodet

Add `aether.installer.hardLink` configuration property (default `true`) that makes `DefaultInstaller` create a hard link from the build output to the local repository instead of copying the file. This avoids redundant I/O for large JARs built locally. When hard linking is not possible (cross-device, unsupported filesystem), the installer automatically falls back to a regular file copy so the build is never broken. The property can be set to `false` to opt out and always use copy. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

gnodet

This PR adds hard-link support to the local repository installer as an opt-in (defaulting to true), which is a useful optimization. The implementation is mostly correct but has a few issues that should be fixed before merging.

Findings summary:

@since 2.0.9 is wrong everywhere — the POM is at 2.0.25-SNAPSHOT
AtomicMoveNotSupportedException in the Javadoc is inaccurate for Files.createLink()
IOException("destination is a directory") is silently swallowed by the fallback handler
ConfigUtils.getBoolean() is called once per artifact instead of once per session
testHardLinkInstallCreatesHardLink degenerates to a content-only test in CI (cross-device)
testHardLinkDisabledFallsThroughToCopy doesn't assert that createLink was NOT called

This review was generated by an AI agent, Hermès on behalf of @gnodet.

…tion (Jimfs/provider mismatch)

gnodet

Re-review after latest commits.

A few issues with the new code:

Corrected @SInCE tags — PREFIX_INSTALLER, INSTALLER_HARD_LINK, and DEFAULT_INSTALLER_HARD_LINK are all tagged @since 2.0.9, but the current POM is 2.0.25-SNAPSHOT. Should be @since 2.0.25.

IOException silently swallowed — the most critical finding: throw new IOException("destination is a directory") at line 220 is inside the same try block whose catch at line 224 catches IOException. The guard you added to detect a directory destination is immediately swallowed, causing a silent fallback to copyWithTimestamp(srcPath, dstPath) — which will also fail because the destination is a directory, but with a less informative error. The guard must be outside the try block, or the catch must exclude this specific case.

ConfigUtils.getBoolean() called per-artifact — this reads the same session config property on every artifact install. For a multi-module build with hundreds of artifacts, that's hundreds of map lookups on the same immutable session. Consider reading it once in the outer install(session, request) method and passing the boolean down.

createDirectories missing from the copy branch — Files.createDirectories(dstPath.getParent()) is called only inside the hard-link try block. If hardLink=false, the copy branch skips directory creation entirely. Worth confirming that pathProcessor.copyWithTimestamp() creates parent directories internally; if it doesn't, that's a latent NPE.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

@SInCE

… hoisting, log level - Fix @SInCE 2.0.9 → @SInCE 2.0.25 on PREFIX_INSTALLER, INSTALLER_HARD_LINK, DEFAULT_INSTALLER_HARD_LINK - Move isDirectory check outside the inner try/catch so IOException propagates correctly instead of being silently swallowed as a 'hard link not supported' fallback - Hoist ConfigUtils.getBoolean() to install(SyncContext,session,request) so it is read once per request instead of once per artifact - Upgrade fallback log from DEBUG to WARN so cross-device link fallback is visible

gnodet

Re-review after 9cfecea.

Addressed ✅ (6 findings from prior two reviews all fixed):

@since 2.0.25 on all three new constants
Javadoc exception types corrected (UnsupportedOperationException, IllegalArgumentException, IOException)
isDirectory guard moved before the inner try block — IOException now propagates correctly to InstallationException
ConfigUtils.getBoolean() hoisted to outer install(SyncContext,…), read once per request
Log level raised to WARN for fallback (was DEBUG)
pathProcessor.copyWithTimestamp() confirmed to create parent directories internally

Still open (2 findings): Test coverage gaps — both new test methods degenerate to content-only assertions in CI environments.

New finding (1): logger.warn fires per-artifact in environments where hard-linking always fails, producing one WARN line per artifact in a multi-module build.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open findings

Deleting the destination before the hard-link attempt can cause data loss if both the hard-link and… · New
The explicit directory check throws an IOException with a clear message, but that exception is… · New
The explicit directory check throws an IOException with a clear message, but that exception is… · New
The PR description states the installer will ‘silently’ fall back to copy, but the implementation… · New

What changed in this PR

Adds optional hard-link based installation to DefaultInstaller (enabled by default via aether.installer.hardLink) to reduce local disk I/O when installing artifacts, with tests covering both default behavior and opt-out.

Changes:

Introduces aether.installer.hardLink (+ default constant) in ConfigurationProperties.
Updates DefaultInstaller to attempt Files.createLink with fallback to copyWithTimestamp.
Adds regression tests validating hard-link behavior (when possible) and copy fallback when disabled.

File	Description
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultInstaller.java	Attempts hard-link artifact installation by default; falls back to copying based on config.
maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java	Defines new installer config key + default for hard-link installation.
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultInstallerTest.java	Adds tests for default hard-link attempt and explicit opt-out behavior.

🧠 Review effort: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

gnodet · 2026-10-12T18:56:35Z

Interaction with Windows IDE-lock fix (PR #2205)

When hardLink=true and IntelliJ (or another IDE) has the destination .jar open on Windows, Files.deleteIfExists(dstPath) inside the try block throws AccessDeniedException (a subclass of IOException). That is correctly caught and falls back to pathProcessor.copyWithTimestamp().

However, copyWithTimestamp() internally uses retryingMove(), which also fails with AccessDeniedException on the same locked target — so by itself, this PR's degradation path still ends with a build failure on Windows+IDE.

PR #2205 fixes retryingMove() to add a last-resort classicCopy() fallback after exhausting all rename attempts. Together the two PRs compose cleanly: hard-link fails → copyWithTimestamp → retryingMove retries → classicCopy fallback → build succeeds.

The degradation chain in hardLink=true mode on Windows with an IDE lock:

Files.deleteIfExists(dstPath) → IOException (caught ✅)
Falls back to pathProcessor.copyWithTimestamp(srcPath, dstPath)
retryingMove() → AccessDeniedException × N retries
With PR Fix #2204: Windows AccessDeniedException on install when IDE holds jar open #2205: classicCopy(source, target) → stream write succeeds ✅

So this PR's fallback degradation is correct but incomplete without #2205.

…sertions to tests - Replace per-artifact logger.warn with accumulate-then-log pattern: collect first link failure in outer loop, addSuppressed() for subsequent ones, log once at WARN after all artifacts are processed — avoids N WARN lines in multi-module builds where ~/.m2 is on a different mount - Change private install(..., hardLink) return type from void to Exception (the link failure, null on success) so the outer loop can collect failures - Add CountingPathProcessor in tests: counts copyWithTimestamp invocations, allowing testHardLinkInstallCreatesHardLink to assert copy was NOT called on same-device FS and WAS called on cross-device, and testHardLinkDisabledFallsThroughToCopy to assert copy IS called when hardLink=false (addresses review feedback on test coverage gaps)

gnodet-bot

Re-review after 5fdca38.

Addressed ✅ (2 findings from prior review fixed):

Per-artifact WARN spam replaced with accumulate-and-log-once pattern using addSuppressed() ✅
testHardLinkDisabledFallsThroughToCopy: assertEquals(1, countingProcessor.copyWithTimestampCount, ...) added after the try block — always runs ✅

Still open (1 finding): testHardLinkInstallCreatesHardLink — counting assertions still silently skipped on non-POSIX FS

The CountingPathProcessor counting assertions are inside the try { } catch (UnsupportedOperationException | IllegalArgumentException ignored) block. On non-POSIX filesystems (Windows, or a Jimfs-based test), Files.getAttribute(srcPath, "unix:dev") throws UnsupportedOperationException, the catch fires, and all counting assertions are silently skipped — the test degenerates to content-only.

Maven-resolver CI is Linux-only (so the cross-device else branch now fires correctly on CI), but a Windows developer running the suite locally would get zero behavioral coverage from this test.

Fix: add a counting assertion in the catch block (or move a FS-agnostic count check after the try-catch):

This review was generated by an AI agent, Hermès on behalf of @gnodet.

…nstallCreatesHardLink On non-POSIX filesystems (Windows, Jimfs), the unix:dev attribute lookup throws UnsupportedOperationException, causing all counting assertions inside the try block to be silently skipped. Add a minimal assertion in the catch block: copyWithTimestampCount must be <= 1 (either the hard link succeeded with count=0, or the fallback fired exactly once with count=1). This ensures the test provides some behavioral coverage even when inode/device checks are unavailable.

gnodet-bot

Re-review after 8d475cd.

Addressed ✅ — one finding from prior review resolved:

testHardLinkInstallCreatesHardLink: the counting assertion is now present inside the catch (UnsupportedOperationException | IllegalArgumentException ignored) block. On non-POSIX filesystems where Files.getAttribute(srcPath, "unix:dev") throws, the test now asserts copyWithTimestampCount <= 1. This is the tightest assertion possible without mocking Files.createLink at the syscall level — with a single artifact install, the correct outcome is either 0 (hard link succeeded) or 1 (fallback copy), and any double-copy regression would fire this assertion.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

gnodet mentioned this pull request Oct 10, 2026

Auto-detect module source hierarchy and skip main artifact at install apache/maven-install-plugin#468

Merged

2 of 3 tasks

gnodet commented Oct 10, 2026

View reviewed changes

fix: fall back to copy when hard link fails with IllegalArgumentExcep…

ae08e0b

…tion (Jimfs/provider mismatch)

gnodet commented Oct 10, 2026

View reviewed changes

gnodet requested review from cstamas and desruisseaux October 10, 2026 16:47

gnodet commented Oct 10, 2026

View reviewed changes

desruisseaux reviewed Oct 11, 2026

View reviewed changes

Comment thread maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultInstaller.java

desruisseaux reviewed Oct 11, 2026

View reviewed changes

Comment thread maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultInstaller.java Outdated

cstamas added the enhancement New feature or request label Oct 12, 2026

cstamas added this to the 2.0.25 milestone Oct 12, 2026

cstamas requested a balanced review from Copilot October 12, 2026 17:18

Copilot AI reviewed Oct 12, 2026

View reviewed changes

Copilot stopped reviewing on behalf of cstamas due to an error October 12, 2026 18:20
The job was not acquired by a runner even after multiple attempts.

gnodet mentioned this pull request Oct 12, 2026

Fix #2204: Windows AccessDeniedException on install when IDE holds jar open #2205

Open

gnodet-bot reviewed Oct 13, 2026

View reviewed changes

Comment thread maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultInstallerTest.java

gnodet-bot reviewed Oct 13, 2026

View reviewed changes

Conversation

gnodet commented Oct 10, 2026

Summary

Motivation

Behaviour

Design notes

Testing

Configuration

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gnodet commented Oct 12, 2026

Interaction with Windows IDE-lock fix (PR #2205)

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants