Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v6]
On Thu, 25 Apr 2024 13:22:01 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove /jre path check Change still good > Still looks good. We should maybe change the synopsis (title) of the bug to > something like "libjli GetApplicationHome cleanups"? Well, it does enhance tracing, so the title is not lying :) - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2078751493
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v6]
On Thu, 25 Apr 2024 13:22:01 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove /jre path check Still looks good. We should maybe change the synopsis (title) of the bug to something like "libjli GetApplicationHome cleanups"? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2077289573
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v6]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove /jre path check - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/4d998244..4d8b6802 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=05 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=04-05 Stats: 24 lines in 2 files changed: 0 ins; 24 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
On Tue, 23 Apr 2024 14:31:44 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust output I removed the mentioned 'private JRE' check and also the related size check. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2077160938
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Tue, 23 Apr 2024 14:29:05 GMT, Matthias Baesken wrote: > > `/* Does the app ship a private JRE in /jre directory? */` > > part meant? This looks indeed obsolete . Yes, this is code that doesn't make sense since JDK 9 and should be removed/cleanup at some point. I suspect we had to leave it at the time because of the issue of installers copying java.exe into a shared location and expecting it to work with any JRE or JDK release, something that doesn't make sense now of course. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2077120542
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
On Tue, 23 Apr 2024 14:31:44 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust output LGTM - Marked as reviewed by stuefe (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18699#pullrequestreview-2022399268
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
On Tue, 23 Apr 2024 14:31:44 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust output I changed the added output to 'JRE path' to have more consistency with the existing JLI trace messages. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2075012914
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v5]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust output - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/8b7a9d58..4d998244 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=04 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=03-04 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output I adjusted the output ot 'JRE path' . > The GetJREPath function has no business looking for jre/lib as that location > does not exist since JDK 9. Is the at/below `/* Does the app ship a private JRE in /jre directory? */` part meant? This looks indeed obsolete . - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072472623
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Mon, 22 Apr 2024 11:57:19 GMT, Alan Bateman wrote: >> Hi, any additional comments / reviews ? >> Thanks Matthias > >> Hi, any additional comments / reviews ? Thanks Matthias > > It still looks like left over trace messages from a debugging session, need > to think about about what tracing make sense here. > @AlanBateman If this is not acceptable to add, I must once again iterate my > question: What is the use case for the tracing functionality then? The tracing was mostly for the options and timing when it was originally added. I don't object to expanding the tracing. The issue is that the proposed additions are inconsistent with the existing trace messages, e.g. GetJREPath's existing trace messages use "JRE path", the proposed messages switch to "JRE location". BTW: the vestiges of the "JRE" should really be removed. The GetJREPath function has no business looking for jre/lib as that location does not exist since JDK 9. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072415065
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output I agree; the launcher uses two different modes for discovery. If you want to troubleshoot, knowing which of these are attempted is helpful. @AlanBateman If this is not acceptable to add, I must once again iterate my question: What is the use case for the tracing functionality then? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072349646
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Mon, 22 Apr 2024 17:19:14 GMT, Magnus Ihse Bursie wrote: > But that sounds like a very special case. Not sure if it is really such a special case. Currently both modes (getting the image path from launcher binary path , and getting the image path from 'the dll' / GetApplicationHomeFromDll (libjli.so on Linux) exist and are to my knowledge supported. At least one jtreg test fails when the GetApplicationHomeFromDll does not work any more. So I think it is natural that both modes are also supported/augmented by some tracing. We could probably reduce the trace message(s) added to just one if this is desired . - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2072090837
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output It helped to understand the issue behind https://bugs.openjdk.org/browse/JDK-8329653 - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2070365643
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output What is the use case for this tracing functionality? I recently was quite helped by it when debugging static java launching. And in that case, the more logging the better. But that sounds like a very special case. Is this something that end users are supposed to use to help solve problems? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2070309758
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Mon, 22 Apr 2024 11:30:41 GMT, Matthias Baesken wrote: > Hi, any additional comments / reviews ? Thanks Matthias It still looks like left over trace messages from a debugging session, need to think about about what tracing make sense here. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2069209127
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
On Fri, 19 Apr 2024 10:07:22 GMT, Matthias Baesken wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > remove launcher executable path trace output Hi, any additional comments / reviews ? Thanks Matthias - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2069158463
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v3]
On Fri, 19 Apr 2024 09:20:27 GMT, Christoph Langer wrote: > This trace seems a bit unsymmetric to its Windows counterpart. Maybe it > should be left out here, too, since there is tracing in GetJREPath. I removed the JLI trace output for the launcher exe path. - PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1572140833
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v4]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove launcher executable path trace output - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/ddb3e0fe..8b7a9d58 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=03 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=02-03 Stats: 4 lines in 2 files changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 10:20:23 GMT, Alan Bateman wrote: >> We have already good JLI tracing capabilities. But GetApplicationHome and >> GetApplicationHomeFromDll lack some tracing and should be enhanced. > > I think this is way too ad hoc and looks like lefts over from a debugging > session. So I don't think it should be integrated without stepping back and > thinking more about what this tracing option is intended for. Generally this looks more consistent now. Let's see what @AlanBateman thinks... - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2066177891
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v3]
On Thu, 18 Apr 2024 06:57:05 GMT, Matthias Baesken wrote:
>> We have already good JLI tracing capabilities. But GetApplicationHome and
>> GetApplicationHomeFromDll lack some tracing and should be enhanced.
>
> Matthias Baesken has updated the pull request incrementally with one
> additional commit since the last revision:
>
> remove obsolete USE_REGISTRY_LOOKUP usages
src/java.base/unix/native/libjli/java_md_common.c line 85:
> 83: const char *execname = GetExecName();
> 84: if (execname != NULL) {
> 85: JLI_TraceLauncher("Launcher executable path is %s\n", execname);
This trace seems a bit unsymmetric to its Windows counterpart. Maybe it should
be left out here, too, since there is tracing in GetJREPath.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1572086726
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v3]
On Thu, 18 Apr 2024 06:31:03 GMT, Alan Bateman wrote: >> Hi Christoph, seems the USE_REGISTRY_LOOKUP is currently unused (at least >> without additional defines that are not present usually) >> >> src/java.base/windows/native/libjli/java_md.c:52:#ifdef USE_REGISTRY_LOOKUP >> src/java.base/windows/native/libjli/java_md.c:333:#ifdef USE_REGISTRY_LOOKUP >> >> >> I am not sure if this even works any more. Maybe Alan could comment on this >> ? > > @MBaesken I checked into the building with -DUSE_REGISTRY_LOOKUP as that > compiled in code that the Oracle installer needed when it copied java.exe to > somewhere. This is indeed left over code and can be removed from java_md.c. Hi Alan, thanks for checking this ! I removed the USE_REGISTRY_LOOKUP usages. - PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1570109527
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v3]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: remove obsolete USE_REGISTRY_LOOKUP usages - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/344d1b89..ddb3e0fe Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=02 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=01-02 Stats: 12 lines in 1 file changed: 0 ins; 12 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v2]
On Tue, 16 Apr 2024 14:29:13 GMT, Matthias Baesken wrote:
>> src/java.base/windows/native/libjli/java_md.c line 326:
>>
>>> 324: }
>>> 325:
>>> 326: JLI_TraceLauncher("GetJREPath - attempt to get JRE location from
>>> shared lib of the image\n");
>>
>> Maybe add a trace also in the USE_REGISTRY_LOOKUP section
>
> Hi Christoph, seems the USE_REGISTRY_LOOKUP is currently unused (at least
> without additional defines that are not present usually)
>
> src/java.base/windows/native/libjli/java_md.c:52:#ifdef USE_REGISTRY_LOOKUP
> src/java.base/windows/native/libjli/java_md.c:333:#ifdef USE_REGISTRY_LOOKUP
>
>
> I am not sure if this even works any more. Maybe Alan could comment on this ?
@MBaesken I checked into the building with -DUSE_REGISTRY_LOOKUP as that
compiled in code that the Oracle installer needed when it copied java.exe to
somewhere. This is indeed left over code and can be removed from java_md.c.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1570072071
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing [v2]
> We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust trace messages - Changes: - all: https://git.openjdk.org/jdk/pull/18699/files - new: https://git.openjdk.org/jdk/pull/18699/files/128300a0..344d1b89 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18699=01 - incr: https://webrevs.openjdk.org/?repo=jdk=18699=00-01 Stats: 10 lines in 3 files changed: 0 ins; 4 del; 6 mod Patch: https://git.openjdk.org/jdk/pull/18699.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18699/head:pull/18699 PR: https://git.openjdk.org/jdk/pull/18699
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 9 Apr 2024 15:28:08 GMT, Matthias Baesken wrote:
> We have already good JLI tracing capabilities. But GetApplicationHome and
> GetApplicationHomeFromDll lack some tracing and should be enhanced.
I adjusted the trace messages a bit to make the coding more consistent to the
existing Jli trace code,
I removed the print of the function name because this is usually avoided in the
existing cases.
I removed the 'did not succeed' Jli trace messages in GetJREPath because in
case of success we would getJli trace output ('JRE path is ') so
missing success output means it failed.
-
PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2061561008
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 14:29:13 GMT, Matthias Baesken wrote: > I am not sure if this even works any more. Maybe Alan could comment on this ? The GetPublicJREHome function was removed at some point, I think JDK 9, as it didn't make sense to have in the OpenJDK project. However, Oracle installer did depend on it and will require a bit of effort to see if removing it now will cause an issue now or not. - PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1567595933
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 10:20:23 GMT, Alan Bateman wrote: > I think this is way too ad hoc and looks like lefts over from a debugging > session. So I don't think it should be integrated without stepping back and > thinking more about what this tracing option is intended for. Currently there seem to be two (with the unused registry stuff 3, if it is still valid ?) approaches to find the 'JRE' location (should this better be named image location?) - using the launcher exe location and using the location of the libjli shared lib. Unfortunately those approaches are not well supported by JLI-traces , this is what the change is about. We could probably adjust/reduce the added tracing code but I would like to know a bit more about the concerns raised. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2059252352
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 09:09:00 GMT, Christoph Langer wrote:
>> We have already good JLI tracing capabilities. But GetApplicationHome and
>> GetApplicationHomeFromDll lack some tracing and should be enhanced.
>
> src/java.base/windows/native/libjli/java_md.c line 326:
>
>> 324: }
>> 325:
>> 326: JLI_TraceLauncher("GetJREPath - attempt to get JRE location from
>> shared lib of the image\n");
>
> Maybe add a trace also in the USE_REGISTRY_LOOKUP section
Hi Christoph, seems the USE_REGISTRY_LOOKUP is currently unused (at least
without additional defines that are not present usually)
src/java.base/windows/native/libjli/java_md.c:52:#ifdef USE_REGISTRY_LOOKUP
src/java.base/windows/native/libjli/java_md.c:333:#ifdef USE_REGISTRY_LOOKUP
I am not sure if this even works any more. Maybe Alan could comment on this ?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1567466784
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 9 Apr 2024 15:28:08 GMT, Matthias Baesken wrote: > We have already good JLI tracing capabilities. But GetApplicationHome and > GetApplicationHomeFromDll lack some tracing and should be enhanced. I think this is way too ad hoc and looks like lefts over from a debugging session. So I don't think it should be integrated without stepping back and thinking more about what this tracing option is intended for. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2058746338
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 16 Apr 2024 09:14:50 GMT, Christoph Langer wrote: > > What exactly do you see as inconsistent ? > > Maybe the output of the tracing should look similar to other traces done > through `JLI_TraceLauncher`? E.g. not mention method names but just tell what > the program is doing... ? Okay, makes sense ! I will change that to make the output similar to existing traces. - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2058627174
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Mon, 15 Apr 2024 12:47:08 GMT, Matthias Baesken wrote: > > If we expand the tracing then I think it should be consistent with the > > existing tracing. > > What exactly do you see as inconsistent ? Maybe the output of the tracing should look similar to other traces done through `JLI_TraceLauncher`? E.g. not mention method names but just tell what the program is doing... ? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2058621283
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Tue, 9 Apr 2024 15:28:08 GMT, Matthias Baesken wrote:
> We have already good JLI tracing capabilities. But GetApplicationHome and
> GetApplicationHomeFromDll lack some tracing and should be enhanced.
To me this looks useful, although maybe the overall JLI tracing could be
revisited.
src/java.base/windows/native/libjli/java_md.c line 326:
> 324: }
> 325:
> 326: JLI_TraceLauncher("GetJREPath - attempt to get JRE location from
> shared lib of the image\n");
Maybe add a trace also in the USE_REGISTRY_LOOKUP section
-
Marked as reviewed by clanger (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18699#pullrequestreview-2003079325
PR Review Comment: https://git.openjdk.org/jdk/pull/18699#discussion_r1567018779
Re: RFR: 8329862: GetApplicationHome and GetApplicationHomeFromDll enhance jli tracing
On Wed, 10 Apr 2024 07:16:49 GMT, Matthias Baesken wrote: > If we expand the tracing then I think it should be consistent with the > existing tracing. What exactly do you see as inconsistent ? - PR Comment: https://git.openjdk.org/jdk/pull/18699#issuecomment-2056772309
