[corrade] Fix build of magnum on MSVC 2019

[mosra]

(I'm the author of these libraries.)

In 71214e6 (PR #7025) a flag was added to make corrade build on MSVC 2019. This was done on Jun 25 (merged on Jul 7), however real MSVC 2019 support in corrade sources was introduced soon after (on Jul 5). Until then, MSVC 2019 wasn't officially supported by these libraries.

Setting this flag unconditionally unfortunately breaks the build of Magnum (an error about an unknown _deg literal in GenerateNormals.cpp, a regression of MSVC 2019 compared to 2017) and besides that causes some code parts to be overly pessimistic about C++ standard support, leading to a reduced feature set. So I removed it, together with one additional fix for a typo in a flag.

Note that v2019.01 doesn't work with MSVC 2019, the next release (v2019.09, scheduled to go out in September, mosra/magnum#340) will. I hope that's not a problem for your CIs.

---

By the way there's a fixC2666.patch, is it still needed? I'm not aware of any crash related to /permissive- not being present, rather the opposite. I asked on #5126 but never got a reply.

[tarcila]

Hi @mosra

Not sure about the intent here. Reverting the workaround flag without fixing the original issue will break back the generation for vs2019. So, waiting for the next version of Magnum to be released, how about:

• leaving the workaround flag as is
• back-porting required fixes if simple enough

[mosra]

@tarcila, thanks for your reply. The intent is:

• Users are encouraged to install the --head version of corrade and magnum (because master is stable enough for production use), and with the current state, magnum fails to build on MSVC 2019 because of the incorrectly enabled CORRADE_MSVC2017_COMPATIBILITY. That's a serious problem for the users, because next time they update to a --head version, things will break.
• The v2019.01 version won't work with MSVC 2019 (it works with 2017) and backporting the required fixes there is not feasible -- I have too many things to do and I'd rather spend my time working on the next release than on backporting flags to a version that is used very little. Enabling the flag only gives a false impression of things "working" for this particular package -- but neither magnum nor any other depending packages will work correctly.

Releasing a new tagged version that works with MSVC 2019 will take about a month and until then it's not ideal, I agree. However I'd like to have this sorted out because I don't want to answer bugreports related to this twice a day. It's

• either about the --head of magnum not working at all because the flag is present (affecting majority of users)
• or about corrade v2019.01 (which is used less and less) not working on MSVC 2019 because the flag is not set (while magnum v2019.01 doesn't work with it at all, independently of the flag being set)

... and I'd prefer the second option, if you understand :)

[tarcila]

Thanks for giving insights about how Magnum is expected to be used. And I do understand why you prefer the second option! That being said:

• Forcing the use of master in a context such a vcpkg defeats the use of fixed tags. vcpkg gives the ability to reuse the exact same version of dependencies when rebuilding the software months later. This makes it impossible with Magnum.
• I use Magnum v2019.01 with vs2019 and it works fine to the feature extent I am using (GL, Shaders, Trade, Primitives). Not sure about what is not working when applying vs2017 workarounds.

How about defining the -D<variable> only when not building head? In that case, a warning can be added stating that upstream does not support v2019.01 with vs2019. If upstream support is needed, master is the branch to be used (vcpkg install --head)
I think VCPKG_HEAD_VERSION could be used to detect if building from master instead of fixed ref.

[mosra]

OK, I see your point :) I tried to figure out what VCPKG_HEAD_VERSION is set to, but couldn't decipher it from the docs (and right now I don't have a Windows system around, so I can't experiment). Any chance you could propose the needed change? 🙏

Thanks for all the contributions you did to the magnum packages, btw.

[tarcila]

I confirm that the documentation is not clear about the content of the variable. I don't have a Windows system at hand for now, but, no problem, I should be able to propose something in the coming days. Keeping you posted.

[tarcila]

@mosra Please have a look at #7566

[mosra]

Obsoleted by #7566, thank you @tarcila! :)