Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v4]

2024-03-05 Thread David Holmes
On Tue, 5 Mar 2024 14:19:14 GMT, Erik Joelsson  wrote:

> Does the release note also looks ok?

Yes.

> Is there any process to formally review release notes?

No. Typically just add a comment to the RN sub-task indicating it is okay. My 
comment, that I had made some minor changes was meant as an indicator of that.

-

PR Comment: https://git.openjdk.org/jdk/pull/18050#issuecomment-1980229076


Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v4]

2024-03-05 Thread Magnus Ihse Bursie
On Mon, 4 Mar 2024 23:44:14 GMT, Erik Joelsson  wrote:

>> Executables and dynamic libraries on Linux can encode a search path that the 
>> dynamic linker will use when looking up library dependencies, generally 
>> referred to as an "rpath". In the JDK we use this with the $ORIGIN feature 
>> to set search paths relative to the location of the binary itself. Typically 
>> executables in the bin/ directory have the rpath "$ORIGIN/../lib" to find 
>> libjli.so. Most of the libraries in lib/ have rpath set to just "$ORIGIN" to 
>> find each other.
>> 
>> There are two different types of such rpaths, RPATH and RUNPATH. The former 
>> is the earlier incantation but RUNPATH has been around since about 2003 and 
>> has been default in prominent Linux distros for a long time, and now also 
>> seems to be default in the linker directly from binutils. The toolchain used 
>> by Oracle defaulted to RPATH until at least JDK 11, but since then with some 
>> toolchain upgrade, the default was flipped to RUNPATH.
>> 
>> The main (relevant in this case) difference between the two is that RPATH is 
>> considered before the LD_LIBRARY_PATH environment variable, while RUNPATH is 
>> only considered after LD_LIBRARY_PATH. For libraries that are part of a 
>> Linux distribution, letting users, or the system, control and override 
>> builtin rpaths with LD_LIBRARY_PATH seems like a reasonable thing to prefer. 
>> However, for the JDK, there really is no usecase for having an externally 
>> configured LD_LIBRARY_PATH potentially getting in the way of the JDK 
>> libraries finding each other correctly. If a user environment sets 
>> LD_LIBRARY_PATH, and there is a library in that path with the same name as a 
>> JDK library (e.g. libnet.so or some other generically named library) that 
>> external library will be loaded instead of the JDK internal library and that 
>> is basically guaranteed to break the JDK. There is no supported usecase that 
>> I can think of for injecting other versions of such libraries in a JDK 
>> distribution.
>> 
>> I propose that we explicitly configure the JDK build to set RPATH instead of 
>> RUNPATH for Linux binaries. This is done with the linker flag 
>> "--disable-new-dtags".
>
> Erik Joelsson has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   bug ref

Yes, the release note looks good. Is there any process to formally review 
release notes?

-

PR Comment: https://git.openjdk.org/jdk/pull/18050#issuecomment-197625


Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v4]

2024-03-05 Thread Erik Joelsson
On Tue, 5 Mar 2024 07:16:10 GMT, David Holmes  wrote:

> Thanks for the further explanation and adding the comment.
> 
> LGTM.

Thanks! Does the release note also looks ok? I understand it needs to be 
reviewed together with the PR.

-

PR Comment: https://git.openjdk.org/jdk/pull/18050#issuecomment-1978879864


Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v4]

2024-03-04 Thread David Holmes
On Mon, 4 Mar 2024 23:44:14 GMT, Erik Joelsson  wrote:

>> Executables and dynamic libraries on Linux can encode a search path that the 
>> dynamic linker will use when looking up library dependencies, generally 
>> referred to as an "rpath". In the JDK we use this with the $ORIGIN feature 
>> to set search paths relative to the location of the binary itself. Typically 
>> executables in the bin/ directory have the rpath "$ORIGIN/../lib" to find 
>> libjli.so. Most of the libraries in lib/ have rpath set to just "$ORIGIN" to 
>> find each other.
>> 
>> There are two different types of such rpaths, RPATH and RUNPATH. The former 
>> is the earlier incantation but RUNPATH has been around since about 2003 and 
>> has been default in prominent Linux distros for a long time, and now also 
>> seems to be default in the linker directly from binutils. The toolchain used 
>> by Oracle defaulted to RPATH until at least JDK 11, but since then with some 
>> toolchain upgrade, the default was flipped to RUNPATH.
>> 
>> The main (relevant in this case) difference between the two is that RPATH is 
>> considered before the LD_LIBRARY_PATH environment variable, while RUNPATH is 
>> only considered after LD_LIBRARY_PATH. For libraries that are part of a 
>> Linux distribution, letting users, or the system, control and override 
>> builtin rpaths with LD_LIBRARY_PATH seems like a reasonable thing to prefer. 
>> However, for the JDK, there really is no usecase for having an externally 
>> configured LD_LIBRARY_PATH potentially getting in the way of the JDK 
>> libraries finding each other correctly. If a user environment sets 
>> LD_LIBRARY_PATH, and there is a library in that path with the same name as a 
>> JDK library (e.g. libnet.so or some other generically named library) that 
>> external library will be loaded instead of the JDK internal library and that 
>> is basically guaranteed to break the JDK. There is no supported usecase that 
>> I can think of for injecting other versions of such libraries in a JDK 
>> distribution.
>> 
>> I propose that we explicitly configure the JDK build to set RPATH instead of 
>> RUNPATH for Linux binaries. This is done with the linker flag 
>> "--disable-new-dtags".
>
> Erik Joelsson has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   bug ref

Thanks for the further explanation and adding the comment.

LGTM.

-

Marked as reviewed by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/18050#pullrequestreview-1916027301


Re: RFR: 8326891: Prefer RPATH over RUNPATH for $ORIGIN rpaths in internal JDK binaries [v4]

2024-03-04 Thread Erik Joelsson
> Executables and dynamic libraries on Linux can encode a search path that the 
> dynamic linker will use when looking up library dependencies, generally 
> referred to as an "rpath". In the JDK we use this with the $ORIGIN feature to 
> set search paths relative to the location of the binary itself. Typically 
> executables in the bin/ directory have the rpath "$ORIGIN/../lib" to find 
> libjli.so. Most of the libraries in lib/ have rpath set to just "$ORIGIN" to 
> find each other.
> 
> There are two different types of such rpaths, RPATH and RUNPATH. The former 
> is the earlier incantation but RUNPATH has been around since about 2003 and 
> has been default in prominent Linux distros for a long time, and now also 
> seems to be default in the linker directly from binutils. The toolchain used 
> by Oracle defaulted to RPATH until at least JDK 11, but since then with some 
> toolchain upgrade, the default was flipped to RUNPATH.
> 
> The main (relevant in this case) difference between the two is that RPATH is 
> considered before the LD_LIBRARY_PATH environment variable, while RUNPATH is 
> only considered after LD_LIBRARY_PATH. For libraries that are part of a Linux 
> distribution, letting users, or the system, control and override builtin 
> rpaths with LD_LIBRARY_PATH seems like a reasonable thing to prefer. However, 
> for the JDK, there really is no usecase for having an externally configured 
> LD_LIBRARY_PATH potentially getting in the way of the JDK libraries finding 
> each other correctly. If a user environment sets LD_LIBRARY_PATH, and there 
> is a library in that path with the same name as a JDK library (e.g. libnet.so 
> or some other generically named library) that external library will be loaded 
> instead of the JDK internal library and that is basically guaranteed to break 
> the JDK. There is no supported usecase that I can think of for injecting 
> other versions of such libraries in a JDK distribution.
> 
> I propose that we explicitly configure the JDK build to set RPATH instead of 
> RUNPATH for Linux binaries. This is done with the linker flag 
> "--disable-new-dtags".

Erik Joelsson has updated the pull request incrementally with one additional 
commit since the last revision:

  bug ref

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/18050/files
  - new: https://git.openjdk.org/jdk/pull/18050/files/faafef8a..8639eee5

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=18050&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18050&range=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/18050.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/18050/head:pull/18050

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