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

Drop support for building with old Visual Studio versions #15403

Closed
wants to merge 5 commits into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 14, 2024

The first commit requires users to have at least Visual Studio 2015; the second commit raises that to Visual Studio 2017 RTW; the third commit even raises that to Visual Studio 2019 RTW 16.0.

Note that building with Visual Studio 2017 15.9.64 (latest version) fails on

char *bc_copy_and_toggle_bcd(char *restrict dest, const char *source, const char *source_end);

because the restrict keyword is not supported. Thus, if we still want to support Visual Studio 2017, we would need to fix this (and maybe more; I haven't done a full snapshot build).

However, I think we should (or even need to) raise the bar even further. Builds with Visual Studio 2019 16.11.38 (latest version) appear to work flawlessly, but I'm not sure whether somewhat older Visual Studio 2019 versions would work (and I don't think I can test this since going back to older Visual Studio 2019 version is impossible, to my knowledge; you don't need to update, but you can't install an older version).

For details regarding the versioning details, see https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170.

`MSC_VER` 1900 refers to Visual Studio 2015[1], and this should be the
bare minimum which we support nowadays.  If users use an older Visual
Studio version, we fail gracefully during `configure`.

[1] <https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170>
`MSC_VER` 1910 refers to Visual Studio 2017 RTW[1], and this should be
the bare minimum which we support nowadays.  If users use an older
Visual Studio version, we fail gracefully during `configure`.

[1] <https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170>
`MSC_VER` 1920 refers to Visual Studio 2019 RTW 16.0[1], and this
should be the bare minimum which we support nowadays.  If users use an
older Visual Studio version, we fail gracefully during `configure`.

[1] <https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170>
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

This makes sense to me

@cmb69
Copy link
Member Author

cmb69 commented Aug 15, 2024

Oh, just noticed that I forgot to clean up toolset_get_compiler_name():

if (version >= 1950) {
return name;
} else if (version >= 1930) {
name = short ? "VS17" : "Visual C++ 2022";
} else if (version >= 1920) {
/* NOTE - VS is intentional. Due to changes in recent Visual Studio
versioning scheme referring to the exact VC++ version is
hardly predictable. From this version on, it refers to
Visual Studio version and implies the default toolset.
When new versions are introduced, adapt also checks in
php_win32_image_compatible(), if needed. */
name = short ? "VS16" : "Visual C++ 2019";
} else if (version >= 1910) {
name = short ? "VC15" : "Visual C++ 2017";
} else if (version >= 1900) {
name = short ? "VC14" : "Visual C++ 2015";
} else {
ERROR("Unsupported Visual C++ compiler " + version);
}

There might be more. I'll have a closer look.

@cmb69
Copy link
Member Author

cmb69 commented Aug 15, 2024

Indeed, need to check all _MSC_VER uses in the sources.

@cmb69
Copy link
Member Author

cmb69 commented Aug 15, 2024

Indeed, need to check all _MSC_VER uses in the sources.

D'oh! That opens up a can of worms, and is probably better adressed on its own (probably multiple PRs).

…name()

Since we error out in `toolset_get_compiler_version()` now, there is
even no more need to raise an error here.
@cmb69 cmb69 mentioned this pull request Aug 31, 2024
@cmb69 cmb69 closed this in b3d6414 Aug 31, 2024
@cmb69 cmb69 deleted the cmb/VCVERS branch August 31, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants