Restored monitor service creation state before a36524e:
Pass core client provider into new instances as a field.
Closes#1161
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
The "Arduino IDE" workflow performs the following operations when triggered on push and pull request events:
- Build application
- Lint code
- Run tests
- Produce tester packages
All of these operations are specific to the TypeScript/JavaScript code base and its infrastructure.
Previously, the workflow ran whenever any file in the repository was changed. This includes files that have no
relevance, meaning the operations performed by the workflow were pointless. In addition to general inefficiency, these
lengthy and sometimes spuriously failing unnecessary workflow runs might cause delay or confusion to both the
contributors and maintainers for what would otherwise be a simple process.
GitHub Actions provides the ability to configure path filters for the workflow triggers. The workflow will only run on
events that change files satisfying these path filters. This is "AND"ed with the `branches` filters, meaning the existing
restrictions on which branches produce a run remain unchanged. The `tags` filter is independent from the `paths` and
`branches` filters, meaning the added path filters don't make any change to which tag push events will trigger the
workflow.
GitHub Actions workflows may require access to privileged information in order to perform certain operations. GitHub
provides the capability for doing this via "repository secrets".
For security reasons, repository secrets are only accessible to a GitHub Actions workflow run when it is triggered by an
event from within the repository containing the secret. This means that a workflow which requires such secrets would
fail when run in a fork (unless the fork owner was able to set up their own secrets with suitable values).
In order to make the relevant components of the CI system friendly for use in forks by contributors validating their
work in preparation for submitting a PR, when the operations that require access to a secret are supplemental, those
operations should be configured to only run from branches of the parent repository.
Due to its unfortunate monolithic design, in addition to operations useful to contributors, the "Arduino IDE" workflow
contains several such supplemental operations:
- Code signing
- Publishing release artifacts to Arduino's server
Some attempt was previously made to configure the workflow to skip these operations when run in forks, but that
configuration was not done correctly. This made the workflow only usable by contributors with a deep enough
understanding of GitHub Actions to be able to make the necessary modifications provisionally every time they needed to
use the workflow.
The average contributor would not be capable or willing to do this, which might result in PRs being
submitted in a less validated state, increasing the burden on maintainers.
The specific misconfigurations:
**`build` job was conditional on the workflow running from `arduino/arduino-ide`**
The job itself can run just fine in a fork, so there is no reason to impose this restriction.
Since the time this conditional was added, some changes have been made to the GitHub Actions system which makes this
sort of configuration unnecessary:
- GitHub Actions is globally disabled in forks by default
- Workflows which contain a `schedule` trigger (as is the case with this one) are individually disabled by default,
requiring the repository owner to enable it specifically even after enabling GitHub Actions in general.
This means this workflow will never run unexpectedly in a fork. The fork owner will always have intentionally enabled it.
So this conditional can be removed completely.
**Code signing was conditional on PR being submitted from a branch of the base repo**
This would cause a spurious failure of the signing operation on PRs made within the contributor's fork when the signing
secrets were not defined.
The more appropriate condition of whether the signing secrets are defined or not is now used. The environment variable
name has been updated accordingly.
**`release` job was conditional on running from `arduino/arduino-ide`**
The GitHub release creation step of this job can run in any repository. It is only the step that uploads to Arduino's
AWS server which would only make sense to run from `arduino/arduino-ide`.
So the conditional is moved to the AWS upload step, allowing contributors to test the workflow's release operation in
their forks to validate related proposals.
Previously, there was some code duplication of the complex code signing certificate handling commands, which made the
related code more difficult to understand, maintain, and develop.
The cause of this duplication is that there is a separate certificate for each operating system, each of which is stored
in separate repository secrets, as well as a different certificate file extension for each OS. Since the secret names
and file extensions are associated with the operating system, it is most logical to define them via attributes alongside
the operating system definition in the job matrix configuration already used to generate the parallel job runs for
native build on each OS.
That done, the certificate handling commands are universal and the system can easily expand to additional host targets
(e.g., Apple M1) as time goes on.
The Arduino IDE attempts to provide some additional guidance to users based on matches against compilation error
messages.
This practice was established during a time when some significant breaking changes were made to the common APIs in order
to ease the transition.
Since that time, the practice has mostly been discontinued. The interpretations are only valid for very old code that is
unlikely to be used by the target users now. So their benefit is negligible. The patterns used are inexact, meaning that
the interpretations may be printed inappropriately, which is more and more likely as the cases where the matches would
be valid become increasingly rare. When the maintenance burden is taken into consideration, it is clear that the harm is
far more than any benefits from these. So they are removed.
Notes for specific interpretations:
> Please import the SPI library from the Sketch > Import Library menu.
> As of Arduino 0019, the Ethernet library depends on the SPI library.
> You appear to be using it or another library that depends on the SPI library.
The target error was more common prior to Arduino IDE 1.6.6 (released ~6.5 years ago), when it was necessary for the
sketch to contain `#include` directives for transitive in addition to direct library dependencies (SPI is a common
transitive dependency).
Due to the nature of the SPI library, it is not often used directly, and when it is used directly it is done by more
advanced users who are unlikely to forget the `#include` directive and would have no need for this interpretation even
if they did.
It is far more likely for the user to forget an `#include` for a popular library, yet Arduino rightly does not attempt
to maintain interpretations for those.
The "Sketch > Import Library" menu path was renamed to "Sketch > Include Library" ~7 years ago.
Arduino IDE 0019 was released ~12 years ago. We can safely assume the migration to the new Ethernet API is complete.
> The 'BYTE' keyword is no longer supported.
> As of Arduino 1.0, the 'BYTE' keyword is no longer supported.
> Please use Serial.write() instead.
Arduino IDE 1.0 was released ~10.5 years ago. We can safely assume the migration to the new Serial API is complete.
This compilation error pattern is now far more likely to occur due to incorrect usage of a completely unrelated
occurrence of the common `BYTE` name in the user's code.
> The Server class has been renamed EthernetServer.
> As of Arduino 1.0, the Server class in the Ethernet library has been renamed to EthernetServer.
> The Client class has been renamed EthernetClient.
> As of Arduino 1.0, the Client class in the Ethernet library has been renamed to EthernetClient.
> The Udp class has been renamed EthernetUdp.
> As of Arduino 1.0, the Udp class in the Ethernet library has been renamed to EthernetUdp.
Arduino IDE 1.0 was released ~10.5 years ago. We can safely assume the migration to the new Ethernet API is complete.
The compilation error patterns are in no way specific to the Ethernet library so is prone to false positives.
> Wire.send() has been renamed Wire.write().
> As of Arduino 1.0, the Wire.send() function was renamed to Wire.write() for consistency with other libraries.
> Wire.receive() has been renamed Wire.read().
> As of Arduino 1.0, the Wire.receive() function was renamed to Wire.read() for consistency with other libraries.
Arduino IDE 1.0 was released ~10.5 years ago. We can safely assume the migration to the new Wire API is complete.
Due to the nature of the Wire library, it is not often used directly, and when it is used directly it is done by more
advanced users who have less need for an interpretation of the compiler error.
> 'Mouse' not found. Does your sketch include the line '#include <Mouse.h>'?
> 'Keyboard' not found. Does your sketch include the line '#include <Keyboard.h>'?
I left these in because they are the most "recent" (added due to a breaking change made 7 years ago).
However, I also feel that these are harmful and should either be removed or changed. The problem is that there is a
false match when the user attempts to compile the Keyboard or Mouse libraries for a board which does not have native USB
support (e.g., Uno, Mega), even when their sketch does contain the `#include` directives that are recommended by the
interpretation. That cause of the compilation error matching the pattern is more common than the case where the user is
compiling old code or forgot the `#include` directive, for which the interpretation is valid.
Contributors may submit pull requests against development branches in the repository for either of the following valid
reasons:
- Propose changes to a previous proposal, either while it is still in development, or else in the case where the changes
are more complex/extensive than can be efficiently proposed via the PR review framework.
- The proposal is dependent on work from an unmerged PR.
Previously, the "Arduino IDE" GitHub Actions workflow was unnecessarily configured to only run for PRs based on the
`main` branch. This meant that validation and tester builds were not provided for the PRs based on other branches.
* 1032 failing upload flag for monitor mgr
* move upload failure fix logic to frontend
* misc corrections
* avoid starting monitor when upload is in progress
* avoid starting monitor when upload is in progress
* prevent monitor side effects on upload (WIP)
* send upload req after notifying mgr
* dispose instead of pause on upld (code not final)
* Revert "dispose instead of pause on upld (code not final)"
This reverts commit 2d5dff2a2d85754467470b5973b7ecac8017aa58.
* force wait before upload (test)
* always start queued services after uplaod finishes
* test cli with monitor close delay
* clean up unnecessary await(s)
* remove unused dependency
* revert CLI to 0.23
* use master cli for testing, await in upload finish
* remove upload port from pending monitor requests
* fix startQueuedServices
* refinements queued monitors
* clean up monitor mgr state
* fix typo from prev cleanup
* avoid dupl queued monitor services
* variable name changes
* reference latest cli commit in package.json
Co-authored-by: Alberto Iannaccone <a.iannaccone@arduino.cc>
