Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

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

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
# for free to join this conversation on GitHub. Already have an account? # 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