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

Add-on generation: strictly binary option by removing source code and other files through an optional switch #5781

Closed
josephsl opened this issue Feb 26, 2016 · 1 comment

Comments

@josephsl
Copy link
Collaborator

Hi,

There is a proposal floating around the NVDA add-ons list that advocates creating strictly binary version of add-ons by excluding source code and other files via an optional switch to add-ons sconstruct. A related prposal is to compile .pyc/.pyo files during add-on installation.

Background: When community add-ons are created via SCons, various source code and other files are included into the bundle. These files include .py (Python source code), .po (Gettext source) and .md (Markdown source for add-on help files). Although this allows developers to work with add-ons on the fly, this is perceived as waste of space when end users install add-ons (particularly when the target files (.pyc/.pyo, .mo and .html) are created, thus making source code files unnecessary).

The rationale for the proposal are:

  1. NVDA Core does not include source files (.py, .po, .t2t, etc.) when it is compiled to binaries (technically a pseudo-binary, as nvda.exe still relies on Python runtime system to function). This results in disk space savings. This isn't limited to NVDA - there are mainstream software that are open-source yet only include runtime/binary files in compiled form.
  2. Consequently, end users who'd like to know more about internals of NVDA or read the source code are encouraged to clone NVDA Git repository (and optionally build their own binary builds).
  3. During add-on import phase, each Python source file is compiled into correct bytecode. By providing ready-made bytecode files, this step could be skipped (unless the source itself changes).

Discussion so far:

  1. Originally I wrote to NvDA add-ons list proposing that .po and .md files should be removed during add-on generation.
  2. (Jamie (@jcsteh) noted that, for consistency, .py files should also be removed when creating binary builds.
  3. Some developers objected to this, saying that this may open up possible issues of mismatch between bytecode and source code sent for review (although possibility of disassembly was noted), file size of source code, tweaking add-ons on the fly thanks to readily available source code and so on.
  4. A number of possible solutions were proposed (see below).
  5. Potential issues were noted, including the fact that Python 3.5 drops the concept of .pyo files completely (see PEP 488).

Possible solutions:

Solution 1: compiling .py files during add-on installation: Proposed by Noelia Martinez, this would involve .py files being compiled to .pyc/.pyo files depending on the version of NVDA in use (debug and release, respectively). This has two serious drawbacks:

  1. NVDA Core comes with py_compile. In order to use this, one needs to know the exact path of a .py file to compile. There is no way to tell which optimization flavor to use.
  2. An improved approach is including compileall module and telling it to compile the entire add-on directory tree. However, NVDA Core does not ship with compileall in binary version, and one needs to run this twice if needed (to produce .pyc and .pyo) and specifying destination directory should be restricted.

Implementation: Add-on handler should include a routine to compile add-on directory tree as part of add-on install routine.

Solution 2: Modifying sconstruct file: Proposed by Joseph Lee (me), this solution proposes modifying the sconstruct file used by those compiling add-ons using SCons by:

  1. Introducing two new switches named "package" and "verbose".
  2. The "package" switch (default is False) would build strictly binary form of the add-on (excluding source code). This means bytecode compilation must be the first thing to be done before proceeding (both to prepare the bytecode files and to let Python throw errors). For backward compatibility, this will be set to false (it'll be set to true by the maintainer if releasing add-ons from stable branch or an equivalent one).
  3. The "verbose" switch (set to True by default), if set to false, will suppress messages generated during Gettext and Markdown operation.

There are drawbacks: some may accidentally use "package" switch when they wanted to include source code, and this could be abused by some to create possibly malicious add-on (mismatch between advertisement and inside content).

Solution 3: Custom build script for add-ons: Also proposed by Joseph Lee, this solution proposes creating a custom build script to be used by those who'd like to distribute add-ons. The only job of the script is to compile bytecode and pass in appropriate option to the modified sconstruct (implies solution 2). This allows one to plug in custom action before SCons gets called (for example, creating Python 3 bytecode), with the drawback of code duplication and solution 2 should be adopted in the process.

Comments are welcome. Thanks.

@jcsteh
Copy link
Contributor

jcsteh commented Mar 4, 2016

The only solution that affects NVDA itself is solution 1. However, solution 1 requires a change which brings no noticeable benefit. It doesn't make the add-on smaller, since you still have to include the .py files. The only theoretical benefit, then, is that the files are compiled before the first use. The time to compile is negligible; I've never seen an NVDA add-on where this has been a problem. Furthermore, even if it were slightly noticeable, it would only occur on first restart just after the add-on was installed. Whether you do it before the restart or after makes no difference to the user.

@jcsteh jcsteh closed this as completed Mar 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants