arduino / arduino-ide

Arduino IDE 2.x
https://www.arduino.cc/en/software
GNU Affero General Public License v3.0
2.34k stars 398 forks source link

Remove inaccurate and redundant signing determination code from build system #2567

Closed per1234 closed 1 week ago

per1234 commented 1 week ago

Motivation

The "build" workflow signs the macOS and Windows builds of the application. The signing process relies on access to GitHub Actions secrets. For this reason, the workflow is configured to only sign the builds when it has access to GitHub Actions secrets to avoid spurious failures of the workflow that would otherwise be caused by signing failure.

A flexible general purpose system for accurately determining whether to attempt signing of a build was established years ago (https://github.com/arduino/arduino-ide/pull/1115) However, when the Windows signing system was reworked to support the new "eToken" hardware authenticator-based approach (https://github.com/arduino/arduino-ide/pull/2452), that system was circumvented, causing workflow runs triggered by pull requests to fail spuriously (https://github.com/arduino/arduino-ide/issues/2545). When this was later discovered, an inferior redundant system was added (https://github.com/arduino/arduino-ide/pull/2554) specific to the Windows build instead of just using the existing system.

This redundant system determines whether to sign based on the value of the github.event.pull_request.head.repo.fork context item. That is effective for the use case of the workflow being triggered by a pull request from a fork (for security reasons, GitHub Actions does not give access to secrets under these conditions).

However, there is another context under which the workflow might run without access to the signing secrets, for which the use of context item is not appropriate. It is important to support the use of the workflow in forks of the repository. In addition to the possible value to hard forked projects, this is essential to allow conscientious contributors to test contributions to the build and release system in their own fork prior to submitting a pull request. The previous configuration would cause a workflow run performed by a contributor in a fork to attempt to sign the Windows build. Unless the contributor had set up the ridiculously complex infrastructure required to perform the signing for the Windows build (https://github.com/arduino/arduino-ide/pull/2452), which is utterly infeasible, this would cause the workflow to fail spuriously:

Custom windows signing was no performed one of the following variables was not provided: SIGNTOOL_PATH (C:/Program Files (x86)/Windows Kits/10/bin/10.0.19041.0/x86/signtool.exe), INSTALLER_CERT_WINDOWS_CERT (C:/Users/RUNNER~1/AppData/Local/Temp/cert.cer), CERT_PASSWORD (), CONTAINER_NAME ()
file:///D:/a/arduino-ide/arduino-ide/electron-app/node_modules/execa/lib/error.js:60
        error = new Error(message);
                ^

Error: Command failed with exit code 1: electron-builder --publish never -c.electronVersion 27.0.3 -c.extraMetadata.version 2.3.4 -c.extraMetadata.name arduino-ide -c.win.artifactName arduino-ide_2.3.4_Windows_64bit.${ext} -c.extraMetadata.theia.frontend.config.appVersion 2.3.4 -c.extraMetadata.theia.frontend.config.cliVersion 1.0.4 -c.extraMetadata.theia.frontend.config.buildDate 2024-11-16T13:27:51.643Z -c.extraMetadata.main ./arduino-ide-electron-main.js
    at makeError (file:///D:/a/arduino-ide/arduino-ide/electron-app/node_modules/execa/lib/error.js:60:11)
    at handlePromise (file:///D:/a/arduino-ide/arduino-ide/electron-app/node_modules/execa/index.js:124:26)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async exec (D:\a\arduino-ide\arduino-ide\electron-app\scripts\package.js:162:22)
    at async run (D:\a\arduino-ide\arduino-ide\electron-app\scripts\package.js:49:3) {
  shortMessage: 'Command failed with exit code 1: electron-builder --publish never -c.electronVersion 27.0.3 -c.extraMetadata.version 2.3.4 -c.extraMetadata.name arduino-ide -c.win.artifactName arduino-ide_2.3.4_Windows_64bit.${ext} -c.extraMetadata.theia.frontend.config.appVersion 2.3.4 -c.extraMetadata.theia.frontend.config.cliVersion 1.0.4 -c.extraMetadata.theia.frontend.config.buildDate 2024-11-16T13:27:51.643Z -c.extraMetadata.main ./arduino-ide-electron-main.js',
  command: 'electron-builder --publish never -c.electronVersion 27.0.3 -c.extraMetadata.version 2.3.4 -c.extraMetadata.name arduino-ide -c.win.artifactName arduino-ide_2.3.4_Windows_64bit.${ext} -c.extraMetadata.theia.frontend.config.appVersion 2.3.4 -c.extraMetadata.theia.frontend.config.cliVersion 1.0.4 -c.extraMetadata.theia.frontend.config.buildDate 2024-11-16T13:27:51.643Z -c.extraMetadata.main ./arduino-ide-electron-main.js',
  escapedCommand: 'electron-builder --publish never -c.electronVersion 27.0.3 -c.extraMetadata.version 2.3.4 -c.extraMetadata.name arduino-ide -c.win.artifactName "arduino-ide_2.3.4_Windows_64bit.${ext}" -c.extraMetadata.theia.frontend.config.appVersion 2.3.4 -c.extraMetadata.theia.frontend.config.cliVersion 1.0.4 -c.extraMetadata.theia.frontend.config.buildDate "2024-11-16T13:27:51.643Z" -c.extraMetadata.main "./arduino-ide-electron-main.js"',
  exitCode: 1,
  signal: undefined,
  signalDescription: undefined,
  stdout: undefined,
  stderr: undefined,
  cwd: 'D:\\a\\arduino-ide\\arduino-ide\\electron-app',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}

Node.js v18.17.1
error Command failed with exit code 1.

Change description

Remove the inferior and redundant signing determination code and use the previously established system for all targets.

This makes the workflow easier to understand and maintain.

Reviewer checklist