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

Compile l10nUtil.exe with py2exe as one of NVDA's executables, rather than using nuitka which took way too long to compile. #17712

Merged
merged 5 commits into from
Feb 23, 2025

Conversation

michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Feb 20, 2025

Link to issue number:

None

Summary of the issue:

l10nUtil (which allows translators to download and upload to Crowdin, and convert xliffs to markdown and html) s currently build using nuitka. However, this takes a significant amount of time. On Appveyor this takes roughly 20 minutes, which is more than a rd of the total build time.In many cases, this pushes the buildtime past 60 minutes and the build is cancelled by appveyor.

Description of user facing changes

None.

Description of development approach

Instead of using Nuitka, compile l10nUtil.exe as one of NVDA's executables using py2exe.
Several modules and packages in lxml and markdown extensions needed to be explicitly listed in setup.py for this to work. Also, l1nutil.py, markdownTranslate.py, md2html.py and keyCommandsDoc.py were moved from user_docs to source, which made it easier to handle with py2exe.
l10nUtil.exe still works just the same, and build time is cut down by at least 19 minutes.

Testing strategy:

Tested l10nUtil downloadTranslationFile, xliff2md and xliff2html.

Known issues with pull request:

To get l10nUtil.exe you now need to download NVDA and eitherinstall or make a portable copy, as l10nUtil.exe is one of the executables inside.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

… than using nuitka which took way too long to compile.
@michaelDCurran michaelDCurran requested a review from a team as a code owner February 20, 2025 03:12
@AppVeyorBot
Copy link

See test results for failed build of commit 115a627d2a

@hwf1324
Copy link
Contributor

hwf1324 commented Feb 20, 2025

Can you clean up the other trailing spaces in requirements.txt by the way?

@AppVeyorBot
Copy link

See test results for failed build of commit 199a1e7f65

@AppVeyorBot
Copy link

See test results for failed build of commit 49174e2afc

@wmhn1872265132
Copy link
Contributor

Need to be updated.github/workflows/regenerate_english_userDocs_translation_source.yml

python user_docs/markdownTranslate.py updateXliff -x $xliff -m $file -o $tempXliff

@SaschaCowley
Copy link
Member

@michaelDCurran

To get l10nUtil.exe you now need to download NVDA and eitherinstall or make a portable copy, as l10nUtil.exe is one of the executables inside.

Couldn't we retain the command to build l10nutil, but make it depend on dist, (or source) and then copy l10nutil from there? We could optionally delete l10nutil from dist (and thus launcher) if we wanted to, since most users won't need it

@CyrilleB79
Copy link
Collaborator

How big is it? If it is not too big, from a translator's point of view, it's quite nice to have it already packed in NVDA install.

@michaelDCurran michaelDCurran merged commit e6e443b into master Feb 23, 2025
5 checks passed
@michaelDCurran michaelDCurran deleted the l10nUtil branch February 23, 2025 20:31
@github-actions github-actions bot added this to the 2025.1 milestone Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants