Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8291454: Missing check for JLI C runtime library in CoreLibraries.gmk #9669

Closed
wants to merge 2 commits into from

Conversation

TheShermanTanker
Copy link
Contributor

@TheShermanTanker TheShermanTanker commented Jul 28, 2022

CoreLibraries.gmk is missing a check for MSVCR_DLL. This results in a defined, but empty macro in libjli if it is not defined in the build system, which is incorrect


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-8291454: Missing check for JLI C runtime library in CoreLibraries.gmk

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9669

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

Using diff file

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

@TheShermanTanker TheShermanTanker marked this pull request as draft July 28, 2022 02:20
@bridgekeeper
Copy link

bridgekeeper bot commented Jul 28, 2022

👋 Welcome back jwaters! 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 openjdk bot changed the title 8291454 8291454: Missing check for JLI C runtime library in CoreLibraries.gmk Jul 28, 2022
@openjdk
Copy link

openjdk bot commented Jul 28, 2022

@TheShermanTanker The following labels will be automatically applied to this pull request:

  • build
  • core-libs

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

@openjdk openjdk bot added build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Jul 28, 2022
@TheShermanTanker TheShermanTanker marked this pull request as ready for review July 28, 2022 11:27
@openjdk openjdk bot added the rfr Pull request is ready for review label Jul 28, 2022
@mlbridge
Copy link

mlbridge bot commented Jul 28, 2022

Webrevs

@TheShermanTanker
Copy link
Contributor Author

/integrate

@openjdk
Copy link

openjdk bot commented Jul 28, 2022

@TheShermanTanker 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:

8291454: Missing check for JLI C runtime library in CoreLibraries.gmk

Reviewed-by: erikj, ihse

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

  • 5a539e8: 8291893: riscv: remove fence.i used in user space
  • b2f0cbd: 8064787: [macosx] In Nimbus LaF, Ctrl+Alt mnemonic doesn't work
  • 891df21: 8259687: JTabbedPane.setComponentAt doesn't hide previously visible tab component
  • d4fb91b: 8291956: Simplify the loop condition in sun.net.httpserver.Request constructor
  • 20123ea: 8291901: IGV: Preference menu disappears after JDK-8288750

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 (@erikj79, @magicus) 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 openjdk bot added ready Pull request is ready to be integrated sponsor Pull request is ready to be sponsored labels Jul 28, 2022
@openjdk
Copy link

openjdk bot commented Jul 28, 2022

@TheShermanTanker
Your change (at version 4fb3424) is now ready to be sponsored by a Committer.

@TheShermanTanker
Copy link
Contributor Author

Comment to keep thread alive

@TheShermanTanker
Copy link
Contributor Author

Merged with latest, currently awaiting further review and/or sponsorship

@openjdk openjdk bot removed the sponsor Pull request is ready to be sponsored label Aug 8, 2022
@magicus
Copy link
Member

magicus commented Aug 8, 2022

/sponsor

@openjdk
Copy link

openjdk bot commented Aug 8, 2022

@magicus The PR has been updated since the change author (@TheShermanTanker) issued the integrate command - the author must perform this command again.

@TheShermanTanker
Copy link
Contributor Author

/integrate

@magicus
Copy link
Member

magicus commented Aug 8, 2022

/sponsor

@openjdk openjdk bot added the sponsor Pull request is ready to be sponsored label Aug 8, 2022
@openjdk
Copy link

openjdk bot commented Aug 8, 2022

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

@openjdk
Copy link

openjdk bot commented Aug 8, 2022

Going to push as commit 8a804f6.
Since your change was applied there have been 5 commits pushed to the master branch:

  • 5a539e8: 8291893: riscv: remove fence.i used in user space
  • b2f0cbd: 8064787: [macosx] In Nimbus LaF, Ctrl+Alt mnemonic doesn't work
  • 891df21: 8259687: JTabbedPane.setComponentAt doesn't hide previously visible tab component
  • d4fb91b: 8291956: Simplify the loop condition in sun.net.httpserver.Request constructor
  • 20123ea: 8291901: IGV: Preference menu disappears after JDK-8288750

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Aug 8, 2022
@openjdk openjdk bot closed this Aug 8, 2022
@openjdk 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 Aug 8, 2022
@openjdk
Copy link

openjdk bot commented Aug 8, 2022

@magicus @TheShermanTanker Pushed as commit 8a804f6.

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

@TheShermanTanker TheShermanTanker deleted the patch-1 branch August 8, 2022 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org integrated Pull request has been integrated
3 participants