Add scripts to allow addons from personal repos to be synchronized with Crowdin#1
Add scripts to allow addons from personal repos to be synchronized with Crowdin#1nvdaes wants to merge 58 commits intonvaccess:masterfrom
Conversation
… addedwith dev role to Crowdin if they use a project not owned by them to upload source files)
…flow to upload/update files in Crowdin
…k pass creating a PR at nvdaes/translateNvdaaddonsWithCrowdin repo
| strictSetInference = true | ||
|
|
||
| # Compliant rules | ||
| reportAbstractUsage = true |
There was a problem hiding this comment.
it's probably better to keep these rules than dropping to NVDA's standard
PurposeAdd-on authors may wish to help translators use Crowdin, the same framework where they translate NVDA. to translate messages and documentation for maintained add-ons: Other details
Development approachUse the Crowdin registration repo to add scripts usable by individual add-ons in personal repos. |
|
I've tested that all check pass using this pyproject.toml file on this PR: nvdaes/translateNvdaAddonsWithCrowdin#11 I use precommit, CodeQL and a workflow to check that all translatable messages have comments for translators. I'll try to use the cache action to cache some add-on metadata like its id, and also hashfiles from l10nSources (taking the value of buildVars.py), and the hasf¡hfile of the readme.md, to determine if pot and xliff files should be updated. |
|
Export translations to Crowdin running the workflow with update=False works properly: https://github.com/nvdaes/translateNvdaAddonsWithCrowdin/actions/runs/19802210157 |
|
This time, updatexLiff is failing. Seems that adding blank lines to readme may cause problems: https://github.com/nvdaes/translateNvdaAddonsWithCrowdin/actions/runs/19802391926/job/56731562709 |
|
If someone can help with this issue when update xliff, I'll be grateful. |
|
It might be easier to avoid xliff and just translate the markdown files directly. This won't support diffs very well but worth experimenting with |
|
@seanbudd wrote:
OK. |
|
@CyrilleB79, you were interested in this framework. If you want, feel free to see how the translateNvdaAddonsWithCrowdin.md can be translated in the project. Using xliff files is causing problems, as mentioned, and we are experimenting uploading md files instead. |
|
@nvdaes - now that nvaccess/nvdaL10n#1 is merged, can we use the l10nUtil exe from https://github.com/nvaccess/nvdaL10n/actions/runs/23521126188 here in #1 and close #7? |
|
@seanbudd wrote:
I think it would be better to build the executable by checking out the repo, since the artifact may be deleted after the retention period. Let me know if you disagree. |
|
I disagree - instead we could create a release and upload the artifact with every master push |
|
To be clear - I think we should be building the release from nvaccess/nvdal10n. This repo and NVDA only needs the exe |
|
I'm testing in the reportSymbols add-on, using the buildAddon workflow to build the pot file, though seems that Gettext can be installed on Windows too. But the buildAddon workflow is failing at least due to Pyright. https://github.com/nvdaes/reportSymbols/actions/runs/23554914090/job/68578896225 |
|
Even removing code checks from build, the crowdin workflow is failing due to absence of modules like sha256. I'm thinking about uploading source files in all cases instead of checking if they should uploaded from scratch or updated, since the same command is used now. Previously the crowdinSync.py file was used for updates, but this is not the case now. |
|
Seems that it's difficult to download the l10nUtil.exe file using gh (GitHub CLI), and it's not possible with curl. |
|
@nvdaes - can you create a workflow in |
Requested in nvaccess/AddonTemplate#1 Summary This creates a release when pushing to the main branch, uploading the l10nUtil.exe as an asset. Additional note I've included dependency updates from dependabot. @seanbudd , we can revert uv.lock if you want. We use the ncipollo/release action since I find it more updated than other similar actions and it uses node24. See
|
I think that it would be better to use the nvdaaddons/crowdinRegistration repo, or this template, to export translations periodically, and add-on authors would use their secrets.CROWDIN_TOKEN just to upload md and pot file to Crowdin. https://github.com/nvdaes/readFeeds/actions/runs/23920196250/job/69764310501 |
|
Anotheridea that I'm thinking about is to create an action (nvdaes/l10n or something like that), like the build-discussion action for the store. This potentially would make possible to create a branch with translations for personal repos. I can determine where to export translations on the action repo. Also, I can parse the json file with approved submitters available on the store to upload source files (readme.md and nvda.pot) for each available add-on, periodically. |
There was a problem hiding this comment.
Pull request overview
Adds GitHub Actions automation and helper scripts to sync an add-on’s documentation/messages with Crowdin and pull translated outputs back into the repository, building on the translations submodule work in #7.
Changes:
- Introduces a Crowdin localization workflow plus a Python helper to compute update flags/outputs.
- Adds a SHA-256 helper script and documents the intended translation workflow in the README.
- Updates repo tooling/configuration (Python version pin, pre-commit hook, .gitignore, reusable build workflow support).
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
sha256.py |
Adds a CLI/helper for SHA-256 hashing (used by the workflow helper). |
.github/workflows/setOutputs.py |
Computes hashes/update flags and emits outputs for the Crowdin workflow. |
.github/workflows/crowdinL10n.yml |
New scheduled/manual workflow to upload sources and download translations. |
.github/workflows/build_addon.yml |
Makes the build workflow reusable via workflow_call. |
readme.md |
Documents the Crowdin translation workflow. |
.python-version |
Pins Python version for tooling/CI that reads this file. |
.pre-commit-config.yaml |
Adds a lockfile verification hook and related config. |
.gitignore |
Expands ignored Python/build artifacts and add-on generated files. |
buildVars.py |
Minor whitespace cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def sha256_checksum(binaryReadModeFiles: list[typing.BinaryIO], blockSize: int = BLOCK_SIZE): | ||
| """ | ||
| :param binaryReadModeFiles: A list of files (mode=='rb'). Calculate its sha256 hash. | ||
| :param blockSize: The size of each read. | ||
| :return: The Sha256 hex digest. | ||
| """ | ||
| sha256 = hashlib.sha256() | ||
| for f in binaryReadModeFiles: | ||
| with open(f, "rb") as file: | ||
| assert file.readable() and file.mode == "rb" | ||
| for block in iter(lambda: file.read(blockSize), b""): | ||
| sha256.update(block) |
There was a problem hiding this comment.
sha256_checksum iterates over binaryReadModeFiles and then calls open(f, "rb"). This breaks for actual BinaryIO objects (what the type annotation/docstring claims) and currently only works if f is a path-like. Align the API: either accept paths (Sequence[str | os.PathLike]) or accept file objects and hash their .read() without reopening them.
| def sha256_checksum(binaryReadModeFiles: list[typing.BinaryIO], blockSize: int = BLOCK_SIZE): | |
| """ | |
| :param binaryReadModeFiles: A list of files (mode=='rb'). Calculate its sha256 hash. | |
| :param blockSize: The size of each read. | |
| :return: The Sha256 hex digest. | |
| """ | |
| sha256 = hashlib.sha256() | |
| for f in binaryReadModeFiles: | |
| with open(f, "rb") as file: | |
| assert file.readable() and file.mode == "rb" | |
| for block in iter(lambda: file.read(blockSize), b""): | |
| sha256.update(block) | |
| def sha256_checksum( | |
| binaryReadModeFiles: typing.BinaryIO | list[typing.BinaryIO], | |
| blockSize: int = BLOCK_SIZE, | |
| ): | |
| """ | |
| :param binaryReadModeFiles: A file or list of files already opened in binary read mode | |
| (mode=='rb'). Calculate their combined sha256 hash. | |
| :param blockSize: The size of each read. | |
| :return: The Sha256 hex digest. | |
| """ | |
| sha256 = hashlib.sha256() | |
| files = ( | |
| [binaryReadModeFiles] | |
| if hasattr(binaryReadModeFiles, "read") | |
| else binaryReadModeFiles | |
| ) | |
| for file in files: | |
| assert file.readable() | |
| for block in iter(lambda: file.read(blockSize), b""): | |
| sha256.update(block) |
| def main(): | ||
| parser = argparse.ArgumentParser() | ||
| parser.add_argument( | ||
| type=argparse.FileType("rb"), | ||
| dest="file", | ||
| help="The NVDA addon (*.nvda-addon) to use when computing the sha256.", | ||
| ) | ||
| args = parser.parse_args() | ||
| checksum = sha256_checksum(args.file) | ||
| print(f"Sha256:\t {checksum}") |
There was a problem hiding this comment.
argparse.FileType("rb") returns a single open file handle (not a list), but sha256_checksum expects an iterable and then re-opens each element. This will raise at runtime. Consider parsing one-or-more file paths (e.g., nargs='+') or changing sha256_checksum to accept a single file handle and/or an iterable of file handles without re-opening.
| hashFile = os.path.join(os.getcwd(), "hash.json") | ||
| data = dict() | ||
| if os.path.isfile(hashFile): | ||
| with open(hashFile, "rt") as f: | ||
| data = json.load(f) | ||
| shouldUpdateMd = data.get("readmeSha") != readmeSha and data.get("readmeSha") is not None | ||
| shouldUpdatePot = ( | ||
| data.get("i18nSourcesSha") != i18nSourcesSha and data.get("i18nSourcesSha") is not None | ||
| ) | ||
| shouldAddMdFromScratch = data.get("readmeSha") is None | ||
| shouldAddPotFromScratch = data.get("i18nSourcesSha") is None | ||
| if readmeSha is not None: | ||
| data["readmeSha"] = readmeSha | ||
| if i18nSourcesSha is not None: | ||
| data["i18nSourcesSha"] = i18nSourcesSha | ||
| with open(hashFile, "wt", encoding="utf-8") as f: | ||
| json.dump(data, f, indent="\t", ensure_ascii=False) | ||
| name = "addonId" | ||
| value = addonId | ||
| name0 = "shouldUpdateMd" | ||
| value0 = str(shouldUpdateMd).lower() | ||
| name1 = "shouldUpdatePot" | ||
| value1 = str(shouldUpdatePot).lower() | ||
| name2 = "shouldAddMdFromScratch" | ||
| value2 = str(shouldAddMdFromScratch).lower() | ||
| name3 = "shouldAddPotFromScratch" | ||
| value3 = str(shouldAddPotFromScratch).lower() | ||
| with open(os.environ["GITHUB_OUTPUT"], "a") as f: | ||
| f.write(f"{name}={value}\n{name0}={value0}\n{name1}={value1}\n{name2}={value2}\n{name3}={value3}\n") |
There was a problem hiding this comment.
hash.json is written every run, but the workflow only stages/commits addon/locale and addon/doc. If hash.json is not checked in, the next run will treat everything as "from scratch" again and the update detection won’t work. Either include hash.json in the commit/push or store the hashes somewhere persistent (artifact/cache/branch) that survives runs.
| shouldUpdateMd = data.get("readmeSha") != readmeSha and data.get("readmeSha") is not None | ||
| shouldUpdatePot = ( | ||
| data.get("i18nSourcesSha") != i18nSourcesSha and data.get("i18nSourcesSha") is not None | ||
| ) | ||
| shouldAddMdFromScratch = data.get("readmeSha") is None |
There was a problem hiding this comment.
When readme.md is missing, readmeSha stays None but shouldUpdateMd can still become True if hash.json previously had a value, which will make the workflow attempt to Move-Item readme.md and fail. Gate the update/from-scratch flags on readmeSha is not None (or fail fast with a clear error) so the workflow behaves deterministically.
| shouldUpdateMd = data.get("readmeSha") != readmeSha and data.get("readmeSha") is not None | |
| shouldUpdatePot = ( | |
| data.get("i18nSourcesSha") != i18nSourcesSha and data.get("i18nSourcesSha") is not None | |
| ) | |
| shouldAddMdFromScratch = data.get("readmeSha") is None | |
| shouldUpdateMd = ( | |
| readmeSha is not None | |
| and data.get("readmeSha") != readmeSha | |
| and data.get("readmeSha") is not None | |
| ) | |
| shouldUpdatePot = ( | |
| data.get("i18nSourcesSha") != i18nSourcesSha and data.get("i18nSourcesSha") is not None | |
| ) | |
| shouldAddMdFromScratch = readmeSha is not None and data.get("readmeSha") is None |
| uv sync | ||
| uv run ./.github/workflows/setOutputs.py | ||
| - name: Download l10nUtil from nvdal10n | ||
| if: ${{ inputs.dry-run != true }} |
There was a problem hiding this comment.
gh run download ... typically requires an auth token (GH_TOKEN/GITHUB_TOKEN) to be set for GitHub CLI. Without exporting GH_TOKEN: ${{ github.token }} (or similar), this step may fail in non-interactive CI environments.
| if: ${{ inputs.dry-run != true }} | |
| if: ${{ inputs.dry-run != true }} | |
| env: | |
| GH_TOKEN: ${{ github.token }} |
| git add addon/locale addon/doc | ||
| $diff = git diff --staged --quiet | ||
| if ($LASTEXITCODE -eq 0) { | ||
| Write-Host "Nothing added to commit." | ||
| } else { | ||
| git commit -m "Update translations for ${{ steps.getAddonInfo.outputs.addonId }}" | ||
| git checkout -b ${{ env.downloadTranslationsBranch }} | ||
| git push -f --set-upstream origin ${{ env.downloadTranslationsBranch }} |
There was a problem hiding this comment.
The workflow commits and pushes, but doesn’t configure user.name/user.email before git commit. In GitHub Actions this often fails with "Author identity unknown". Add a step to set these (e.g., to github-actions[bot]) before committing.
|
|
||
| - id: uv-lock | ||
| name: Verify uv lock file | ||
| # Override python interpreter from .python-versions as that is too strict for pre-commit.ci |
There was a problem hiding this comment.
The comment references .python-versions, but the repo uses .python-version. This is likely a typo and can mislead contributors.
| # Override python interpreter from .python-versions as that is too strict for pre-commit.ci | |
| # Override python interpreter from .python-version as that is too strict for pre-commit.ci |
| Then, to export your add-on to Crowdin for the first time, run the `.github/workflows/exportAddonsToCrowdin.yml`, ensuring that the update option is set to false. | ||
| When you have updated messages or documentation, run the workflow setting update to true (which is the default option). |
There was a problem hiding this comment.
This documentation references .github/workflows/exportAddonsToCrowdin.yml and an update option, but the repo only adds crowdinL10n.yml (with a dry-run input). Update the README to reference the actual workflow file name and its real inputs so the instructions are actionable.
| Then, to export your add-on to Crowdin for the first time, run the `.github/workflows/exportAddonsToCrowdin.yml`, ensuring that the update option is set to false. | |
| When you have updated messages or documentation, run the workflow setting update to true (which is the default option). | |
| Then, to export your add-on to Crowdin, run the `.github/workflows/crowdinL10n.yml` workflow. | |
| If you want to test the workflow without pushing changes to Crowdin, enable the `dry-run` input; otherwise, leave `dry-run` disabled to upload updated messages or documentation. |
|
I think just using the template to sync with Crowdin is ideal. Is there a need for an external repo? |
|
@seanbudd Is there a need for an external repo? I think we need an external repo. The l10nUtil.exe has the ability to use |
|
I think we should expect add-on authors to set up an API token if they wish to sync with Crowdin. I don't think it should be centralised to a single API token. I think we should try and encourage some level of shared incentive for maintaining the translation infrastructure. Would it be too much work to register add-on authors? I figured they should be registered in Crowdin to maintain their add-ons presence |
|
Registering add-on authors may be a lot of work. I'm having issues to invite a member as a translator via web. Unless we can add an option to do it from l10nUtil.exe using the api without the web. At least for me, the crowdin web is not very intuitive. |
|
If there's an issue registering anyone, that's a big concern right? I imagine there will be just as many translators as add-on authors. Do you think other community members can assist you in admin? |
|
Sean wrote:
Not sure, really, and not sure that this would be maintainable a long time. |
|
So we don't have a good solution for adding translators to the community Crowdin then? Do we need that before this project goes any further? |
|
I think the interface for adding people in Crowdin is a bit dodgy for all users. I've frequently had to cancel/reject invites and try again many times before an invite successfully occurred. |
|
@seanbudd wrote:
Seems that the Crowdin API can be used to manage members, though I don't know if this is available for free. I'll try to add you for testing. Please, feel free to accept or ignore the invitation in case you receive it. |
|
@nvdaes - my Crowdin name is same as GitHub - I do encourage testing with the community though. I think it's really important that the add-on dev community helps support this project. |
|
@seanbudd wrote:
I agree. Otherwise, we hav a lot of things to do in different projects. I'll request for volunteers for this. Good news is that I can print the list of members in the project via l10nUtil.exe. |
Blocked by #7