Skip to content

{Bp-14267} tools/espressif: add esptool version check to Espressif build system#14268

Merged
xiaoxiang781216 merged 1 commit into
apache:releases/12.7from
jerpelea:bp-14267
Oct 16, 2024
Merged

{Bp-14267} tools/espressif: add esptool version check to Espressif build system#14268
xiaoxiang781216 merged 1 commit into
apache:releases/12.7from
jerpelea:bp-14267

Conversation

@jerpelea

@jerpelea jerpelea commented Oct 14, 2024

Copy link
Copy Markdown
Contributor

Summary
Add a Python script to check if esptool is installed and if its version is up-to-date (see minimum version at ESPTOOL_MIN_VERSION on .mk files).

This fixes build issues as reported in #13824.

Impact
New Python script added. Stops build if esptool version is below 4.8.
Script runs for ESP32, S2, S3, C3, C6.

Testing
Tested locally and on internal CI with Python3.8+.

NOTE
backported #14176 to fix the CI

@jerpelea

jerpelea commented Oct 14, 2024

Copy link
Copy Markdown
Contributor Author

@fdcavalcanti please check 12.7 release

@fdcavalcanti

Copy link
Copy Markdown
Contributor

Not sure what is happening on the CI

@fdcavalcanti

Copy link
Copy Markdown
Contributor

Hi @jerpelea, how is this backport PR different from the one I created?

@jerpelea

Copy link
Copy Markdown
Contributor Author

@fdcavalcanti it is a backport.

@fdcavalcanti

Copy link
Copy Markdown
Contributor

@fdcavalcanti it is a backport.

My PR was already targeting the release 12.7. Am I missing something? Should I change that one to target master?

@jerpelea

Copy link
Copy Markdown
Contributor Author

after i created the backport you force-pushed the patch so I had to force-push this one manually
if you force-push again please notify me :) to update the backport
Best regards
Alin

@fdcavalcanti

Copy link
Copy Markdown
Contributor

after i created the backport you force-pushed the patch so I had to force-push this one manually if you force-push again please notify me :) to update the backport Best regards Alin

Thanks @jerpelea

@fdcavalcanti

Copy link
Copy Markdown
Contributor

CI issue is new pip rules not allowing packages installed system-wide.
Has already been fixed on master #14176

@jerpelea

Copy link
Copy Markdown
Contributor Author

@fdcavalcanti backported

@jerpelea

Copy link
Copy Markdown
Contributor Author

@fdcavalcanti it is a backport.

My PR was already targeting the release 12.7. Am I missing something? Should I change that one to target master?

@fdcavalcanti please rebase yours on master

@cederom

cederom commented Oct 14, 2024

Copy link
Copy Markdown
Contributor

Hmm this seems mix of #14267 and CI fix?

Can we push CI fix here and exptool fix here #14267 ?

@cederom cederom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah this is backport to releases/12.7 sorry :-) After #14267 is updated and merged go ahead, thank you!! :-)

@jerpelea jerpelea marked this pull request as draft October 15, 2024 15:13
@jerpelea jerpelea marked this pull request as ready for review October 16, 2024 08:25
@jerpelea

jerpelea commented Oct 16, 2024

Copy link
Copy Markdown
Contributor Author

please merge after CI passes

@github-actions github-actions Bot removed the Area: CI label Oct 16, 2024
@xiaoxiang781216 xiaoxiang781216 merged commit 71d57aa into apache:releases/12.7 Oct 16, 2024
@jerpelea jerpelea deleted the bp-14267 branch June 23, 2025 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Tooling Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants