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

Defaults for Meson build options, linking issues with MSVC #381

Closed
dnicolodi opened this issue Apr 5, 2023 · 4 comments · Fixed by #383
Closed

Defaults for Meson build options, linking issues with MSVC #381

dnicolodi opened this issue Apr 5, 2023 · 4 comments · Fixed by #383
Labels
bug Something isn't working
Milestone

Comments

@dnicolodi
Copy link
Member

Meson native default build options set buildtype=debug https://mesonbuild.com/Builtin-options.html#core-options This implies debug=true, optimization=0 https://mesonbuild.com/Builtin-options.html#details-for-buildtype

meson-python sets debug=false, optimization=2. This was understood to result in a "release" kind of build. Note that buildtype=release would set optimization=3 thus is not exactly the same. Also note that, despite what the Meson documentation says, setting debug and optimization separately has no effect on the buildtype configured for the build.

However, these settings do not result in the buildtype to change. Therefore, other build options whose values are derived from buildtype are set as for a debug build.

In particular, the default value for the b_vscrt build option https://mesonbuild.com/Builtin-options.html#base-options in particular is from_buildtype which means that on a debug build the MSVC compiler will link with the debug version of the VS runtime library, which in turn will make the linker look for the debug build of all the linked DLLs. This is problematic because the regular Python distribution for Windows does not contain a debug version of python.dll and linking fails. This was the reason for build issues in #371 and most likely for other build issue reported in unrelated issues.

This also result in the b_ndebug option that meson-python sets to if-release to do not work as intended (or, at least, not as I understood it was intended to work): buildtype different from release implies b_ndebug=false independent of the value assigned to the debug option. This is demonstrated in #379.

There may be other behavior of Meson that depends on the value assigned to buildtype regardless to the value assigned to the separate debug and optimization options.

While the -DNDEBUG issue is not critical and other consequences of having buildtype=debug may at worst be not-so-well optimized builds, the b_vscrt issue results in build failures and should be addressed somehow.

If we focus the attention to the b_vscrt option only, there are a few possible solutions:

  1. Do not change the build options and document that the default Meson options need to be adjusted by package authors to avoid link issue with the most common distribution of Python on Windows when compiling with MSVC.
  2. Adjust the buildtype to something different from debug so that b_vscrt gets the correct value. This incurs in the minor issue that Meson emits a warning when debug or optimization in combination with buildtype.
  3. adjust the value of b_vscrt to do the right thing regardless of the value of buildtype.

I don't think that (1) goes in the spirit of the defaults adopted for meson-python so far.

Solution (2) seems a bit heavy handed, but for most things there is no much of a practical difference between setting debug=false, optimization=2 and setting buildtype=release. Another option could be to set buildtype=debugoptimized, debug=false, but this still presents the b_ndebug issue and having a debug build without debug symbols seems paradoxical.

Because setting the from VS runtime library compilation flags results in a link failure on most systems, maybe we should do (3) regardless. However, I'm not sure about what the right value for that option would be: does Python require a dynamically or statically linked VS runtime?

@rgommers
Copy link
Contributor

rgommers commented Apr 6, 2023

This is problematic because the regular Python distribution for Windows does not contain a debug version of python.dll and linking fails. This was the reason for build issues in #371 and most likely for other build issue reported in unrelated issues

Thanks for getting to the bottom of this!

I agree that (1) is not ideal. I think we should do (2) indeed, set it to release. For context: when we added debug=false, optimization=2 we thought that the only difference was optimization level, and did not realize how buildtype affected other options. In my experience, optimization=2 was preferred over 3 because of a number of issues in numerical libraries that only show up with -O3. However, those are only relevant for a limited set of projects, and those projects can fairly easily deal with the problem themselves. In general, release is a very reasonable choice for meson-python to make, arguably the most logical one.

Because setting the from VS runtime library compilation flags results in a link failure on most systems, maybe we should do (3) regardless. However, I'm not sure about what the right value for that option would be: does Python require a dynamically or statically linked VS runtime?

I'm fairly sure that dynamic linking is the default for both msvcrt and ucrt is what's most commonly used for regular wheels. If you search Lib/distutils/ in the CPython 3.11 source tree, you'll find multiple /MD usages, and no /MT. Same for numpy.distutils.

@dnicolodi
Copy link
Member Author

These posts contain relevant information:

mesonbuild/meson#10808
https://blogs.gnome.org/mcatanzaro/2022/07/15/best-practices-for-build-options/
https://nibblestew.blogspot.com/2022/08/making-decision-without-all-information.html

I particular:

On most platforms it means "debug info" but on Windows (or, specifically, with the MSVC toolchain) "debug" means a special build type that uses the "debug runtime" that has additional runtime checks that are useful during development. More info can be found e.g. here. This made the word "debug" doubly problematic. Not only do people on Windows want it to refer to the debug runtime but then some (but not all) people on Linux think that "debugoptimized" means that it should only be used during development. Originally that was not the case, it was supposed to mean "build a binary with the default optimizations and debug info". What I originally wanted was that distros would build packages with buildtype set to debugoptimized as opposed to living in the roaring 90s, passing a random collection of flags via CFLAGS and hoping for the best.

@rgommers
Copy link
Contributor

rgommers commented Apr 6, 2023

I did some testing with SciPy, and there are no extra failures with buildtype=release. In NumPy there may be a few, in particular with Intel compilers I think, but those are a bit flaky anyway (they optimize for performance rather than correctness sometimes). So I think it's fairly safe to change the optimization level.

dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Apr 6, 2023
Switch from debug=false, optimization=2 to buildtype=release. Despite
what the Meson documentation says, the former does not change the
default buildtype=debug, which undesired effects on other settings
derived from the value of the buildtype option.

Whit the default b_vscrt=from_buildtype option, Meson instructs the
MSVC compiler to use the debug version of the VS runtime library when
buildtype=debug. This causes the linker look for the debug build of
all the linked DLLs. The Python distribution for Windows does not
contain a debug version of the python.dll and linking fails. To avoid
this issue when the user explicitly asks for a debug build, also set
b_vscrt=mt.

The b_ndebug=if-release option set by meson-python also looks at the
value of the buildtype and not to the value of the debug options.
Setting buildtype=release has the desired effect of disabling
assertions.

The prefix, python.purelibdir, and python.platlibdir build options are
no more necessary after the heuristic for determining the location
where installed files need to be placer from their installation
location has been removed in mesonbuild#280. Remove them.

Fixes mesonbuild#381.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Apr 6, 2023
Switch from debug=false, optimization=2 to buildtype=release. Despite
what the Meson documentation says, the former does not change the
default buildtype=debug, which undesired effects on other settings
derived from the value of the buildtype option.

Whit the default b_vscrt=from_buildtype option, Meson instructs the
MSVC compiler to use the debug version of the VS runtime library when
buildtype=debug. This causes the linker look for the debug build of
all the linked DLLs. The Python distribution for Windows does not
contain a debug version of the python.dll and linking fails. To avoid
this issue when the user explicitly asks for a debug build, also set
b_vscrt=mt.

The b_ndebug=if-release option set by meson-python also looks at the
value of the buildtype and not to the value of the debug options.
Setting buildtype=release has the desired effect of disabling
assertions.

The prefix, python.purelibdir, and python.platlibdir build options are
no more necessary after the heuristic for determining the location
where installed files need to be placer from their installation
location has been removed in mesonbuild#280. Remove them.

Fixes mesonbuild#381.
@rgommers
Copy link
Contributor

rgommers commented Apr 6, 2023

Wow, read those blog posts now, that is .... not good.

dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Apr 7, 2023
Switch from debug=false, optimization=2 to buildtype=release. Despite
what the Meson documentation says, the former does not change the
default buildtype=debug, which undesired effects on other settings
derived from the value of the buildtype option.

Whit the default b_vscrt=from_buildtype option, Meson instructs the
MSVC compiler to use the debug version of the VS runtime library when
buildtype=debug. This causes the linker look for the debug build of
all the linked DLLs. The Python distribution for Windows does not
contain a debug version of the python.dll and linking fails. To avoid
this issue when the user explicitly asks for a debug build, also set
b_vscrt=mt.

The b_ndebug=if-release option set by meson-python also looks at the
value of the buildtype and not to the value of the debug options.
Setting buildtype=release has the desired effect of disabling
assertions.

The prefix, python.purelibdir, and python.platlibdir build options are
no more necessary after the heuristic for determining the location
where installed files need to be placer from their installation
location has been removed in mesonbuild#280. Remove them.

Fixes mesonbuild#381.
@rgommers rgommers added this to the v0.13.0 milestone Apr 8, 2023
@rgommers rgommers added the bug Something isn't working label Apr 8, 2023
rgommers pushed a commit that referenced this issue Apr 8, 2023
Switch from debug=false, optimization=2 to buildtype=release. Despite
what the Meson documentation says, the former does not change the
default buildtype=debug, which undesired effects on other settings
derived from the value of the buildtype option.

Whit the default b_vscrt=from_buildtype option, Meson instructs the
MSVC compiler to use the debug version of the VS runtime library when
buildtype=debug. This causes the linker look for the debug build of
all the linked DLLs. The Python distribution for Windows does not
contain a debug version of the python.dll and linking fails. To avoid
this issue when the user explicitly asks for a debug build, also set
b_vscrt=mt.

The b_ndebug=if-release option set by meson-python also looks at the
value of the buildtype and not to the value of the debug options.
Setting buildtype=release has the desired effect of disabling
assertions.

The prefix, python.purelibdir, and python.platlibdir build options are
no more necessary after the heuristic for determining the location
where installed files need to be placer from their installation
location has been removed in #280. Remove them.

Fixes #381.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants