Skip to content

dist/tools/esptool: fix the all target #21129

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 13, 2025
Merged

dist/tools/esptool: fix the all target #21129

merged 1 commit into from
Jan 13, 2025

Conversation

gschorcht
Copy link
Contributor

@gschorcht gschorcht commented Jan 12, 2025 •
edited
Loading

Contribution description

This PR fixes the all parameter in dist/tools/esptool/intall.sh and dist/tools/esptool/export.sh.

With PR #21094, the installation of the ESP8266 toolchain was added to the tools by overriding the ESP32_GCC_RELEASE variable with the ESP8266_GCC_RELEASE variable. This leads to wrong settings when iterating over all architectures if the tools are called with parameter all.

Testing procedure

dist/tools/esptool/install.sh all
. dist/tools/esptool/export.sh all

Without the patch, the installation only works for the ESP8266 toolchain.

Issues/PRs references

Fixes PR #21094

@gschorcht gschorcht requested a review from benpicco January 12, 2025 16:43
@github-actions github-actions bot added the Area: tools Area: Supplementary tools label Jan 12, 2025
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: ESP Platform: This PR/issue effects ESP-based platforms CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs and removed Area: tools Area: Supplementary tools labels Jan 12, 2025
mguetschow
mguetschow approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@mguetschow mguetschow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks for fixing!

@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 13, 2025
@riot-ci
Copy link

riot-ci commented Jan 13, 2025

Murdock results

✔️ PASSED

c1208ad dist/tools/esptool: fix the all target

Success Failures Total Runtime
1 0 1 01m:17s

Artifacts

@mguetschow mguetschow added this pull request to the merge queue Jan 13, 2025
Merged via the queue into RIOT-OS:master with commit 58adb81 Jan 13, 2025
31 checks passed
@gschorcht
Copy link
Contributor Author

@mguetschow Thanks for reviewing and merging.

@gschorcht gschorcht deleted the dist/tools/esptools/fix_all_target branch January 13, 2025 20:52
@MrKevinWeiss MrKevinWeiss added this to the Release 2025.01 milestone Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mguetschow mguetschow mguetschow approved these changes

@benpicco benpicco Awaiting requested review from benpicco

Assignees
No one assigned
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2025.01
Development

Successfully merging this pull request may close these issues.
4 participants
@gschorcht @riot-ci @mguetschow @MrKevinWeiss