Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v27]

2024-04-17 Thread Suchismith Roy 
On Wed, 17 Apr 2024 10:42:46 GMT, Martin Doerr  wrote:

> Please take a look at Mandy's suggestions. I think they make sense.

Done. Sorry i missed it. I got notified that the patch had got approved so i 
went ahead with integrate.

-

PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2060978059

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v27]

2024-04-17 Thread Martin Doerr 
On Mon, 15 Apr 2024 16:10:38 GMT, Suchismith Roy  wrote:

>> Allow support for both .a and .so files in AIX.
>> If .so file is not found, allow fallback to .a extension.
>> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)
>
> Suchismith Roy has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - new line
>  - spaces fix
>  - spaces fix

Please take a look at Mandy's suggestions. I think they make sense.

-

PR Comment: https://git.openjdk.org/jdk/pull/17945#issuecomment-2060963452

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v27]

2024-04-15 Thread Martin Doerr 
On Mon, 15 Apr 2024 16:10:38 GMT, Suchismith Roy  wrote:

>> Allow support for both .a and .so files in AIX.
>> If .so file is not found, allow fallback to .a extension.
>> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)
>
> Suchismith Roy has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - new line
>  - spaces fix
>  - spaces fix

LGTM, now. A 2nd review is needed, especially for the test.

-

Marked as reviewed by mdoerr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17945#pullrequestreview-2001896452

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v27]

2024-04-15 Thread Mandy Chung 
On Mon, 15 Apr 2024 16:10:38 GMT, Suchismith Roy  wrote:

>> Allow support for both .a and .so files in AIX.
>> If .so file is not found, allow fallback to .a extension.
>> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)
>
> Suchismith Roy has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - new line
>  - spaces fix
>  - spaces fix

Copying the library from JDK and renamed as `.a` is not the best idea.  I am 
not sure how much work to execute the tool chain on AIX to generate a proper 
archive for a shared object.It's okay with me with this approach provided 
that the AIX Reviewer approves it.

test/jdk/java/lang/RuntimeTests/loadLibrary/aix/LoadAIXLibraryFromArchiveObject.java
 line 43:

> 41: // launches a java application passing this directory through 
> "-Djava.library.path".
> 42: // the java application then attempts to load the library using 
> System.loadLibrary()
> 43: public static void main(final String[] args) throws Exception {

Suggestion:

public static void main(String[] args) throws Exception {


You can drop `final` in the method signature and local variables below in the 
method body.

test/jdk/java/lang/RuntimeTests/loadLibrary/aix/LoadAIXLibraryFromArchiveObject.java
 line 53:

> 51: final Path testNativeLibDir = Path.of("native").toAbsolutePath();
> 52: Files.createDirectories(testNativeLibDir);
> 53: final Path libDummyArchive = 
> testNativeLibDir.resolve(archiveFileName);

Suggestion:

   Path libFooBarArchive = testNativeLibDir.resolve(archiveFileName);


Nit: match the expected library name.

-

PR Review: https://git.openjdk.org/jdk/pull/17945#pullrequestreview-2001874277
PR Review Comment: https://git.openjdk.org/jdk/pull/17945#discussion_r1566259916
PR Review Comment: https://git.openjdk.org/jdk/pull/17945#discussion_r1566262563

Re: RFR: 8319516: AIX System::loadLibrary needs support to load a shared library from an archive object [v27]

2024-04-15 Thread Suchismith Roy 
> Allow support for both .a and .so files in AIX.
> If .so file is not found, allow fallback to .a extension.
> JBS Issue: [JDK-8319516](https://bugs.openjdk.org/browse/JDK-8319516)

Suchismith Roy has updated the pull request incrementally with three additional 
commits since the last revision:

 - new line
 - spaces fix
 - spaces fix

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17945/files
  - new: https://git.openjdk.org/jdk/pull/17945/files/de7f3a72..12270d06

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17945=26
 - incr: https://webrevs.openjdk.org/?repo=jdk=17945=25-26

  Stats: 3 lines in 2 files changed: 2 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17945.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17945/head:pull/17945

PR: https://git.openjdk.org/jdk/pull/17945