8281814: Debuginfo.diz contains redundant build path after backport JDK-8025936 #26

dongbohe · 2022-03-30T07:45:08Z

Hi,

I would like to backport 8035134 to fix debuginfo regression, introduced by JDK-8025936.
As discussed on JDK-8281814, we cannot see the details of the original patch.
It is only a different context, no risk.

Testing: worked correctly after patch.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8281814: Debuginfo.diz contains redundant build path after backport JDK-8025936

Reviewers

Severin Gehwolf (@jerboaa - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk8u-dev pull/26/head:pull/26
$ git checkout pull/26

Update a local copy of the PR:
$ git checkout pull/26
$ git pull https://git.openjdk.java.net/jdk8u-dev pull/26/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26

View PR using the GUI difftool:
$ git pr show -t 26

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk8u-dev/pull/26.diff

bridgekeeper · 2022-03-30T07:45:36Z

👋 Welcome back dongbohe! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2022-03-30T07:46:47Z

⚠️ @dongbohe the issue with id 8035134 from commit ffbb7125b25a88f4495d94c9c8fb25af89af1856 does not exist in project JDK.

dongbohe · 2022-03-31T07:08:22Z

@jerboaa Hi, severin. Can you help me look at this? It looks like the bot can't recognize original issue.

jerboaa · 2022-03-31T08:17:21Z

@jerboaa Hi, severin. Can you help me look at this? It looks like the bot can't recognize original issue.

@dongbohe The original bug is not publicly accessible. I think the way to work around this is to use PR title: Backport JDK-8035134 See https://bugs.openjdk.java.net/browse/SKARA-1076. I'm not sure it'll work as this was added for closed backports where the <sha> is not accessible, but the bug actually is. This is a different issue. If it fails we should just create a 8u-only bug which mentions the closed bug via the summary so it'll be discoverable if somebody does a git log --grep <bug-id> on the repo.

openjdk · 2022-03-31T08:32:42Z

⚠️ @dongbohe the issue with id 8035134 does not exist in project JDK.

jerboaa · 2022-03-31T08:35:08Z

Doesn't seem to work. Use JDK-8281814 and add a /summary to mention that it's actually a backport of JDK-8035134.

dongbohe · 2022-03-31T08:41:41Z

/summary 8u backport of JDK-8035134

openjdk · 2022-03-31T08:42:42Z

@dongbohe Setting summary to 8u backport of JDK-8035134

mlbridge · 2022-03-31T08:46:09Z

Webrevs

00: Full (104cb9ea)

jerboaa

LGTM

openjdk · 2022-03-31T08:59:59Z

@dongbohe This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8281814: Debuginfo.diz contains redundant build path after backport JDK-8025936

8u backport of JDK-8035134

Reviewed-by: sgehwolf

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 3 new commits pushed to the master branch:

c5ca29f: 8255239: The timezone of the hs_err_pid log file is corrupted in Japanese locale
e403fd5: 8274751: Drag And Drop hangs on Windows
f8a6695: 8274658: ISO 4217 Amendment 170 Update

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@jerboaa) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

dongbohe · 2022-04-06T09:27:57Z

Got the push approval.
/integrate

openjdk · 2022-04-06T09:28:59Z

@dongbohe
Your change (at version 104cb9e) is now ready to be sponsored by a Committer.

RealFYang · 2022-04-06T12:14:06Z

/sponsor

openjdk · 2022-04-06T12:15:50Z

Going to push as commit 62bbb3e.
Since your change was applied there have been 3 commits pushed to the master branch:

c5ca29f: 8255239: The timezone of the hs_err_pid log file is corrupted in Japanese locale
e403fd5: 8274751: Drag And Drop hangs on Windows
f8a6695: 8274658: ISO 4217 Amendment 170 Update

Your commit was automatically rebased without conflicts.

openjdk · 2022-04-06T12:16:13Z

@RealFYang @dongbohe Pushed as commit 62bbb3e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

gnu-andrew · 2022-05-12T17:54:36Z

This commit seems to have broken the Mac OS build

gmake[2]: *** [lib/CoreLibraries.gmk:109: /Users/runner/work/jdk8u-dev/jdk8u-dev/jdk/build/macos-x64/jdk/objs/libverify/libverify.diz] Error 12
zip error: Nothing to do! (/Users/runner/work/jdk8u-dev/jdk8u-dev/jdk/build/macos-x64/jdk/objs/libverify/libverify.diz)

I'm guessing there is now no argument to zip. I'm going to look if there is a further follow-up commit in later JDKs.

@dongbohe while this one looks to have gone in before GHA support was added, and so missed a run on MacOS run, can you make sure you have actions enabled for future PRs? Thanks.

gnu-andrew · 2022-05-13T03:12:55Z

It looks like it is largely reverted again in this 11u commit. I'll look at backporting just the changes to that file, as the rest is intrusive or not relevant.

On Mac, $1_DEBUGINFO_FILES contains just a directory, $$($1_OBJECT_DIR)/$$($1_BASENAME).dSYM and so ends up empty when this is filtered out. That patch changes the MacOS setup of $1_DEBUGINFO_FILES to refer to files.

It's not clear to me why this notdir fix was applied in the first place. The subst change in the later commit was identified in JDK-8281814 but not applied.

dongbohe · 2022-05-13T06:21:52Z

@gnu-andrew I have enabled actions for future PRs and will make sure to pass pre-commit tests before submitting PRs.

It looks like it is largely reverted again in this 11u commit. I'll look at backporting just the changes to that file, as the rest is intrusive or not relevant.

thanks for looking into this.

It's not clear to me why this notdir fix was applied in the first place. The subst change in the later commit was identified in JDK-8281814 but not applied.

From Contribute to jdk8u, it is recommended to use the patch from the repository of the JDK version closest to 8u to minimise changes, so I used notdir change from the earlier commit to fix the problem.

dongbohe · 2022-05-17T03:59:43Z

Hi, @gnu-andrew I have reproduced this problem on a MAC, here is the statements that produced the error:

cd /Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libnpt && /usr/bin/zip -q /Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libnpt/libnpt.diz Info.plist libnpt.dylib && /usr/bin/zip -q /Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libnpt/libnpt.diz Info.plist libnpt.dylib

$ cd /Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libnpt
$ pwd
/Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libnpt
$ ls 
libnpt.dylib.dSYM npt.d             npt.o             utf.d             utf.o             utf_md.d          utf_md.o
$ tree libnpt.dylib.dSYM
libnpt.dylib.dSYM
└── Contents
    ├── Info.plist
    └── Resources
        └── DWARF
            └── libnpt.dylib

3 directories, 2 files
$ /usr/bin/zip -q /Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libjli_static/libjli_static.diz Info.plist libjli_static.dylib
zip error: Nothing to do! (/Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libjli_static/libjli_static.diz)

So, I think we can fix this problem with subst, and GHA on mac was successful.

diff --git a/make/common/NativeCompilation.gmk b/make/common/NativeCompilation.gmk
index 98af6babc8..d1fc0c93a9 100644
--- a/make/common/NativeCompilation.gmk
+++ b/make/common/NativeCompilation.gmk
@@ -530,7 +530,7 @@ define SetupNativeCompilation
             # to be rebuilt properly.
             $$($1_DEBUGINFO_ZIP): $$($1_DEBUGINFO_FILES) $$($1_TARGET)
                $(CD) $$($1_OBJECT_DIR) \
-               && $(ZIP) -q $$@ $$(notdir $$($1_DEBUGINFO_FILES))
+               && $(ZIP) -q $$@ $$(subst $$($1_OBJECT_DIR)/,, $$($1_DEBUGINFO_FILES))
           endif
         else
           ifneq ($$($1_STRIP_POLICY), no_strip)

The statements and build result after using subst are as follows:

cd /Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libnpt && /usr/bin/zip -q /Users/hedongbo/myprojects/openjdk/jdk8u-dev/build/macosx-x86_64-normal-server-release/jdk/objs/libnpt/libnpt.diz libnpt.dylib.dSYM/Contents/Info.plist libnpt.dylib.dSYM/Contents/Resources/DWARF/libnpt.dylib
$ unzip libnpt.diz
Archive:  libnpt.diz
 inflating: libnpt.dylib.dSYM/Contents/Info.plist
 inflating: libnpt.dylib.dSYM/Contents/Resources/DWARF/libnpt.dylib

This looks ok. What do you think？

Backport ffbb7125b25a88f4495d94c9c8fb25af89af1856

104cb9e

dongbohe changed the title ~~Backport ffbb7125b25a88f4495d94c9c8fb25af89af1856~~ Backport JDK-8035134 Mar 31, 2022

dongbohe changed the title ~~Backport JDK-8035134~~ 8281814: Debuginfo.diz contains redundant build path after backport JDK-8025936 Mar 31, 2022

openjdk bot added the rfr Pull request is ready for review label Mar 31, 2022

jerboaa approved these changes Mar 31, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 31, 2022

openjdk bot added the sponsor Pull request is ready to be sponsored label Apr 6, 2022

openjdk bot added the integrated Pull request has been integrated label Apr 6, 2022

openjdk bot closed this Apr 6, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Apr 6, 2022

gnu-andrew mentioned this pull request May 25, 2022

8286989: Build failure on macOS after 8281814 #61

Closed

3 tasks

8281814: Debuginfo.diz contains redundant build path after backport JDK-8025936 #26

8281814: Debuginfo.diz contains redundant build path after backport JDK-8025936 #26

Uh oh!

Conversation

dongbohe commented Mar 30, 2022 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Mar 30, 2022

Uh oh!

openjdk bot commented Mar 30, 2022

Uh oh!

dongbohe commented Mar 31, 2022

Uh oh!

jerboaa commented Mar 31, 2022

Uh oh!

openjdk bot commented Mar 31, 2022

Uh oh!

jerboaa commented Mar 31, 2022

Uh oh!

dongbohe commented Mar 31, 2022

Uh oh!

openjdk bot commented Mar 31, 2022

Uh oh!

mlbridge bot commented Mar 31, 2022

Webrevs

Uh oh!

jerboaa left a comment

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongbohe commented Apr 6, 2022

Uh oh!

openjdk bot commented Apr 6, 2022

Uh oh!

RealFYang commented Apr 6, 2022

Uh oh!

openjdk bot commented Apr 6, 2022

Uh oh!

openjdk bot commented Apr 6, 2022

Uh oh!

gnu-andrew commented May 12, 2022

Uh oh!

gnu-andrew commented May 13, 2022

Uh oh!

dongbohe commented May 13, 2022

Uh oh!

dongbohe commented May 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

dongbohe commented Mar 30, 2022 •

edited by openjdk bot

Loading

openjdk bot commented Mar 31, 2022 •

edited

Loading