On Mon, 28 Sep 2026 05:35:31 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this change which proposes to improve the error 
>> message when a JAR file with no manifest is used to launch a java 
>> application using `java -jar <jarfile>` command? This addresses 
>> https://bugs.openjdk.org/browse/JDK-8392966.
>> 
>> With the changes in this PR, for a JAR without a manifest file, if it is 
>> launched using `java -jar foo.jar` command then the error message will now 
>> say:
>> 
>>> Error: No manifest in JAR file foo.jar
>> 
>> An existing jtreg test has been converted to junit and a new test method has 
>> been introduced to verify this change. tier1, tier2 and tier3 continue to 
>> pass after this change.
>> 
>> 
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jaikiran Pai has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains four additional 
> commits since the last revision:
> 
>  - merge latest from master branch
>  - remove incorrect comment about localization of error messages
>  - update test
>  - 8392966: Improve error message for executable JAR file

src/java.base/share/native/libjli/emessages.h line 66:

> 64: #define JAR_ERROR_CANNOT_OPEN       "Error: Unable to open JAR file %s"
> 65: #define JAR_ERROR_MANIFEST_MISSING  "Error: No manifest in JAR file %s"
> 66: #define JAR_ERROR_MANIFEST_PARSE    "Error: Unable to parse the manifest 
> of JAR file %s"

Although these names are easier to read than the old names, it means a 
different convention to all the other errors defined in emessages.h. It's not 
bad, just looks different.

src/java.base/share/native/libjli/java.c line 1473:

> 1471:             JLI_ReportErrorMessage(JAR_ERROR_MANIFEST_MISSING, 
> jar_path);
> 1472:         } else {
> 1473:             JLI_ReportErrorMessage(JAR_ERROR_MANIFEST_PARSE, jar_path);

This is "Unable to parse the manifest of JAR file" so I assume is the error 
when a corrupt JAR file is opened.  The existing code use JAR_ERROR3 had 
"Invalid or corrupt .." and I'm wondering if the replacement message needs to 
retain some of this because -3 is both the corrupt JAR file and no manifest 
case now.

src/java.base/share/native/libjli/parse_manifest.c line 589:

> 587:     if ((rc = find_file(fd, &entry, manifest_name)) != 0) {
> 588:         close(fd);
> 589:         return rc == -2 ? rc : -3; // -2 if manifest file not found, -3 
> for other errors

This would be easier to read/understand if you change to:

if (rc == -2) {
    return -2;  // manfiest entry not found
}
return -3;      // other error

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/33064#discussion_r4134736925
PR Review Comment: https://git.openjdk.org/jdk/pull/33064#discussion_r4134755713
PR Review Comment: https://git.openjdk.org/jdk/pull/33064#discussion_r4134711444

Reply via email to