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

windows: drop code and curl manifest targeting W2K and older #16453

Closed
wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Feb 24, 2025

curl requires Windows XP since 2023. Drop version detection code using
GetVersionEx() aimed to support earlier Windows versions. With that
call deleted, the embedded manifest in curl.rc becomes unnecessary.
Delete it too, along with the enabler logic in build systems.

This allows to stop forcing /MANIFEST:NO for MSVC builds. Dropping it
fixes VS2008 shared builds, that require an auto-generated SxS
(side-by-side assembly) manifest to find their CRT DLLs. This was the
issue that prevented VS2008 curl.exe launching on AppVeyor CI:

src/curl.exe: error while loading shared libraries: ?: cannot open shared object file: No such file or directory

Ref: https://ci.appveyor.com/project/curlorg/curl/builds/51577006/job/eitypvwlb1rxr11d#L261

FWIW the curl.rc embedded manifest wasn't ever enabled for VS2008 CI
builds either, because CMake did not pass our custom macro via
CMAKE_RC_FLAGS to rc.exe. For reasons I could not figure out.

After this patch the curl build no longer inject its own manifest, and
lets the default be applied by linkers and toolchains. It fixes VS2008
shared builds. curl continues to detect the real Windows version via
RtlVerifyVersionInfo() from ntdll.

Follow-up to 960d601 #12225
Follow-up to 5044909 #7810
Follow-up to ebd2132 #1221
Ref: #15972
Cherry-picked from #16394

curl requires Windows XP since 2023. Drop version detection code using
`GetVersionEx()` aimed to support earlier Windows versions. With that
call deleted, the embedded manifest in `curl.rc` becomes unnecessary.
Delete it too, along with the enabler logic in build systems.

This allows to stop forcing `/MANIFEST:NO` for MSVC builds. Dropping it
fixes VS2008 shared builds, that require an auto-generated SxS manifest
to find their CRT DLLs. This was the issue that prevented VS2008
`curl.exe` launching on AppVeyor CI.

FWIW the `curl.rc` embedded manifest wasn't ever enabled for VS2008 CI
builds either, because CMake did not pass our custom macro via
`CMAKE_RC_FLAGS` to `rc.exe`. For reasons I couldn't figure out.

After this patch the curl build no longer inject its own manifest, and
lets the default be applied by linkers and toolchains. It fixes VS2008
shared builds. curl continues to detect the real Windows version via
`RtlVerifyVersionInfo` from `ntdll`.

Follow-up to 960d601 curl#12225
Follow-up to 5044909 curl#7810
Follow-up to ebd2132 curl#1221
Cherry-picked from curl#16394
@vszakats vszakats added build Windows Windows-specific labels Feb 24, 2025
@vszakats vszakats added the cmake label Feb 24, 2025
@vszakats vszakats closed this in 9b0467b Feb 24, 2025
@vszakats vszakats deleted the w-drop-manifest branch February 24, 2025 20:01
vszakats added a commit that referenced this pull request Feb 24, 2025
- appveyor: restore VS2008 job, after fixing its issues.
  Enable OpenSSL in it. It takes 1 minute.
  Follow-up to 9b0467b #16453
  Follow-up to edfa537 #16456
- appveyor: make a copy of OpenSSL DLLs to have them picked up as an
  artifact (disabled by default) to aid local tests.
- appveyor: dump CMake configuration logs on failure.
- appveyor: tidy up job parameter defaults.
- GHA/windows: add pre-fill check option for dl-mingw jobs.
- GHA/windows: fix pre-fill check option for MSYS jobs by installing
  `diffutils`.
  Follow-up to e7adf3e #15841
- GHA/windows: de-duplicate to `PATH` commands for Cygwin.
- GHA/windows: drop `$SYSTEMROOT/System32` from `PATH` for Cygwin
  configure. It's not needed.
  Follow-up to 36fd2dd #13599
- list `.pdb` files in curl version step for MSVC.
  Ref: #16439

Cherry-picked from #16394
Closes #16458
icing pushed a commit to icing/curl that referenced this pull request Feb 25, 2025
curl requires Windows XP since 2023. Drop version detection code using
`GetVersionEx()` aimed to support earlier Windows versions. With that
call deleted, the embedded manifest in `curl.rc` becomes unnecessary.
Delete it too, along with the enabler logic in build systems.

This allows to stop forcing `/MANIFEST:NO` for MSVC builds. Dropping it
fixes VS2008 shared builds, that require an auto-generated SxS
(side-by-side assembly) manifest to find their CRT DLLs. This was the
issue that prevented VS2008 `curl.exe` launching on AppVeyor CI:
```
src/curl.exe: error while loading shared libraries: ?: cannot open shared object file: No such file or directory
```
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/51577006/job/eitypvwlb1rxr11d#L261

FWIW the `curl.rc` embedded manifest wasn't ever enabled for VS2008 CI
builds either, because CMake did not pass our custom macro via
`CMAKE_RC_FLAGS` to `rc.exe`. For reasons I could not figure out.

After this patch the curl build no longer inject its own manifest, and
lets the default be applied by linkers and toolchains. It fixes VS2008
shared builds. curl continues to detect the real Windows version via
`RtlVerifyVersionInfo()` from `ntdll`.

Follow-up to 960d601 curl#12225
Follow-up to 5044909 curl#7810
Follow-up to ebd2132 curl#1221
Ref: curl#15972
Cherry-picked from curl#16394

Closes curl#16453
icing pushed a commit to icing/curl that referenced this pull request Feb 25, 2025
- appveyor: restore VS2008 job, after fixing its issues.
  Enable OpenSSL in it. It takes 1 minute.
  Follow-up to 9b0467b curl#16453
  Follow-up to edfa537 curl#16456
- appveyor: make a copy of OpenSSL DLLs to have them picked up as an
  artifact (disabled by default) to aid local tests.
- appveyor: dump CMake configuration logs on failure.
- appveyor: tidy up job parameter defaults.
- GHA/windows: add pre-fill check option for dl-mingw jobs.
- GHA/windows: fix pre-fill check option for MSYS jobs by installing
  `diffutils`.
  Follow-up to e7adf3e curl#15841
- GHA/windows: de-duplicate to `PATH` commands for Cygwin.
- GHA/windows: drop `$SYSTEMROOT/System32` from `PATH` for Cygwin
  configure. It's not needed.
  Follow-up to 36fd2dd curl#13599
- list `.pdb` files in curl version step for MSVC.
  Ref: curl#16439

Cherry-picked from curl#16394
Closes curl#16458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants