Skip to content

build: drop experimental support for Windows <10 #54079

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 30, 2024
Merged

build: drop experimental support for Windows <10 #54079

merged 1 commit into from
Jul 30, 2024

Conversation

@targos
Copy link
Member

@targos targos commented Jul 28, 2024

V8 now uses Windows APIs that are not available in older Windows
versions.

Fixes the V8 12.8 build (#54077):
https://ci.nodejs.org/job/node-compile-windows-debug/22986/nodes=win-vs2022/

10:34:02 C:\workspace\node-compile-windows-debug\node\deps\v8\src\base\platform\platform-win32.cc(765,34): error C2065: 'IsUserCetAvailableInEnvironment': undeclared identifier [C:\workspace\node-compile-windows-debug\node\tools\v8_gypfiles\v8_libbase.vcxproj]

@targos
build: drop experimental support for Windows <10
916b35f 
V8 now uses Windows APIs that are not available in older Windows
versions.
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 28, 2024

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@targos
Copy link
Member Author

targos commented Jul 28, 2024

@nodejs/platform-windows

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 28, 2024
@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 28, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 28, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 28, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/60687/

@codecov
Copy link

codecov bot commented Jul 28, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.15%. Comparing base (2b5856e) to head (916b35f).
Report is 16 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54079      +/-   ##
==========================================
- Coverage   87.16%   87.15%   -0.01%     
==========================================
  Files         643      643              
  Lines      181683   181694      +11     
  Branches    34957    34953       -4     
==========================================
- Hits       158362   158361       -1     
- Misses      16605    16609       +4     
- Partials     6716     6724       +8
Files Coverage Δ
src/node_main.cc 100.00% <ø> (ø)

... and 28 files with indirect coverage changes

RafaelGSS
RafaelGSS approved these changes Jul 28, 2024
View reviewed changes
Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM. I assume it shouldn't be backported to v20 (LTS), right?

Regardless, should we mention it on CHANGELOG (notable-change)?

@targos targos added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jul 29, 2024
@targos
Copy link
Member Author

targos commented Jul 29, 2024

Let's make it major. We don't need to backport anywhere.

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 29, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/60707/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 30, 2024

CI: https://ci.nodejs.org/job/node-test-pull-request/60749/

@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 30, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 30, 2024
@nodejs-github-bot nodejs-github-bot merged commit a5decd4 into nodejs:main Jul 30, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 30, 2024

Landed in a5decd4

@targos targos deleted the rm-win8 branch July 30, 2024 12:56
@RafaelGSS RafaelGSS mentioned this pull request Oct 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@tniessen tniessen tniessen approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@StefanStojanovic StefanStojanovic StefanStojanovic approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@targos @nodejs-github-bot @jasnell @lpinca @tniessen @gengjiawen @StefanStojanovic @RafaelGSS