gnodet-bot commented on code in PR #618:
URL: https://github.com/apache/maven-jar-plugin/pull/618#discussion_r4306393236
##########
src/site/markdown/examples/manifest-customization.md.vm:
##########
@@ -29,20 +29,72 @@ under the License.
The Default Manifest
--------------------
-The documentation for [Maven
Archiver](/shared/maven-archiver/examples/manifest.html) describes the default
contents of the manifest.
+The default manifest is built by `ManifestBuilder` based on the `<archive>`
configuration.
+By default, it includes the `Java-Version` attribute (when the compiler target
is discoverable from the POM).
+`Created-By` is always present in the final JAR — set by `ToolExecutor` to a
JDK-independent string for reproducible builds, or supplied by the `jar` tool
itself otherwise.
+When `addBuildEnvironmentEntries` is enabled, `Built-By`, `Build-Jdk`, and
`Build-Jdk-Spec` are also added.
+To add Specification and Implementation details or a classpath, configure them
explicitly in the `<archive>` element.
-Since version 2.1, the Maven JAR Plugin no longer creates the Specification
and Implementation details in the manifest by default.
-To add them, specify them explicitly in your plugin configuration.
-The documentation for [Maven
Archiver](/shared/maven-archiver/examples/manifest.html) describes this as well.
+The `<archive>` Configuration Element
+--------------------------------------
+
+The `<archive>` element accepts the following top-level options:
+
+| Field | Type | Default | Description |
+|---|---|---|---|
+| `compress` | boolean | `true` | Whether to compress the JAR file entries. |
+| `addMavenDescriptor` | boolean | `true` | Whether to include the Maven POM
descriptor (`META-INF/maven/…/pom.xml` and `pom.properties`) in the JAR. |
+| `forced` | boolean | `true` | Whether to recreate the archive even when up
to date. |
+| `manifestFile` | File | — | Path to an existing manifest file to use as a
base. Its entries are merged with any generated entries; explicitly configured
values take precedence. |
Review Comment:
⚠️ **Type mismatch:** The table documents `manifestFile` as type `File`, but
`ArchiveConfiguration.java` declares it as `java.nio.file.Path`.
```suggestion
| `manifestFile` | Path | — | Path to an existing manifest file to use as a
base. Its entries are merged with any generated entries; explicitly configured
values take precedence. |
```
##########
src/site/markdown/examples/manifest-customization.md.vm:
##########
@@ -29,20 +29,72 @@ under the License.
The Default Manifest
--------------------
-The documentation for [Maven
Archiver](/shared/maven-archiver/examples/manifest.html) describes the default
contents of the manifest.
+The default manifest is built by `ManifestBuilder` based on the `<archive>`
configuration.
+By default, it includes the `Java-Version` attribute (when the compiler target
is discoverable from the POM).
+`Created-By` is always present in the final JAR — set by `ToolExecutor` to a
JDK-independent string for reproducible builds, or supplied by the `jar` tool
itself otherwise.
+When `addBuildEnvironmentEntries` is enabled, `Built-By`, `Build-Jdk`, and
`Build-Jdk-Spec` are also added.
+To add Specification and Implementation details or a classpath, configure them
explicitly in the `<archive>` element.
-Since version 2.1, the Maven JAR Plugin no longer creates the Specification
and Implementation details in the manifest by default.
-To add them, specify them explicitly in your plugin configuration.
-The documentation for [Maven
Archiver](/shared/maven-archiver/examples/manifest.html) describes this as well.
+The `<archive>` Configuration Element
+--------------------------------------
+
+The `<archive>` element accepts the following top-level options:
+
+| Field | Type | Default | Description |
+|---|---|---|---|
+| `compress` | boolean | `true` | Whether to compress the JAR file entries. |
+| `addMavenDescriptor` | boolean | `true` | Whether to include the Maven POM
descriptor (`META-INF/maven/…/pom.xml` and `pom.properties`) in the JAR. |
+| `forced` | boolean | `true` | Whether to recreate the archive even when up
to date. |
+| `manifestFile` | File | — | Path to an existing manifest file to use as a
base. Its entries are merged with any generated entries; explicitly configured
values take precedence. |
+| `manifest` | ManifestConfig | — | Manifest-specific configuration; see
below. |
+| `manifestEntries` | Map | — | Arbitrary key/value pairs to add to the main
manifest section. |
+| `manifestSections` | List | — | Additional named manifest sections; each
must have a `<name>` and `<manifestEntries>`. |
+
+### `<manifest>` Options
+
+| Field | Type | Default | Description |
+|---|---|---|---|
+| `mainClass` | String | — | The `Main-Class` attribute value. Sets the entry
point for executable JARs. |
+| `addClasspath` | boolean | `false` | Whether to generate a `Class-Path`
attribute from the project's runtime dependencies. |
+| `classpathPrefix` | String | `""` | A prefix prepended to every entry in the
generated `Class-Path`. |
+| `classpathLayoutType` | String | `"simple"` | Layout type for `Class-Path`
entries. One of `simple`, `repository`, or `custom`. |
+| `customClasspathLayout` | String | — | Custom layout template when
`classpathLayoutType` is `custom`. Supports `${artifact.*}` expressions (see
below). |
+| `useUniqueVersions` | boolean | `true` | Whether to use unique timestamps
for SNAPSHOT artifacts in `Class-Path` entries. |
+| `addDefaultEntries` | boolean | `true` | Whether to add the `Java-Version`
attribute (sourced from the compiler plugin's `release`/`target` setting). |
+| `addBuildEnvironmentEntries` | boolean | `false` | Whether to add
`Build-Jdk-Spec`, `Build-Jdk`, and `Built-By` attributes from the current JVM. |
+| `addDefaultSpecificationEntries` | boolean | `false` | Whether to add
`Specification-Title`, `Specification-Version`, and `Specification-Vendor` from
the POM. |
+| `addDefaultImplementationEntries` | boolean | `false` | Whether to add
`Implementation-Title`, `Implementation-Version`, `Implementation-Vendor`,
`Implementation-Vendor-Id`, and `Implementation-URL` from the POM. |
+
+### Classpath Layout Expressions
+
+When `classpathLayoutType` is `custom`, the `customClasspathLayout` value is a
template that may reference
+the following `${artifact.*}` expressions for each runtime dependency:
+
+| Expression | Description |
+|---|---|
+| `${artifact.groupId}` | The dependency's group ID. |
+| `${artifact.groupIdPath}` | The group ID with `.` replaced by `/` (for
repository-style paths). |
+| `${artifact.artifactId}` | The dependency's artifact ID. |
+| `${artifact.version}` | The dependency's version (unique SNAPSHOT timestamp
when applicable). |
+| `${artifact.baseVersion}` | The dependency's base version (e.g.
`1.0-SNAPSHOT`). |
+| `${artifact.extension}` | The dependency's file extension (e.g. `jar`). |
+| `${artifact.classifier}` | The dependency's classifier, or empty string if
absent. |
+| `${artifact.dashClassifier}` | `-${classifier}` if a classifier is present,
otherwise empty. |
+
+The `repository` layout type is equivalent to:
+`${artifact.groupIdPath}/${artifact.artifactId}/${artifact.baseVersion}/${artifact.artifactId}-${artifact.version}${dashClassifier?}.${artifact.extension}`
+
+The `simple` layout type is equivalent to:
+`${artifact.artifactId}-${artifact.version}${dashClassifier?}.${artifact.extension}`
Review Comment:
⚠️ **Inconsistent expression syntax:** The expressions table (line 82)
documents `${artifact.dashClassifier}` (with `artifact.` prefix, no `?`
suffix), while the layout-equivalent examples (lines 85, 88) use
`${dashClassifier?}` (no `artifact.` prefix, with `?` suffix). Both forms work
at runtime (`ManifestBuilder` strips the `artifact.` prefix and handles both
key variants), but docs must be internally consistent.
Either update the table entry to match the layout examples:
```suggestion
| `${dashClassifier?}` | `-${classifier}` if a classifier is present,
otherwise empty. |
The `repository` layout type is equivalent to:
`${artifact.groupIdPath}/${artifact.artifactId}/${artifact.baseVersion}/${artifact.artifactId}-${artifact.version}${dashClassifier?}.${artifact.extension}`
The `simple` layout type is equivalent to:
`${artifact.artifactId}-${artifact.version}${dashClassifier?}.${artifact.extension}`
```
Or update the layout examples to use `${artifact.dashClassifier}` instead —
either convention is fine, pick one.
##########
src/site/markdown/examples/manifest-customization.md.vm:
##########
@@ -29,20 +29,72 @@ under the License.
The Default Manifest
--------------------
-The documentation for [Maven
Archiver](/shared/maven-archiver/examples/manifest.html) describes the default
contents of the manifest.
+The default manifest is built by `ManifestBuilder` based on the `<archive>`
configuration.
+By default, it includes the `Java-Version` attribute (when the compiler target
is discoverable from the POM).
+`Created-By` is always present in the final JAR — set by `ToolExecutor` to a
JDK-independent string for reproducible builds, or supplied by the `jar` tool
itself otherwise.
+When `addBuildEnvironmentEntries` is enabled, `Built-By`, `Build-Jdk`, and
`Build-Jdk-Spec` are also added.
+To add Specification and Implementation details or a classpath, configure them
explicitly in the `<archive>` element.
-Since version 2.1, the Maven JAR Plugin no longer creates the Specification
and Implementation details in the manifest by default.
-To add them, specify them explicitly in your plugin configuration.
-The documentation for [Maven
Archiver](/shared/maven-archiver/examples/manifest.html) describes this as well.
+The `<archive>` Configuration Element
+--------------------------------------
+
+The `<archive>` element accepts the following top-level options:
+
+| Field | Type | Default | Description |
+|---|---|---|---|
+| `compress` | boolean | `true` | Whether to compress the JAR file entries. |
+| `addMavenDescriptor` | boolean | `true` | Whether to include the Maven POM
descriptor (`META-INF/maven/…/pom.xml` and `pom.properties`) in the JAR. |
+| `forced` | boolean | `true` | Whether to recreate the archive even when up
to date. |
+| `manifestFile` | File | — | Path to an existing manifest file to use as a
base. Its entries are merged with any generated entries; explicitly configured
values take precedence. |
+| `manifest` | ManifestConfig | — | Manifest-specific configuration; see
below. |
+| `manifestEntries` | Map | — | Arbitrary key/value pairs to add to the main
manifest section. |
+| `manifestSections` | List | — | Additional named manifest sections; each
must have a `<name>` and `<manifestEntries>`. |
Review Comment:
⚠️ **Missing field:** `pomPropertiesFile` is declared in
`ArchiveConfiguration` (alongside `manifestFile`) but is absent from this
table. Users who need to supply a custom `pom.properties` file have no
documentation to reference.
```suggestion
| `manifestSections` | List | — | Additional named manifest sections; each
must have a `<name>` and `<manifestEntries>`. |
| `pomPropertiesFile` | Path | — | Path to an existing `pom.properties` file
to embed in the JAR. When set, overrides the auto-generated one. |
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
