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

ENH: adjust default build options #383

Merged
merged 9 commits into from
Apr 8, 2023
Merged

Conversation

dnicolodi
Copy link
Member

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.

@rgommers rgommers added the enhancement New feature or request label Apr 6, 2023
@dnicolodi
Copy link
Member Author

I'm updating the documentation. I wanted to check that this does not break anything before finishing the documentation.

@dnicolodi
Copy link
Member Author

Documentation updated. I still feel that the "Default build options" page belongs to "Reference" and not to "Explanations". I also feels that splitting the documentation is these sections is silly, but this is a longer story...

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 dnicolodi force-pushed the build-otpions branch 11 times, most recently from e58ac01 to 9823669 Compare April 8, 2023 00:29
@dnicolodi
Copy link
Member Author

Running the tests with MSVC bumped into a bug in Meson mesonbuild/meson#11655. I added a work around.

@dnicolodi dnicolodi added this to the v0.13.0 milestone Apr 8, 2023
Copy link
Contributor

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

This looks pretty good, thanks @dnicolodi. One suggestion for security, and one question on --only-changed.

'--python.purelibdir',
sys_paths['purelib'],
'--python.platlibdir',
sys_paths['platlib'],
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll note that the change to --destdir and these options no longer being needed was discussed in #338 (comment). Looks like we agreed on that, and it has a good rationale.

@dnicolodi dnicolodi force-pushed the build-otpions branch 2 times, most recently from dd7b4c4 to 2e98c20 Compare April 8, 2023 21:01
The install directory is always an empty temporary directory, thus
passing the --only-changed option cannot speed up installation. On the
contrary, it most likely requires at least one extra stat() system
call for each file copied. This option was added with the first
implementation of editable installs. There the installation was
performed on a permanent directory and the --only-changed option
helped reducing the installation time.

When meson install is run the project has been just been built, thus
it is not necessary to rebuild it. Add the --no-rebuild option.
Meson assumes that a .pdb file is generated for executables also when
they are compiled without debug symbols, and adds it to the
installation manifest. This causes meson-python to try to add a file
that does not exist to the wheel.
Allow .pyw source file extension on Windows and reject all unknown
file extensions: Python package directory names cannot contain a dot.
Copy link
Contributor

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM, let's give this a go!

The added MSVC CI job is nice. There's no test that only passes with MSVC, so this may in principle pass also if MSVC activation fails - but that's being dealt with in gh-371 already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Defaults for Meson build options, linking issues with MSVC
2 participants