-
-
Notifications
You must be signed in to change notification settings - Fork 398
[skip-changelog] Use a matrix to create parallel builds for each OS #1883
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
[skip-changelog] Use a matrix to create parallel builds for each OS #1883
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! The improvement in time is huge! 🚅
Unfortunately this is not sufficient: we need to find another approach for the checksum generation, because the generated one is not complete.
The task dist:all
was appending the checksums in the same file and then uploading it (since the task were executed sequentially it was possible). Unfortunately this is no longer working.
I suggest removing the checksum calculation from the tasks, from the notarization step. And perform it only during the release creation job
Also it does not make sense to perform the changelog generation in all the jobs.. could be an idea to do it only for one of the runs |
0b60a3e
to
44d57ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some suggestions 👾
742c3bc
to
7335d8e
Compare
Using a matrix to run each build task greatly improves performances, since they can all start concurrently. The finishing time of the job will be equal to the one of the longer task to build, instead of being the sum of each individual task's finishing time.
Checksums of the output files where previously calculated during the initial creation of the artifacts, during the notarization process and, finally, at the release creation's step. The whole process has been simplified and checksums are now computed only during the creation of the release.
The changelog is the same for each OS. It does not make sense to generate it more than once.
7335d8e
to
630506e
Compare
bec617e
to
370016b
Compare
335245c
to
415caa0
Compare
Previously, the different builds were firstly uploaded using a single artifact, which was then downloaded to create different ones and eventually deleted. Now, since builds are created concurrently, the same matrix can be used to directly upload an artifact for each build. It's necessary to use a second job to calculate the checksum related to each build and save them all in a single .txt file.
415caa0
to
86b1fb5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great Job Matteo 🥇
* add step to generate a windows msi installer of the Arduino CLI * remove warning: `warning : Solution properties are only available during IDE builds or when building the solution file from the command line. To turn off this warning set <DefineSolutionProperties>false</DefineSolutionProperties> in your .wixproj` * fix error caused by wix peculiar version constraints: `error CNDL0108: The Product/@Version attribute's value, '0.0.0-test', is not a valid version. Legal version values should look like 'x.x.x.x' where x is an integer from 0 to 65534.` * fix `error LGHT0103: The system cannot find the file 'arduino-cli.exe'` * final adjustments in wix config * exclude prettier from checking the installer config files, prettier does not support xml out of the box * add download links * temporarily disable s3 push and homebrew update for testing * Upload nightly artifacts for testing * add windows check certs * bump to latest windows-sdk and document better specify timestamp algorithm. Apparently this is required in the latest version of the sdk * remove useless job id not used anymore (followup of #1883) * fixed comments * It's useless to use the output from setupmsbuild * use YAML-style syntax * try to uniform the usage of version/tag across the workflow * Revert "Upload nightly artifacts for testing" && "temporarily disable s3 push and homebrew update for testing" This reverts commit 3123dfd. This reverts commit 24fa25c. * Apply suggestions from code review Co-authored-by: per1234 <[email protected]> Co-authored-by: per1234 <[email protected]>
Please check if the PR fulfills these requirements
before creating one)
our contributing guidelines
UPGRADING.md
has been updated with a migration guide (for breaking changes)What kind of change does this PR introduce?
Infrastructure Enhancement
What is the current behavior?
Release-go-task, publish-go-nightly-task and publish-go-tester-task are currently using
task dist:all
to build and create the release artifacts. This is done by calling the tasks related to each OS consecutively. As a result, the finishing time oftask dist:all
is the sum of the finishing times of each individualdist
. Furthermore, the checksums related to the files built are calculated more than once, unnecessarily.What is the new behavior?
publish-tester-build
now uploads a different artifact for each build during the first job, instead of creating a combined one that needs to be split later.Does this PR introduce a breaking change, and is titled accordingly?
No
Other information:
Release run: https://github.com/MatteoPologruto/arduino-cli/actions/runs/3128175485
Test release: https://github.com/MatteoPologruto/arduino-cli/releases/tag/0.0.3
Nightly build run: https://github.com/MatteoPologruto/arduino-cli/actions/runs/3128099818
Tester build run: https://github.com/MatteoPologruto/arduino-cli/actions/runs/3137219983
Publish-go-tester-task was correctly skipped after
run-determination
when I pushed the tag0.0.2
onto my fork (here). Out of curiosity, I triggered the workflow run manually on the same commit and the checksums' calculation was not correct (here). Checksums calculated previously and afterwards seem to be correct, even though the workflow has not been modified. I think it might be worth mentioning.See how to contribute