Skip to content

Conversation

@dnicolodi
Copy link
Member

@dnicolodi dnicolodi commented Apr 4, 2023

The meson-python -Db_ndebug=if-release default option does not have effect unless the project is also configured with -Dbuildtype=release or default_options: ['buildtype=release'] is specified in project(). I believe this was not the desired effect.

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.

Thanks for adding this test. Looks like we should settle the buildtype change discussion in gh-381, and then this will be mergeable.

def test_ndebug(package_scipy_like, tmp_path, args):
with mesonpy._project({'setup-args': args}) as project:
command = subprocess.run(
['ninja', '-C', os.fspath(project._build_dir), '-t', 'commands', '../../mypkg/extmod.c^'],
Copy link
Contributor

Choose a reason for hiding this comment

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

The ^ is a typo I think?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ^ asks ninja to report the build command for the target having the file preceding the ^ as part of its inputs. See the second paragraph here https://ninja-build.org/manual.html#_running_ninja

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah interesting, didn't know that.

@rgommers rgommers added the tests label Apr 6, 2023
@dnicolodi
Copy link
Member Author

I didn't meant this to be merged but only as a reproducer of the issue I saw. It would need some adjustment if we want to merge it.

@rgommers
Copy link
Contributor

rgommers commented Apr 8, 2023

This can be closed now that gh-383 is merged I assume?

@dnicolodi
Copy link
Member Author

The purpose of #383 is also to fix this, thus this can be closed. However, there isn't a test that makes sure the -DNDEBUG thing does not break in the future.

@rgommers
Copy link
Contributor

The purpose of #383 is also to fix this, thus this can be closed. However, there isn't a test that makes sure the -DNDEBUG thing does not break in the future.

Good point. That's a fairly low-probability thing, so I'll close this and open a follow-up issue to add a test (after the next release).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants