Avoid building dlls twice due to signing postAction#20084
Avoid building dlls twice due to signing postAction#20084michaelDCurran wants to merge 8 commits into
Conversation
… builder that copies a source file to the target, and then signs the target all in the same action, thus Scons treats the target as complete once signed. Replace any usage of env.AddPostAction and then env.Install with env.copyAndSign. This should all mean that re-running the build should not treat a target as being changed just because it was signed, then causing the target to be re-build and re-signed inappropriately.
…equiring signing, by inserting the signing action only if signing is enabled.
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent redundant rebuilds (and related symbol mismatches) caused by code-signing implemented as SCons post-actions, by moving signing into the primary build/copy actions so targets are considered “complete” only after signing.
Changes:
- Introduces an
env.copyAndSignhelper to copy outputs into their install locations and sign them within the same SCons action sequence. - Replaces several
AddPostAction(...sign...)+Install(...)patterns in nvdaHelper/third-party builds withcopyAndSign. - Updates CI SCons invocation to build with
--all-cores(supported via custom option insconstruct).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
sconstruct |
Adds copyAndSign, moves launcher/uninstaller signing into main actions, and includes a minor comment tweak. |
nvdaHelper/liblouis/sconscript |
Switches liblouis install/sign flow to use env.copyAndSign. |
nvdaHelper/archBuild_sconscript |
Switches multiple helper library install/sign flows to use env.copyAndSign. |
ci/scripts/setSconsArgs.ps1 |
Changes CI to pass --all-cores instead of -j1. |
…f this method have been changed to pass a single file, and then Install any remaining files separately.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Unfortunately even with these changes |
|
Is it possible to determine the reason for rebuilding these files by using Perhaps it is also possible to decide whether to build files by using a custom |
…ider to be content rather than timestamp for the signed files as otherwise Github actions caching timestamps seem to confuse SCons and cause a rebuild.
|
Closing in favor of #20129. |
… doesn't do it, causing uploaded symbols to mismatch (#20129) Fixes #20033 Replaces pr #20084 Summary of the issue: The debug symbols archived from builds of NvDA do not match the actual files in the final NVDA launcher, if files are required to be signed. This is due to the fact that the API signing key was never set in the CI environment when building NVDA with scons source, although it was present when running scons launcher. NVDA would be initially built without signed files, and these files would be collected for the symbols, but then as the launcher was being created, the files would be retroactively signed. Also if trying to run CI instructing SCons to use multiple cores to speed up builds, some of the files may have been signed after the launcher was created, thus leaving them as the old unsigned copies. Signing with signpath takes a very long time, and can add up to 10 minutes for a build when using a single core. Thus signing properly in scons source would address the incorrect symbols and allow for building using multiple cores. Description of user facing changes: Description of developer facing changes: Description of development approach: Set the API signing token in the environment for the set scons args step in the Build NVDA job. this mirrors what is already done for the Create Launcher job. In the Create Launcher job, Split the powershell commands to install signpath out of the Create launcher step and into its own Install Signpath step. Add an Install Signpath step to the Build NVDA job, mirroring the step in the Create launcher job. In the Create launcher job, split out the building of the NVDA controller client from the Create Launcher step into its own Create NVDA controller client step. Remove the sconsCores variable from the scons launcher command in the Create Launcher step, meaning that scons launcher will always run with only one core, no matter what sconsCores is set to. It is very important that scons launcher never run with multiple cores, as if it ever has any dependencies, they must be build before the NSIS launcher builds. change the sconsCores variable to be --all-cores meaning that scons source, scons user_docs, scons client etc (but not scons launcher) will now run using all available cores). The biggest advantage here is signing files in scons source which dramatically decreases time as files can be signed in parallel to further source building.
Link to issue number:
Fixes #20033
Summary of the issue:
Signing of dlls and executables during NVDA build was implemented in SCons as post actions, as in once the dll or exe target was fully created, then a separate post-action for signing was run. The problems with this approach are:
Description of user facing changes:
Description of developer facing changes:
Description of development approach:
Testing strategy:
Known issues with pull request:
None known.
Code Review Checklist: