8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging" #434

ktakakuri · 2024-01-26T05:33:52Z

Hi all, This is a backport of JDK-8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging".
I worked on backporting JDK-8159695 to jdk8u ( #428 ), and found that VS2010 build fails. This is because strtoull function, which the JDK-8159695 patch contains, is not predefined in VS2010. This patch defines strtoull macro when the earlier versions of Visual Studio are used, so this patch need to be backported for builing with VS2010 if JDK-8159695 is backported as proposed.

This patch couldn't be applied cleanly due to a previously backported fix for earlier versions of Visual Studio at the same location, which needs to be merged.

The risk is low as this only affects earlier VS builds and introduces one macro.
Testing: GHA

Thank you.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
JDK-8137329 needs maintainer approval
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging" (Bug - P4 - Approved)

Reviewers

Thomas Stuefe (@tstuefe - Reviewer) ⚠️ Review applies to 7b175458
Andrew John Hughes (@gnu-andrew - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/434/head:pull/434
$ git checkout pull/434

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 434

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/434.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-01-26T05:34:24Z

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

openjdk · 2024-01-26T05:35:42Z

This backport pull request has now been updated with issue from the original commit.

mlbridge · 2024-01-26T05:38:39Z

Webrevs

tstuefe

Okay.

tstuefe · 2024-01-26T16:43:09Z

Please look at the test errors in GHAs and check them. Provided they have nothing to do with the patch, my approval still stands.

bridgekeeper · 2024-02-23T20:00:52Z

@ktakakuri This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

openjdk · 2024-03-13T20:05:00Z

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

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

8137329: [windows] Build broken on VS2010 after  "8046148: JEP 158: Unified JVM Logging"

Reviewed-by: stuefe, andrew

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 33 new commits pushed to the master branch:

b499ea7: 8030204: com/sun/jdi/JdbExprTest.sh: Required output "Can\'t convert 2147483648 to int" not found
b1e2ea8: 8279164: Disable TLS_ECDH_* cipher suites
6abb3f2: 8035395: sun/management/jmxremote/startstop/JMXStartStopTest.java fails intermittently: Port already in use
6838605: 8311666: Disabled tests in test/jdk/sun/java2d/marlin
bfb1a7d: 4660158: TTY: NumberFormatException while trying to set values by 'set' command
0357abb: 8324632: Update Zlib Data Compression Library to Version 1.3.1
61dde50: 8333724: Problem list security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1
8892b81: 8326521: JFR: CompilerPhase event test fails on windows 32 bit
9da7380: 8331730: [8u] GHA: update sysroot for cross builds to Debian bullseye
9a7aa79: 8333669: [8u] GHA: Dead VS2010 download link
... and 23 more: https://git.openjdk.org/jdk8u-dev/compare/1db6a7606c297fd87d5f143cac40e7652b94404c...master

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 (@tstuefe, @gnu-andrew) 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).

openjdk · 2024-04-08T12:20:50Z

⚠️ @ktakakuri This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

ktakakuri · 2024-04-16T14:30:18Z

Some tests are failing in GHA, but they are irrelevant as they are related to certification or GC.

ktakakuri · 2024-04-16T14:36:19Z

/approval request Fix Request 8u
Backporting JDK-8159695 requires this patch. The patch applies with trivial conflict resolution. Low risk, defining macro.
Tesing: GHA, and hotspot_tier1.

openjdk · 2024-04-16T14:37:01Z

@ktakakuri
8137329: The approval request has been created successfully.

bridgekeeper · 2024-05-14T17:33:59Z

@ktakakuri This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper · 2024-06-11T18:25:11Z

@ktakakuri This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

ktakakuri · 2024-06-11T23:30:14Z

/open

openjdk · 2024-06-11T23:31:23Z

@ktakakuri The target branch of this pull request no longer exists. Please retarget this pull request. Please issue this command again once the problem has been resolved.

ktakakuri · 2024-06-11T23:35:19Z

/open

openjdk · 2024-06-11T23:36:07Z

@ktakakuri This pull request is now open

gnu-andrew · 2024-06-15T17:44:23Z

Given #428 breaks the build because this function is not present on VS2010, I think this change should go in before #428, not after.

@ktakakuri can you update this PR to apply against latest master, so we can get it in? Thanks.

ktakakuri · 2024-06-17T06:26:39Z

I changed the branch to merge into to master.

This reverts commit b62ee29.

ktakakuri · 2024-06-18T07:33:55Z

I reverted JDK-8159695 fix.

gnu-andrew

Thanks for updating. Change looks good to me.

gnu-andrew · 2024-06-25T12:04:57Z

/approve yes

openjdk · 2024-06-25T12:08:04Z

@gnu-andrew
8137329: The approval request has been approved.

ktakakuri · 2024-06-26T00:21:45Z

/integrate

openjdk · 2024-06-26T00:22:05Z

@ktakakuri
Your change (at version ba63f36) is now ready to be sponsored by a Committer.

phohensee · 2024-06-27T17:33:51Z

/sponsor

openjdk · 2024-06-27T17:44:34Z

Going to push as commit 44eac48.
Since your change was applied there have been 34 commits pushed to the master branch:

a0715ab: 8238274: (sctp) JDK-7118373 is not fixed for SctpChannel
b499ea7: 8030204: com/sun/jdi/JdbExprTest.sh: Required output "Can\'t convert 2147483648 to int" not found
b1e2ea8: 8279164: Disable TLS_ECDH_* cipher suites
6abb3f2: 8035395: sun/management/jmxremote/startstop/JMXStartStopTest.java fails intermittently: Port already in use
6838605: 8311666: Disabled tests in test/jdk/sun/java2d/marlin
bfb1a7d: 4660158: TTY: NumberFormatException while trying to set values by 'set' command
0357abb: 8324632: Update Zlib Data Compression Library to Version 1.3.1
61dde50: 8333724: Problem list security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1
8892b81: 8326521: JFR: CompilerPhase event test fails on windows 32 bit
9da7380: 8331730: [8u] GHA: update sysroot for cross builds to Debian bullseye
... and 24 more: https://git.openjdk.org/jdk8u-dev/compare/1db6a7606c297fd87d5f143cac40e7652b94404c...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-06-27T17:44:57Z

@phohensee @ktakakuri Pushed as commit 44eac48.

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

ktakakuri added 2 commits January 25, 2024 14:47

Backport 992b6c464a9655438a808cf2e1979e04356123be

b62ee29

Backport 6c113577e63d359a78ed72e5f37919da26b2f20d

7b17545

openjdk bot changed the title ~~Backport 6c113577e63d359a78ed72e5f37919da26b2f20d~~ 8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging" Jan 26, 2024

openjdk bot added backport rfr Pull request is ready for review labels Jan 26, 2024

tstuefe approved these changes Jan 26, 2024

View reviewed changes

Merge branch 'master' into 8137329

5fbb88b

ktakakuri mentioned this pull request Apr 16, 2024

8159695: Arguments::atojulong() fails to detect overflows #428

Open

4 tasks

openjdk bot added the approval label Apr 16, 2024

bridgekeeper bot closed this Jun 11, 2024

openjdk bot reopened this Jun 11, 2024

ktakakuri changed the base branch from pr/428 to master June 17, 2024 05:43

Revert "Backport 992b6c464a9655438a808cf2e1979e04356123be"

ba63f36

This reverts commit b62ee29.

gnu-andrew approved these changes Jun 25, 2024

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated and removed approval labels Jun 25, 2024

openjdk bot added the sponsor Pull request is ready to be sponsored label Jun 26, 2024

openjdk bot added the integrated Pull request has been integrated label Jun 27, 2024

openjdk bot closed this Jun 27, 2024

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 Jun 27, 2024

8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging" #434

8137329: [windows] Build broken on VS2010 after "8046148: JEP 158: Unified JVM Logging" #434

Uh oh!

Conversation

ktakakuri commented Jan 26, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Jan 26, 2024

Uh oh!

openjdk bot commented Jan 26, 2024

Uh oh!

mlbridge bot commented Jan 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

tstuefe left a comment

Choose a reason for hiding this comment

Uh oh!

tstuefe commented Jan 26, 2024

Uh oh!

bridgekeeper bot commented Feb 23, 2024

Uh oh!

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

Uh oh!

Uh oh!

openjdk bot commented Apr 8, 2024

Uh oh!

ktakakuri commented Apr 16, 2024

Uh oh!

ktakakuri commented Apr 16, 2024

Uh oh!

openjdk bot commented Apr 16, 2024

Uh oh!

bridgekeeper bot commented May 14, 2024

Uh oh!

bridgekeeper bot commented Jun 11, 2024

Uh oh!

ktakakuri commented Jun 11, 2024

Uh oh!

openjdk bot commented Jun 11, 2024

Uh oh!

ktakakuri commented Jun 11, 2024

Uh oh!

openjdk bot commented Jun 11, 2024

Uh oh!

gnu-andrew commented Jun 15, 2024

Uh oh!

ktakakuri commented Jun 17, 2024

Uh oh!

ktakakuri commented Jun 18, 2024

Uh oh!

gnu-andrew left a comment

Choose a reason for hiding this comment

Uh oh!

gnu-andrew commented Jun 25, 2024

Uh oh!

openjdk bot commented Jun 25, 2024

Uh oh!

ktakakuri commented Jun 26, 2024

Uh oh!

openjdk bot commented Jun 26, 2024

Uh oh!

phohensee commented Jun 27, 2024

Uh oh!

openjdk bot commented Jun 27, 2024

Uh oh!

openjdk bot commented Jun 27, 2024

Uh oh!

Uh oh!

ktakakuri commented Jan 26, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Jan 26, 2024 •

edited

Loading

openjdk bot commented Mar 13, 2024 •

edited

Loading