Py3.8: Transparently use a Python virtual environment under the hood #12075

michaelDCurran · 2021-02-15T22:56:57Z

Summary of the issue:

NVDA and its build system have has many Python dependencies, the majority of which are fetched as git submodules to official git repositories, some are git submodules for our own git repositories containing pre-built dependencies (E.g. wxPython) and some are fetched via pip. And even the ones via pip are fetched differ as to where they are fetched into our build system.

The current approach to Python dependencies as the following issues:

There is no standard way of tracking / fetching Python dependencies
For certain dependencies such as wxPython, there is a great deal of extra work to pre-build and store in a git repository each time we want to upgrade to a new version of that dependency
Python dependencies installed within our build system may conflict with Python packages already installed globally on the developer's system
Python dependencies for NVDA may be inappropriately installed globally on the developer's system, which could affect things outside of NVDA.

As we move to Python 3.8, some of our Python dependencies (namely wxPython, pySerial and py2exe) need to be upgraded. We should take this opportunity to standardize on a cleaner way of fetching Python dependencies that address all the above issues.

Description of how this pull request fixes the issue:

A Python virtual environment is now used transparently by the NVDA build system, and all Python dependencies are installed into this environment using pip.
From a developer's perspective using the NVDA build system, there is no need to worry about the Python virtual environment behind the scenes, nor should you create and or activate one manually. All NVDA build system commands will handle this transparently.

To build NVDA, SCons should continue to be used in the usual way. E.g. executing scons.bat in the root of the repository. Running py -m SCons is no longer supported, and scons.py has also been removed.
To run NVDA from source, rather than executing source/nvda.pyw directly, the developer should now use runnvda.bat in the root of the repository. If you do try to execute source/nva.pyw, a message box will alert you this is no longer supported.
To perform unit tests, execute rununittests.bat []
To perform system tests: execute runsystemtests.bat []
To perform linting, execute runlint.bat
Behind the scenes, the above batch files (scons, runnvda, rununittests, runsystemtests and runlint) ensure that the Python virtual environment is created and up to date, activates the environment, runs the command and then deactivates. All transparently. A developer should not have to know about the Python virtual environment at all.
The first time one of these commands are run, the virtual environment will be created, and all required Python dependencies will be installed with pip. You can see the entire list of packages and their exact versions that pip will use, in requirements.txt in the root of the repository.
venvUtils/ensureVenv.py contains the logic to check for, create and update the virtual environment.
If a previous virtual environment exists but has a miss-matching Python version or pip package requirements have changed, The virtual environment is recreated with the updated version of Python and packages.
If a virtual environment is found but does not seem to be ours, the user is asked if it should be overwritten or not.
This script also detects if it is being run from an existing 3rd party Python virtual environment and aborts if so. thus, it is impossible to execute SCons or NVDA from source within another Python virtual environment.
venvUtils/ensureAndActivate.bat can be used to ensure the virtual environment exists and is up to date, and then activates it in the current shell, ready for other commands to be executed in the context of NVDA's build system Python virtual environment. this would never normally be executed by itself, though appVeyor uses it at the beginning of its execution and leaves the environment active for the remainder of its execution.
venvUtils/venvCmd.bat is a script that runs a single command within the context of the NVDA build system Python virtual environment. It ensures and activates the environment, executes the command, and then deactivates the environment. this script is what all the high-level batch files use internally.
SConstruct, and source/nvda.pyw both contain logic that detects the NVDA build system Python virtual environment, and abort if it is not active.
As this PR is specific for Python 3.8, it also upgrades the following Python dependencies:
wxPython to 4.1.1
pySerial to 3.5
py2exe to 0.10.1.0

Testing strategy:

Compiled NVDA locally with scons source, scons dist and scons launcher
Run unit tests and system tests locally with no errors.
Successfully generated try builds on appVeyor.

Known issues with pull request:

None so far.

Change log entry:

Changes for developers:

- NVDA now requires Python 3.8.
- NVDA's build system now fetches all Python dependencies with pip and stores them in a Python virtual environment. This is all done transparently.
* To build NVDA, SCons should continue to be used in the usual way. E.g. executing scons.bat in the root of the repository. Running ``py -m SCons``  is no longer supported, and ``scons.py`` has also been removed. 
* To run NVDA from source, rather than executing ``source/nvda.pyw`` directly, the developer should now use ``runnvda.bat`` in the root of the repository. If you do try to execute ``source/nvda.pyw``, a message box will alert you this is no longer supported.
* To perform unit tests, execute ``rununittests.bat [<extra unittest discover options>]``
* To perform system tests: execute ``runsystemtests.bat [<extra robot options>] ``
* To perform linting, execute ``runlint.bat <base branch>``

Code Review Checklist:

This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review and confirm you have considered the following items.

* SConstruct: specify Pyhton 3.8. * SConstruct: remove Python 3.7.6 check. The latest version of Python 3.7 is 3.7.9 (as of February 2021). Therefore remove 3.7.6 check. * SConstruct: update copyright year * SCons.bat: run SCons under Python 3.8 (32-bit). * Revert "SConstruct: update copyright year" This reverts commit 096f61b due to impending Py2Exe upgrade that will update copyright header. * scons.bat: update comment * Readme: mention Python 3.8. * appveyor.yml: Instruct appveyor to use Python 3.8 Co-authored-by: Michael Curran <mick@nvaccess.org>

…pip.

…f the repository. Remove the existing requirements.txt files and logic for devDocs and system tests and linting. Have appveyor fetch all Python dependencies at the beginning of the build.

…scons.exe rather than our scons.py Remove scons.bat and scons.py.

AppVeyorBot · 2021-02-15T23:06:04Z

Build (for testing PR)

See test results for failed build of commit ff3342a2be

lukaszgo1

Please add .venv to the .gitignore.
Why exactly you've deleted scons.bat and sconns.py when we would look into automating this process they probably would need to be reintroduced in some form.

appveyor.yml

+ # Ensure we have the latest version of pip
+ - py -m pip install --upgrade pip
+ # Create a Python virtual environment
+ - py -3.8-32 -m venv .venv


requirements.txt

…s as it is set globally.

…ly import some of our modules and then break because of globalVars.appDir not existing yet.

…d_dll_directory (#12020) * Louis helper: update copyright header * Louis helper/Python 3.8: call os.add_dll_directory when importing libluis DLL. Python 3.8 adds os.add_dll_directory to securely load DLL's from known locations, which affects Louis helper when trying to load liblouis.dll from NVDA executable folder. Therefore add NVDA executable path to known DLL directories through os.add_dll_directory, which resolves ModuleNotFoundError. * Louis helper: remove header comment, use globalVars.appDir as executable path. Reviewed by Lukasz Golonka: remove file name from header comment. Also, because __file__ will point to library.zip, use globalVars.appDir to locate NVDA executable where liblouis.dll actually resides. * Louis helper: use globalVars.appDir directly rather than going through os.path.dirname. Pointed out by Lukasz Golonka: globalVars.appDir records the actual path to the executable, including in source code version (nvda.pyw). Therefore use app dir directly.

… source version of NVDA

…required for locating unit tests in Python 3.8.

…ger executes callbacks while NVDA's main thread is within apopup menu or message box. (#12072) Monkeypatch wx.CallAfter to post a WM_NULL message to our top-level window after calling the original CallAfter, which causes wx's event loop to wake up enough to execute the callback.

…ystem tests by robotremoteserver, but it can't seem to be imported from our NVDA profile directory.

MarcoZehe · 2021-02-18T10:19:48Z

readme.md

+To activate the virtual environment:
+```
+.venv\scripts\activate.bat
+```


Actually, I would switch the paragraphs around and simply instruct users to first create a virtual environment and then run the pip command. Since this is what AppVeyor will also do, and it is good practice anyway, why bother with howevers and maybes? Just my 2 Cents.

I agree. I think we should move all the "doing a local build" stuff into a dedicated file or at least section. It should be ordered so it can be followed step by step.

… system. It is now possible to run the scons command the same way you normally would, with out worrying about having to create or activate a virtual environment. This is now done behind the scenes for you. Note that you must execute scons, or scons.bat though, not py -m SCons or some other invocation. To run NVDA from source, it is now necessary to execute runnvda (runnvda.bat) in the root of the repository. This script will block until NVDA exits, though you can still run it in the background by doing ``` start /b runnvda ``` Note that trying to execute source\nvda.pyw directly will now display an error as the NVDA build system Python virtual environment will not be detected doing it this way. Behind the scenes, SCons and runnvda ensure that the Python virtual environment is created and up to date, activates the environment, runs the command and then deactivates. All transparently. A developer should not have to know about the Python virtual environment at all. venvUtils/ensureVenv.py contains the logic to check for, create and update the virtual environment. If a previous virtual environment exists but has a miss-matching Python version or pip package requirements have changed, The virtual environement is recreated with the updated version of Python and packages. If a virtual environement is found but does not seem to be ours, the user is asked if it should be overwritten or not. This script also detects if it is being run from an existing 3rd party Python virtual environment and aborts if so. thus, it is impossible to execute SScons or NVDA from source within another Python virtual environment. venvUtils/ensureAndActivate.bat can be used to ensure the virtual environment exists and is up to date, and then activates it in the current shell, ready for other commands to be executed in the context of NVDA's build system Python virtual environment. this would never nroamlly be executed by itself, though appveyor uses it at the beginning of its execution and leaves the environment active for the remainder of its execution. venvUtils/venvCmd.bat is a script that runs a single command within the context of the NVDA build system Python virtual environment. It ensures and activates the environment, executes the command, and then deactivates the environment. this script is what scons.bat and runnvda.bat use internally. SConstruct, and source/nvda.pyw both contain logic that detects the NVDA build system Python virtual environment, and abort if it is not active.

…herwise appveyor renders some messages in the wrong order.

nvdaHelper/espeak/sconscript

@@ -1,4 +1,4 @@
-from include.scons.SCons.Util import CLVar
+from SCons.Util import CLVar


source/nvda.pyw

@@ -10,6 +10,7 @@
 import sys
 import os
 import globalVars
+import ctypes


appveyor.yml

@@ -59,6 +59,12 @@ init:
 clone_depth: 1

 install:
+ # Ensure we have the latest version of pip
+ - py -m pip install --upgrade pip


lukaszgo1 · 2021-02-24T12:10:53Z

Also for whatever reason NVDA fails to exit properly after install on AppVeyor with the following log:


CRITICAL - watchdog._crashHandler (10:38:49.715) - MainThread (4732):
NVDA crashed! Minidump written to C:\projects\nvda\testOutput\nvda_install.log\..\nvda_crash.dmp
INFO - watchdog._crashHandler (10:38:49.715) - MainThread (4732):
Listing stacks for Python threads:
Python stack for thread 5020 (watchdog):
  File "threading.pyc", line 890, in _bootstrap
  File "threading.pyc", line 932, in _bootstrap_inner
  File "threading.pyc", line 870, in run
  File "watchdog.pyc", line 112, in _watcher
  File "winKernel.pyc", line 228, in waitForSingleObject

Python stack for thread 5056 (winInputHook):
  File "threading.pyc", line 890, in _bootstrap
  File "threading.pyc", line 932, in _bootstrap_inner
  File "threading.pyc", line 870, in run
  File "winInputHook.pyc", line 79, in hookThreadFunc

Python stack for thread 3044 (_UIAHandler.UIAHandler.MTAThread):
  File "threading.pyc", line 890, in _bootstrap
  File "threading.pyc", line 932, in _bootstrap_inner
  File "threading.pyc", line 870, in run
  File "_UIAHandler.pyc", line 329, in MTAThreadFunc
  File "queue.pyc", line 170, in get
  File "threading.pyc", line 302, in wait

Python stack for thread 1068 (braille._BgThread):
  File "threading.pyc", line 890, in _bootstrap
  File "threading.pyc", line 932, in _bootstrap_inner
  File "threading.pyc", line 870, in run
  File "braille.pyc", line 2193, in func

Python stack for thread 4732 (MainThread):
  File "nvda.pyw", line 261, in <module>
  File "core.pyc", line 576, in main
  File "wx\core.pyc", line 2237, in MainLoop
  File "watchdog.pyc", line 213, in _crashHandler
  File "watchdog.pyc", line 63, in getFormattedStacksForAllThreads

INFO - watchdog._crashHandler (10:38:49.715) - MainThread (4732):
Restarting due to crash

…shell, or at least in onFinish, the Python environement is not maintained over commands, and therefore we were fetching the global list.

Latest version of Py2exe no longer invalidates signature of Python's dll so this workaround is no longer needed

) * Python 3.8: Mention that on Windows 7 KB3063858 is rewquired * Availabe -> available

…--in pgettext (#12109) * Add-on handler/pgettext: remove pgettext namespace definition. Re #9207. As gettext.pgettext is present, there is no need to manually assign NVDA's own pgettext function to built-in namespace for add-ons. Therefore remove this assignment. * Language handler/pgettext: remove nVDA-specific pgettext function. re #9207. As Python 3.8 includes gettext.pgettext, there is no need for NVDA's implementation of pgettext function. Therefore remove makePgettext and builtins namespace assignment from language handler. * Language handler: remove unused builtins namespace import. * Language handler/pgettext: add 'pgettext' to built-in namespace. Re #9207. As Python 3.8 includes gettext.pgettext, add it to built-in namespace by using 'names' parameter of translation.install method. * Update copyright header * Revert "Add-on handler/pgettext: remove pgettext namespace definition. Re #9207." This reverts commit 991de43 so add-on translations will work. * Add-on handler/pgettext: replace NVDA-specific pgettext call with Python's pgettext function. Re #9207. Add-on initialization will still require assigning pgettext to add-on namespace, therefore use translations.pgettext instead of now removed NVDA-specific implementation.

feerrenrut

Looks good to me. Let's rebase this on master and merge straight to master.

feerrenrut · 2021-03-11T04:34:56Z

On second thoughts, no rebase is necessary. Just make sure this is up to date with master and change the target (base) branch to master.

LeonarddeR · 2021-03-11T06:44:30Z

Let's rebase this on master and merge straight to master.

Note that #11912 is not yet part of master. Also this should be up to date with the py3.8 branch before merged into master, I assume?

Link to issue number: None - Fixes regression introduced in PR #12075 Summary of the issue: When executing system tests ensuring that NVDA starts and exits correctly we rely on the status of its process i.e. when the process is running NVDA is supposed to be running. While this works well for installed copies source copies are not started directly rather intermediate .bat file is used to start pythonw in a separate process. As a consequence when trying to retrieve status of an NVDA process during tests we get wrong results (the bat file is never running). Description of how this pull request fixes the issue: To avoid relying on the state of the process system tests use the NVDA's message window to determine if NVDA is running or not.

josephsl and others added 6 commits February 15, 2021 16:31

Remove any git submodules to distributions we can instead fetch with …

5e87120

…pip.

List all required Python packages in a requirements.txt in the root o…

c8fc42b

…f the repository. Remove the existing requirements.txt files and logic for devDocs and system tests and linting. Have appveyor fetch all Python dependencies at the beginning of the build.

Ssconscripts: import SCons correctly.

9c9d1e1

Appveyor: Create and activate a Python virtual environment. Also use …

3878133

…scons.exe rather than our scons.py Remove scons.bat and scons.py.

Update readme.md

90a7183

michaelDCurran changed the base branch from master to py3.8 February 15, 2021 22:57

lukaszgo1 reviewed Feb 15, 2021

View reviewed changes

appveyor.yml Outdated

# Ensure we have the latest version of pip

- py -m pip install --upgrade pip

# Create a Python virtual environment

- py -3.8-32 -m venv .venv

This comment was marked as outdated.

Sign in to view

requirements.txt Show resolved Hide resolved

michaelDCurran and others added 13 commits February 16, 2021 10:08

appveyor.yml: no need to specifify python version in specific command…

cd4a3cf

…s as it is set globally.

.gitignore: ignore the .venv directory

9503492

setup.py: don't package numpy with NVDA.

fd890d3

Use py2exe 0.10.1.0 rather than py2exe 0.10.2.0 which seems to actual…

4a852d9

…ly import some of our modules and then break because of globalVars.appDir not existing yet.

Merge branch 'py3.8' into py3.8_manualVenv

6d58baa

Use comtypes 1.1.7.

3025d74

System tests: don't specify Python 3.7 when running system tests with…

8074fa1

… source version of NVDA

appveyor: run nose with the --traverse-namespace option as it is now …

c22b7b3

…required for locating unit tests in Python 3.8.

Update to miscDeps containing brlAPI for Python 3.8

0179e48

Merge remote-tracking branch 'origin/py3.8' into py3.8_manualVenv

cbd21be

Explicitly include xmlrpc.server with py2exe, as it is required for s…

6f427af

…ystem tests by robotremoteserver, but it can't seem to be imported from our NVDA profile directory.

MarcoZehe reviewed Feb 18, 2021

View reviewed changes

sourceEnv: stop the user from running with no virtual environment.

e59018e

LeonarddeR mentioned this pull request Feb 23, 2021

Update pySerial to 3.5 #12038

Closed

7 tasks

michaelDCurran added 3 commits February 24, 2021 18:42

Fix typo

ef52123

ensureVenv.py: flush standard out before running any sub processes ot…

e7a54fd

…herwise appveyor renders some messages in the wrong order.

lukaszgo1 reviewed Feb 24, 2021

View reviewed changes

Call pip freeze from a batch file in appveyor, as it seems that power…

110edc1

…shell, or at least in onFinish, the Python environement is not maintained over commands, and therefore we were fetching the global list.

lukaszgo1 mentioned this pull request Mar 9, 2021

Python 3.8: Mention additional system requirements for Windows 7 #12141

Merged

7 tasks

feerrenrut and others added 7 commits March 10, 2021 19:01

Update buildSystemNotes

778e512

buildSystemNotes: address some typos.

06411a0

Remove workaround for issue #9762 (#12142)

9b1c16a

Latest version of Py2exe no longer invalidates signature of Python's dll so this workaround is no longer needed

Python 3.8: Mention additional system requirements for Windows 7 (#12141

cc7d219

) * Python 3.8: Mention that on Windows 7 KB3063858 is rewquired * Availabe -> available

Merge branch 'master' into py3.8

0dbdaed

Merge branch 'py3.8' into py3.8_manualVenv

49ea5ae

feerrenrut approved these changes Mar 11, 2021

View reviewed changes

michaelDCurran changed the base branch from py3.8 to master March 11, 2021 07:30

michaelDCurran added 2 commits March 11, 2021 17:38

Update what's new

7a6fc4d

appveyor: remove mention of py3.8 branch, no longer needed.

cca866a

michaelDCurran merged commit 528d570 into master Mar 11, 2021

michaelDCurran deleted the py3.8_manualVenv branch March 11, 2021 08:07

nvaccessAuto added this to the 2021.1 milestone Mar 11, 2021

dpy013 mentioned this pull request Mar 11, 2021

Upgrade NVDA source code to Python 3.8 #10383

Closed

CyrilleB79 mentioned this pull request Mar 14, 2021

Last NVDA alpha - Unable to rename folder of source code #12166

Closed

lukaszgo1 mentioned this pull request Mar 14, 2021

Add diff used for linting changes to the list of files ignored by git #12169

Merged

7 tasks

CyrilleB79 mentioned this pull request Mar 15, 2021

recreate or abort if Python virtual environment has been moved from where it was originally located #12171

Closed

7 tasks

This was referenced Mar 16, 2021

Task: Update to latest (most appropriate) version of Python #12064

Closed

BRLAPI for Python 3.8 nvaccess/nvda-misc-deps#20

Merged

Update BRLTTY for Python 3.8 #12018

Closed

lukaszgo1 mentioned this pull request Apr 17, 2021

Allow linting without Visual studio installed #11774

Closed

lukaszgo1 mentioned this pull request May 7, 2021

NVDA Alpha fails to clean its system tray icon after restart #12238

Closed

lukaszgo1 mentioned this pull request Feb 9, 2022

Fix system tests for source copies of NVDA #13322

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Py3.8: Transparently use a Python virtual environment under the hood #12075

Py3.8: Transparently use a Python virtual environment under the hood #12075

michaelDCurran commented Feb 15, 2021 •

edited

Loading

AppVeyorBot commented Feb 15, 2021

lukaszgo1 left a comment

This comment was marked as outdated.

MarcoZehe Feb 18, 2021

feerrenrut Feb 18, 2021

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

lukaszgo1 commented Feb 24, 2021

feerrenrut left a comment

feerrenrut commented Mar 11, 2021

LeonarddeR commented Mar 11, 2021 •

edited

Loading

		@@ -1,4 +1,4 @@
		from include.scons.SCons.Util import CLVar
		from SCons.Util import CLVar

Py3.8: Transparently use a Python virtual environment under the hood #12075

Py3.8: Transparently use a Python virtual environment under the hood #12075

Conversation

michaelDCurran commented Feb 15, 2021 • edited Loading

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing strategy:

Known issues with pull request:

Change log entry:

Code Review Checklist:

AppVeyorBot commented Feb 15, 2021

lukaszgo1 left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

MarcoZehe Feb 18, 2021

Choose a reason for hiding this comment

feerrenrut Feb 18, 2021

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

lukaszgo1 commented Feb 24, 2021

feerrenrut left a comment

Choose a reason for hiding this comment

feerrenrut commented Mar 11, 2021

LeonarddeR commented Mar 11, 2021 • edited Loading

michaelDCurran commented Feb 15, 2021 •

edited

Loading

LeonarddeR commented Mar 11, 2021 •

edited

Loading