Skip to content

Conversation

MBaesken
Copy link
Member

@MBaesken MBaesken commented Oct 1, 2025

In the build process, unwanted UL error/warning messages can pollute stdout and influence the build process/results.

An example is the polluted blocked.certs file from https://bugs.openjdk.org/browse/JDK-8357691 .


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8368960: Adjust java UL logging in the build (Bug - P4)

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27588/head:pull/27588
$ git checkout pull/27588

Update a local copy of the PR:
$ git checkout pull/27588
$ git pull https://git.openjdk.org/jdk.git pull/27588/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27588

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27588.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Oct 1, 2025

👋 Welcome back mbaesken! 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
Copy link

openjdk bot commented Oct 1, 2025

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

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

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

8368960: Adjust java UL logging in the build

Reviewed-by: erikj

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 24 new commits pushed to 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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot changed the title JDK-8368960: Adjust java UL logging in the build 8368960: Adjust java UL logging in the build Oct 1, 2025
@openjdk openjdk bot added the build build-dev@openjdk.org label Oct 1, 2025
@openjdk
Copy link

openjdk bot commented Oct 1, 2025

@MBaesken The following label will be automatically applied to this pull request:

  • build

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the rfr Pull request is ready for review label Oct 1, 2025
@mlbridge
Copy link

mlbridge bot commented Oct 1, 2025

Webrevs

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Oct 1, 2025
@dholmes-ora
Copy link
Member

@MBaesken I thought the aim was to redirect log warnings/errors to stderr, not to disable logging completely.

@MBaesken
Copy link
Member Author

MBaesken commented Oct 2, 2025

I thought the aim was to redirect log warnings/errors to stderr, not to disable logging completely.

Yeah I considered this too.
But not sure about it ; bring the UL messages (warnings/errors etc.) really a benefit in the build itself ?
But I am open to set some other UL command line flags keeping messages .

@erikj79
Copy link
Member

erikj79 commented Oct 2, 2025

I thought the aim was to redirect log warnings/errors to stderr, not to disable logging completely.

Yeah I considered this too. But not sure about it ; bring the UL messages (warnings/errors etc.) really a benefit in the build itself ? But I am open to set some other UL command line flags keeping messages .

If it's possible to redirect it to stderr, I think that would be preferred. It would make us aware of misconfigured tool command lines.

In the case of the original warning you saw that triggered this, I think we need to actually fix our usage of CDS in the boot JDK. The current implementation was introduced back when the JDK didn't ship with a default CDS archive. Now that it does, we don't actually need to create one in configure. It would be interesting to see if there is any performance difference using the default archive.

@MBaesken
Copy link
Member Author

MBaesken commented Oct 2, 2025

If it's possible to redirect it to stderr, I think that would be preferred. It would make us aware of misconfigured tool command lines.

I adjusted the settings, please comment if this is better.

@openjdk openjdk bot removed the ready Pull request is ready to be integrated label Oct 2, 2025
@MBaesken
Copy link
Member Author

MBaesken commented Oct 2, 2025

In the case of the original warning you saw that triggered this, I think we need to actually fix our usage of CDS in the boot JDK

So do you think we can just kick out this archive dumping + Xshare=something setting from configure ?

@erikj79
Copy link
Member

erikj79 commented Oct 2, 2025

In the case of the original warning you saw that triggered this, I think we need to actually fix our usage of CDS in the boot JDK

So do you think we can just kick out this archive dumping + Xshare=something setting from configure ?

Yes, I think we can, but I would like to see it verified to not cause a regression in build performance before we do.

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Oct 2, 2025
# Force en-US environment
UTIL_ADD_JVM_ARG_IF_OK([-Duser.language=en -Duser.country=US],boot_jdk_jvmargs,[$JAVA])
UTIL_ADD_JVM_ARG_IF_OK([-Xlog:all=warning:stderr],boot_jdk_jvmargs,[$JAVA])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not sufficient you have to disable the warnings on stdout, not just enable them on stderr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org ready Pull request is ready to be integrated rfr Pull request is ready for review
Development

Successfully merging this pull request may close these issues.

3 participants