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

Upgrade windows msvc CI bot to use a newer version of msvc #4137

Closed
garretrieger opened this issue Feb 24, 2023 · 10 comments
Closed

Upgrade windows msvc CI bot to use a newer version of msvc #4137

garretrieger opened this issue Feb 24, 2023 · 10 comments
Assignees

Comments

@garretrieger
Copy link
Collaborator

I've been investigating the msvc-2019-x86 CI failure on this PR #4121. I was able to get it to reproduce on a 32 bit windows machine using MSVC 2019. I also tested with a newer version of MSVC (2022) and the test no longer fails. Could we update the CI setup to use the more recent version of MSVC?

@khaledhosny
Copy link
Collaborator

We generally try to support as old compilers as possible, but I don’t know if MSVC 2019 is still widely used or now, may be @fanc999 have some insight.

@behdad behdad mentioned this issue Feb 27, 2023
@khaledhosny
Copy link
Collaborator

khaledhosny commented Feb 28, 2023

Both msvc and msys2 32-bit builds are failing from #4121, but msys2 CI job does not use MSVC, it uses mingw-w64-i686-gcc (gcc 12.2.0). Not sure what to do with this one.

@behdad
Copy link
Member

behdad commented Feb 28, 2023

That is indeed suspicious. Wonder if there's actually a bug lurking.

@behdad
Copy link
Member

behdad commented Feb 28, 2023

Okay so there's the MSVC-only fail, which disappears with more recent MSVC. But both MSVC and mingw 32bit bots show a fuzzer failure. That's possibly real... @garretrieger Let me think.

@behdad
Copy link
Member

behdad commented Feb 28, 2023

I can reproduce the "1 subset fuzzer test failed" on Linux 32bit build as well. So, that one I'll debug.

@behdad behdad closed this as completed in 2d33a6b Feb 28, 2023
@behdad behdad reopened this Feb 28, 2023
@fanc999
Copy link
Contributor

fanc999 commented Mar 1, 2023

We generally try to support as old compilers as possible, but I don’t know if MSVC 2019 is still widely used or now, may be @fanc999 have some insight.

Hi,

Sadly, I won't be able to give a good answer on that, but I do assume that since Visual Studio 2015 and later link to the same UCRT DLLs, and that intermixing between Visual Studio 2015~2022 shared builds is normally fine, I would presume for instance where people couldn't use Visual Studio Community editions due to licensing reasons (since, for instance, they are developing proprietary programs in a sizable team), that Visual Studio 2019 is still quite prevalent.

Nowadays, I build HarfBuzz with Visual Studio 2017, and is able to reproduce this issue on 32-bit builds as well.

For this one, I think the places that we need to look out for are:

  • pointers (since Windows is a LLP64 system)
  • Whether we are using APIs that are designated APICALL/__stdcall.

I will also look into things here.

With blessings, and cheers!

@fanc999
Copy link
Contributor

fanc999 commented Mar 1, 2023

Hi,

I tried testing with the latest Visual Studio 2017, 2019 and 2022 x86, and the test failed, sadly...

Looks like the failure was introduced in commit c0fac01 (with the commit before that, things passed), which means the PR #4121 was not related and updating the CI to vs2022 would not have helped (and seems that @behdad’s update would have remedied things).

With Visual Studio 2017 x64, the test passed

With blessings, and cheers!

@garretrieger
Copy link
Collaborator Author

There was a bug in that commit which was fixed here: 2d33a6b are you still seeing the failure with that commit?

@fanc999
Copy link
Contributor

fanc999 commented Mar 2, 2023

There was a bug in that commit which was fixed here: 2d33a6b are you still seeing the failure with that commit?

The failure is remedied by Behdad's update (as in my previous post), which is the commit you mentioned.

@behdad
Copy link
Member

behdad commented Mar 2, 2023

Looks like e471ef7 fixed this after all?

@behdad behdad closed this as completed Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants