search

finos / perspective

A data visualization and analytics component, especially well-suited for large and/or streaming datasets.
https://perspective.finos.org/
Apache License 2.0
7.72k stars 1.05k forks source link

Fix compatibility with other MSVC versions by explicitly converting to string #2570

Closed timkpaine closed 3 months ago

timkpaine commented 3 months ago

This is a really minor bug, but causes some annoyance during our build process and is the reason we have to explicitly set PSP_GENERATOR in a few places.

texodus commented 3 months ago

What is the bug? What is PSP_GENERATOR? How do I know not to revert this in the future?

timkpaine commented 3 months ago

PSP_GENERATOR is the user-configureable override (similar to e.g. PSP_DEBUG) for the cmake-generator used by the C++ build system. This doesn't matter too much on Mac and Linux, where you can likely use makefiles or ninja interchangeably, but on Windows you may want to override the default generator to be a certain version of Visual Studio for compatibility with python or other libraries. In the perspective codebase, we use this here and its also used in e.g. conda-forge or when building from source.

We have always hardcoded it in our CI, which is why we likely never saw the issue. It's also possible that we hardcoded it originally BECAUSE of this bug. Regardless, this fixes the Visual Studio detection logic so that by default we build against the same VS that your python built against (whereas before the bug caused it to always use the default because of the wrong key type)