Conversation
See test results for failed build of commit 8737fb3a36 |
michaelDCurran
left a comment
There was a problem hiding this comment.
Agreed with @lukaszgo1 that this logic differs from what we used to have and that we can no longer make try-release builds.
Note that try-release builds are marked as release but also as a test version.
But there is a contradiction between setup.py and SConstruct in that if we are building a release, python setup.py will be run with -O (optimise=1) but if this is a test version, setup.py itself tells the compiled NVDA to be run with optimise=0. Thus try-release builds will end up with optimise=0.
release should be passed into setup.py, and optimise should be 1 if release is true.
I acknowledge that this then means we will again treat betas as release, and therefore assertions won't be on in betas. But, personally, I do think that assertions should really only be for alphas (bleeding-edge code). Betas although prerelease, should still function as releases in terms of performance.
d14916d to
e71101f
Compare
Fix up of nvaccess#15544 Summary of the issue: nvaccess#15544 changed our compiler flag to raise exceptions if an assert statement fails. This slows down NVDA (having to check exceptions) and adds risk to final releases. This lead to nvaccess#16212 being discovered Description of user facing changes Assertions should be ignored in builds Description of development approach Restored optimize level to 1 https://docs.python.org/3.11/tutorial/modules.html#compiled-python-files
Link to issue number:
Fix up of #15544
Summary of the issue:
#15544 changed our compiler flag to raise exceptions if an assert statement fails.
This slows down NVDA (having to check exceptions) and adds risk to final releases.
This lead to #16212 being discovered
Description of user facing changes
Assertions should be ignored in builds
Description of development approach
Restored optimize level to 1
https://docs.python.org/3.11/tutorial/modules.html#compiled-python-files
Testing strategy:
Known issues with pull request:
#16212 is still an issue that needs to be fixed
Code Review Checklist: