RFR: 8350964: Add an ArtifactResolver.fetch(clazz) method [v3]

Matthew Donovan mdonovan at openjdk.org
Wed Mar 12 20:28:32 UTC 2025


On Wed, 12 Mar 2025 16:39:41 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Matthew Donovan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Changed ArtifactResolver.fetchOne() to throw a skipped exception
>
> test/jdk/sun/security/pkcs11/PKCS11Test.java line 749:
> 
>> 747:     private static Path fetchNssLib(Class<?> clazz, Path libraryName) throws IOException {
>> 748:         Path p = ArtifactResolver.fetchOne(clazz);
>> 749:         return findNSSLibrary(p, libraryName);
> 
> So this method should never return null. Can we change `fetchNssLib(String osId, Path libraryName)` to also never return null (default throws `SkippedException`)? Then inside `getNSSLibPath(String library)` there is no need for the null check.

Yep, i removed the unnecessary null-checks

> test/lib/jdk/test/lib/artifacts/ArtifactResolver.java line 78:
> 
>> 76:      * <p>
>> 77:      * Artifacts are defined with the {@link jdk.test.lib.artifacts.Artifact}
>> 78:      * annotation. The file name should have the format ARTIFACT_NAME-VERSION.EXT
> 
> It could also be a directory, right? So maybe `path name` is more precise.
> 
> Also, you haven't defined what each piece in ARTIFACT_NAME-VERSION.EXT is. On the other hand, the names in `Artifact` are `name`, `revision`, and `extension`.

This sentence doesn't really belong here anyway so I removed it entirely

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23989#discussion_r1992239433
PR Review Comment: https://git.openjdk.org/jdk/pull/23989#discussion_r1992239895


More information about the security-dev mailing list