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.

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

[GITHUB] Fix builds by dropping VS2017 and retargeting to VS2022 #6899

Merged
merged 2 commits into from
May 17, 2024
Merged

[GITHUB] Fix builds by dropping VS2017 and retargeting to VS2022 #6899

merged 2 commits into from
May 17, 2024

Conversation

maj113
Copy link
Contributor

@maj113 maj113 commented May 16, 2024 •
edited

Purpose

Github dropped VS2017 in the windows-latest and seemingly also in windows-2019 (since clang-cl fails to find the 14.1 toolset and it uses windows-2019) runners

actions/runner-images@4411220 It also looks like 14.2 is getting removed

JIRA issue: N/A

Proposed changes

  • Drop VS2017 builder
  • Add VS2022 builder
  • Add comment regarding Clang-CL, I tested a build from Github Actions that was compiled with 14.1, it did not boot, fails with 0x1E

TODO

  • Either Fix arm64 debug VS2022/14.3 builder or only compile with VS2019/14.2
  • Clang-CL now fails at 1st stage with 0x7F (0x00000008 | Double Fault), investigate

@maj113 maj113 marked this pull request as draft May 16, 2024 02:10
@maj113
Copy link
Contributor Author

maj113 commented May 16, 2024 •
edited

Made some mistakes regarding msbuild (thought it was failing before too, gonna revert back), will fix in a couple hours

Decided to keep MSBuild updated too.

@maj113 maj113 force-pushed the GHA-fix-build branch 2 times, most recently from 4ee7707 to cfb8a01 Compare May 16, 2024 10:29
@maj113 maj113 marked this pull request as ready for review May 16, 2024 10:53
@binarymaster binarymaster added the documentation Code and community documentation changes and maintenance label May 16, 2024
@binarymaster binarymaster added this to New PRs in ReactOS PRs via automation May 16, 2024
@binarymaster
Copy link
Member

Issue also described here: ilammy/msvc-dev-cmd#80

@tkreuzer
Copy link
Contributor

I'm fine with this, but what about moving VS 2017 to Windows 2019 environment?

@maj113
Copy link
Contributor Author

maj113 commented May 16, 2024 •
edited

I'm fine with this, but what about moving VS 2017 to Windows 2019 environment?

sure but if we also keep VS2022 we would have too many builders/artifacts, plus additional complexity with the excludes (so that Windows-2019 runners don't try to build with VS2022)

cbialorucki
cbialorucki approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@cbialorucki cbialorucki left a comment •
edited

Choose a reason for hiding this comment

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

Contingent upon a fix for the ARM build on VS2022.

ReactOS PRs automation moved this from New PRs to Approved by reviewers May 16, 2024
@maj113
Copy link
Contributor Author

maj113 commented May 16, 2024 •
edited

Putting this here since it seems to be related to the error that the ARM builder experiences https://jira.reactos.org/browse/CORE-14042 seems like it's just ifndef'd out, do we do that for msvc arm (_M_ARM64) too?

@maj113 @binarymaster
[GITHUB] Fix builds by dropping VS2017 and retargeting to VS2022 (rea…
503e68f 
…ctos#6899)

GitHub dropped VS2017 in the windows-latest runner:
- actions/runner-images#9701
- ilammy/msvc-dev-cmd#80

Addendum to 0f81f46.
@maj113 @binarymaster
[CRT] Fix ARM64 VS2022 debug build (reactos#6899)
f22704a 
__globallocalestatus is missing on VS2022/ARM64 just like on Clang-cl.

Addendum to 861360c. CORE-14042 CORE-18200
@binarymaster binarymaster merged commit f22704a into reactos:master May 17, 2024
34 checks passed
ReactOS PRs automation moved this from Approved by reviewers to Done May 17, 2024
@maj113 maj113 deleted the GHA-fix-build branch May 17, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@binarymaster binarymaster binarymaster left review comments

@cbialorucki cbialorucki cbialorucki approved these changes

@zefklop zefklop Awaiting requested review from zefklop

@ColinFinck ColinFinck Awaiting requested review from ColinFinck

@DarkFire01 DarkFire01 Awaiting requested review from DarkFire01

@tkreuzer tkreuzer Awaiting requested review from tkreuzer

@learn-more learn-more Awaiting requested review from learn-more

@Extravert-ir Extravert-ir Awaiting requested review from Extravert-ir

Assignees
No one assigned
Labels
documentation Code and community documentation changes and maintenance
Projects
ReactOS PRs
  
Done
Milestone
No milestone
4 participants
@maj113 @binarymaster @tkreuzer @cbialorucki