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

Temporarily restore support for Win7 / Server 2008 R2 #4857

Merged
merged 6 commits into from
Jul 24, 2024

Conversation

StephanTLavavej
Copy link
Member

🗺️ Overview

My ultrabosses have decided that dropping support for targeting Win7 / Server 2008 R2 in VS 2022 17.12 is too aggressive, so I'm being required to restore it. However, dropping support has been approved for the 18.0 Preview 1 release (whenever that will happen, and whatever its major version branding will be), so this restoration is temporary.

Windows 7 and Server 2008 R2 are insecure operating systems, which haven't been receiving security updates for many months. Targeting them is making the world a worse place.

📜 Commits

  • Revert Drop support for Win7 / Server 2008 R2 #4742.
    • Resolve conflicts with Always use cmpxchg16b on x64 for atomic_ref<16 bytes> #4751:
    • In yvals_core.h, we dropped _STL_WIN32_WINNT_WIN7, then we dropped _STL_WIN32_WINNT_WINBLUE.
    • In atomic_wait.cpp, we dropped _ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE machinery, then we dropped __std_atomic_compare_exchange_128_fallback.
  • [cherry-pick] Cauterize usage of Win8 GetCurrentPackageId.
    • As previously explained:

      This is technically a bonus cleanup - instead of removing machinery dealing with the conditional absence of Win8 APIs, it's removing machinery dealing with the conditional presence of Win8 APIs.

    • We should preserve this improvement, because it doesn't affect Win7 targeting.
    • Conflict resolutions: We want to remove DEFINEFUNCTIONPOINTER/STOREFUNCTIONPOINTER for GetCurrentPackageId next to where the machinery for GetSystemTimePreciseAsFileTime has been re-added.
  • Restore the system_clock comment change.
    • We don't need to mention the GetSystemTimeAsFileTime fallback for Win7 here.
  • Restore the P1135R6_atomic_wait test changes.
    • This made it self-contained (instead of centralized in test_atomic_wait.hpp).
    • This also made it not bother to verify the API level. __std_atomic_set_api_level exists for test purposes only, and our dev/test machines are all modern.
  • Rename P1135R6_atomic_wait_vista to P1135R6_atomic_wait_win7.
    • The directory name was confusing, since we've totally dropped Vista support.
  • Fuse test_atomic_wait.hpp into P1135R6_atomic_wait_win7.
    • No changes other than dropping the duplicate banner and #pragma once.
    • Note divergence: <atomic>: ADL-proof implementation of atomic and atomic_ref #4221 (merged 2024-01-17, before we removed Win7 on 2024-06-24) added compile-only test coverage for incomplete types to P1135R6_atomic_wait only. There's no need to replicate that coverage.

Making P1135R6_atomic_wait and P1135R6_atomic_wait_win7 independent obviously duplicates test code, but will make final removal simpler (in 18.0p1).

⚠️ Note to self:

I will need to revert some of MSVC-PR-559637's internal changes, specifically "NON-GitHub: Link src/vctools/PDB binaries against synchronization.lib."

This reverts commit 0be5257.

Conflict resolutions:

* yvals_core.h
  + GH 4742 dropped `_STL_WIN32_WINNT_WIN7`, then GH 4751 dropped `_STL_WIN32_WINNT_WINBLUE`.
* atomic_wait.cpp
  + GH 4742 dropped `_ATOMIC_WAIT_ON_ADDRESS_STATICALLY_AVAILABLE` machinery, then GH 4751 dropped `__std_atomic_compare_exchange_128_fallback`.
Conflict resolutions:

We want to remove `DEFINEFUNCTIONPOINTER`/`STOREFUNCTIONPOINTER` for `GetCurrentPackageId`
next to where the machinery for `GetSystemTimePreciseAsFileTime` is being restored.
We don't need to mention the `GetSystemTimeAsFileTime` fallback for Win7 here.
This made it self-contained (instead of centralized in test_atomic_wait.hpp).

This also made it not bother to verify the API level. `__std_atomic_set_api_level` exists for test purposes only, and our dev/test machines are all modern.
No changes other than dropping the duplicate banner and `#pragma once`.

This obviously duplicates test code, but will make removal simpler.

Note divergence: GH 4221 (merged 2024-01-17, before GH 4742 removed Win7 on 2024-06-24) added compile-only test coverage for incomplete types to `P1135R6_atomic_wait` only. There's no need to replicate that coverage.
@StephanTLavavej StephanTLavavej added the performance Must go faster label Jul 24, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner July 24, 2024 00:39
Copy link
Member

@MahmoudGSaleh MahmoudGSaleh left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@jovibor
Copy link
Contributor

jovibor commented Jul 24, 2024

A PR that was fraught with pain and sorrow😶‍🌫️...

@StephanTLavavej StephanTLavavej merged commit d2fa3a3 into microsoft:main Jul 24, 2024
39 checks passed
@StephanTLavavej StephanTLavavej deleted the crying-cat-emoji branch July 24, 2024 09:51
@darkalexis1990
Copy link

darkalexis1990 commented Aug 13, 2024

Windows Server 2008 R2 is still supported ->Grandfathered[7] Premium Assurance security update support until January 13, 2026.[8][9]

August Updates ->
https://www.catalog.update.microsoft.com/Search.aspx?q=KB5041838

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants