On Fri, 14 Mar 2025 12:29:22 GMT, Christoph Langer <[email protected]> wrote:
> Alternative approach to #24012
>
> This keeps the current handling of *.pdb vs *.stripped.pdb which allows
> debugging at the cost of a little hack in jlink. Maybe the code in jlink can
> be improved, e.g. make it more conditional.
>
> I'm running this through our testing still to see whether it'll resolve all
> of the test issues and does not introduce regressions.
Could we change the synopsis of the bug to something like `Handle Windows
specific combination of JEP 493 and --with-external-symbols-in-bundles=public`?
make/Bundles.gmk line 248:
> 246: %.stripped.pdb, \
> 247: $(call FindFiles, $(SYMBOLS_IMAGE_DIR)) \
> 248: )
Why filter *.stripped.pdb? Because they are useless for debugging? Then it
should be mentioned as a comment.
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 220:
> 218: // Read from the base JDK image.
> 219: Path path = BASE.resolve(m.resPath);
> 220: // special case to handle stripped pdb files,
> e.g. --with-external-symbols-in-bundles=public
Suggestion:
// Windows debug symbols special case to handle
stripped pdb files. For example builds
// configured with
--with-external-symbols-in-bundles=public
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JRTArchive.java line 224:
> 222: String strippedFile = m.resPath.substring(0,
> m.resPath.lastIndexOf(".pdb")) + ".stripped.pdb";
> 223: Path strippedPath =
> BASE.resolve(strippedFile);
> 224: if (Files.exists(strippedPath)) {
Suggestion:
// With --with-external-symbols-in-bundles=public
configuration, stripped
// pdb files will be present in the image *in
addition* to non-stripped pdb
// files. Since the JMODs at build time only
include stripped pdb files
// (without '.stripped.' in their name), we need to
do the same here so that
// actually the same file content gets checked.
Remember: *.stripped.pdb
// files will only exist for builds configured with
// --with-external-symbols-in-bundles=public
configured builds. Exists check
// is present to allow for regular windows builds
as well.
if (Files.exists(strippedPath)) {
-------------
PR Review: https://git.openjdk.org/jdk/pull/24057#pullrequestreview-2685668451
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995654219
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995652710
PR Review Comment: https://git.openjdk.org/jdk/pull/24057#discussion_r1995642566
