-
-
Notifications
You must be signed in to change notification settings - Fork 422
regression: fixed ARDUINO_USER_AGENT pollution / resolve compile-sketch GH action failure on Teensy
#2669
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
Conversation
ace211b to
8d07cda
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@ ## master #2669 +/- ## ========================================== + Coverage 70.22% 70.26% +0.03% ========================================== Files 222 222 Lines 21346 21343 -3 ========================================== + Hits 14991 14997 +6 + Misses 5176 5170 -6 + Partials 1179 1176 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
A few questions:
- Is
1.0.0-snapshotclear enough? Maybe1.0.0-git-snapshot,1.0.0-snapshot.git, and1.0.0-snapshot+gitcan be clearer? - At this point should we change the nightly as well? Like
1.0.0-nightly?
To be honest it's confusing to see this 1.0.0 prefix for nightly and snapshot builds. I'll probably live as it was.
Shouldn't the 968689f alone fix the #2658?
To be honest it's confusing to see this
1.0.0prefix for nightly and snapshot builds. I'll probably live as it was.
Shouldn't the 968689f alone fix the #2658?
I tried to run into a CI by using the example given by @per1234 here #2658 (comment), since it builds the CLI from a git ref, the version reported in the user agent is git-snapshot and it will trigger the bug in #2658. -> https://github.com/cmaglie/cli-action-test/actions/runs/9980439654
BTW It's also true that the compile-sketch action will run only tagged versions, so it's impossible to run a build with an invalid semver like git-snapshot or nightly. This also means that running a build of the CLI from a task build or a nightly will always trigger the execution of the Teensy tool.
Said that, given the pro and cons, I'm positive to leave git-snapshot as it was.
@cmaglie ok, I'm more keen to leave the git-snapshot as it is. Because, as you said, the compile-sketch action runs only tagged versions, changing that for the purposes of making Teensy tool work when manually building, feels like something that should be addressed from their side.
8d07cda to
aae8c8a
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.
I like more git-snapshot too
Uh oh!
There was an error while loading. Please reload this page.
Please check if the PR fulfills these requirements
See how to contribute
before creating one)
our contributing guidelines
UPGRADING.mdhas been updated with a migration guide (for breaking changes)configuration.schema.jsonupdated if new parameters are added.What kind of change does this PR introduce?
Arduino CLI sets the environment variable
ARDUINO_USER_AGENTwith the currently running version and passes this variable down to the tools (compilers, uploaders, etc.). This PR fixes a regression in the user agent string.What is the current behavior?
In the official release build
ARDUINO_USER_AGENTis set asarduino-cli/1.0.2 gRPCClientUnknown/0.0.0.A build from source via
task buildsetsARDUINO_USER_AGENTasarduino-cli/git-snapshot gRPCClientUnknown/0.0.0.What is the new behavior?
The new official release build will set
ARDUINO_USER_AGENTasarduino-cli/1.0.3.A build from source via
task buildnow setsARDUINO_USER_AGENTasarduino-cli/git-snapshot.(削除)git-snapshothas been replaced by1.0.0-snapshotto comply with semver specification. This change also reflects that the snapshot has been built from the 1.x.x series branch. (削除ここまで)Does this PR introduce a breaking change, and is titled accordingly?
No
Other information
This PR also fixes #2658. The Teensy tools check for the
ARDUINO_USER_AGENTvariable, in particular, it seems to be very picky about thearduino-cli/x.x.xpart (it must be a strict version >=0.35.0) and there must be no extra parts.This is a regression starting from 1.0.0.