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

8260518: Change default -mmacosx-version-min to 10.12 #810

Closed
wants to merge 2 commits into from

Conversation

VladimirKempik
Copy link

@VladimirKempik VladimirKempik commented Feb 8, 2022

Please review this backport of JDK-8260518 to jdk11u-dev
The backport is not applying clean due to the context code difference.
It's needed mostly to resolve this compilation error on macos-aarch64
splashscreen_sys.m:274:39: error: 'NSAlphaFirstBitmapFormat' is deprecated: first deprecated in macOS 10.14 [-Werror,-Wdeprecated-declarations]
This will allow to add macos-aarch64 build into github actions for jdk11u-dev ( once jep-391 backport is integrated)


Progress

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

Issue

  • JDK-8260518: Change default -mmacosx-version-min to 10.12

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 810

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

Using diff file

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

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Feb 8, 2022

👋 Welcome back vkempik! 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 Backport 53f1b93881a6404508628388ba590fe1f7f958ff 8260518: Change default -mmacosx-version-min to 10.12 Feb 8, 2022
@openjdk
Copy link

@openjdk openjdk bot commented Feb 8, 2022

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

@openjdk openjdk bot added backport rfr labels Feb 8, 2022
@mlbridge
Copy link

@mlbridge mlbridge bot commented Feb 8, 2022

Webrevs

Copy link
Member

@phohensee phohensee left a comment

Lgtm.

Note to Maintainers: 10.9 through 10.14 are eol, and the oldest machines supported by 10.12 are from 2009/2010, see https://support.apple.com/kb/SP742?locale=en_USh. Imo it's safe to say that it's very unlikely there are machines running OSX versions earlier than 10.12.

@openjdk
Copy link

@openjdk openjdk bot commented Feb 8, 2022

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

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

8260518: Change default -mmacosx-version-min to 10.12

Reviewed-by: phh, burban

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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 added the ready label Feb 8, 2022
@VladimirKempik
Copy link
Author

@VladimirKempik VladimirKempik commented Feb 8, 2022

Thanks Paul.

@lewurm
Copy link
Member

@lewurm lewurm commented Feb 14, 2022

Note to Maintainers: 10.9 through 10.14 are eol, and the oldest machines supported by 10.12 are from 2009/2010, see https://support.apple.com/kb/SP742?locale=en_USh. Imo it's safe to say that it's very unlikely there are machines running OSX versions earlier than 10.12.

While I agree, why take the risk? This can be easily set conditionally for aarch64.

@VladimirKempik
Copy link
Author

@VladimirKempik VladimirKempik commented Feb 14, 2022

Note to Maintainers: 10.9 through 10.14 are eol, and the oldest machines supported by 10.12 are from 2009/2010, see https://support.apple.com/kb/SP742?locale=en_USh. Imo it's safe to say that it's very unlikely there are machines running OSX versions earlier than 10.12.

While I agree, why take the risk? This can be easily set conditionally for aarch64.

This was reasoned in the bug description:

Having a large gap between the target versions becomes problematic as we hit a lot of deprecation warnings in shared code. To be able to fix these deprecation warnings, we need a smaller version gap.

lewurm
lewurm approved these changes Feb 14, 2022
@VladimirKempik
Copy link
Author

@VladimirKempik VladimirKempik commented Feb 23, 2022

@GoeLin, two weeks later - no objections ( https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2022-February/012026.html )
can we get approval or as an alternative, I can reduce this fix and not change min macos version for intel64.
I would like to make macos-aarch64 port buildable without --disable-warnings-as-errors before rdp2.

@lewurm
Copy link
Member

@lewurm lewurm commented Mar 2, 2022

Can this be merged?

@phohensee
Copy link
Member

@phohensee phohensee commented Mar 2, 2022

The JBS issue has been tagged with jdk11u-fix-yes, so yes, it can be merged.

@VladimirKempik
Copy link
Author

@VladimirKempik VladimirKempik commented Mar 2, 2022

/integrate

@openjdk
Copy link

@openjdk openjdk bot commented Mar 2, 2022

Going to push as commit 5561fd1.
Since your change was applied there have been 20 commits pushed to the master branch:

  • 16d98b3: 8282501: Bump update version for OpenJDK: jdk-11.0.16
  • 5cad68f: 8255410: Add ChaCha20 and Poly1305 support to SunPKCS11 provider
  • c0effc2: 8211333: AArch64: Fix another build failure after JDK-8211029
  • 80919eb: 8279669: test/jdk/com/sun/jdi/TestScaffold.java uses wrong condition
  • ba6c4c1: 8261107: ArrayIndexOutOfBoundsException in the ICC_Profile.getInstance(InputStream)
  • 80a2777: 8282372: [11] build issue on MacOS/aarch64 12.2.1 using Xcode 13.1: call to 'log2_intptr' is ambiguous
  • 68c6320: 8214004: Missing space between compiler thread name and task info in hs_err
  • 16cbd32: 8250750: JDK-8247515 fix for OSX pc_to_symbol() lookup fails with some symbols
  • 6f9d287: 8277488: Add expiry exception for Digicert (geotrustglobalca) expiring in May 2022
  • ffa5ae8: 8247515: OSX pc_to_symbol() lookup does not work with core files
  • ... and 10 more: https://git.openjdk.java.net/jdk11u-dev/compare/eb0708f75aa3c196e41014addfaa5667fd940cc2...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated label Mar 2, 2022
@openjdk openjdk bot closed this Mar 2, 2022
@openjdk openjdk bot removed ready rfr labels Mar 2, 2022
@openjdk
Copy link

@openjdk openjdk bot commented Mar 2, 2022

@VladimirKempik Pushed as commit 5561fd1.

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

@lewurm
Copy link
Member

@lewurm lewurm commented Mar 2, 2022

Thank you Paul and Vladimir!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport integrated
3 participants