Skip to content

Update idf_tools.py (IDFGH-17849)#18750

Open
max-allan wants to merge 1 commit into
espressif:masterfrom
max-allan:issue-14901
Open

Update idf_tools.py (IDFGH-17849)#18750
max-allan wants to merge 1 commit into
espressif:masterfrom
max-allan:issue-14901

Conversation

@max-allan

@max-allan max-allan commented Jun 22, 2026

Copy link
Copy Markdown

Description

See issue #14901

The idf_tools.py was using http.client in a way that would fail silently with a short read from the network

Related

Fixes #14901

Testing

Ran the install.sh script where it was previously failing and it didn't fail.


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • [na] Documentation is updated as needed.
  • [na] Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

Note

Medium Risk
Changes the core download path used by install.sh and tool installs; HTTP Range resume depends on server support, though failures surface as explicit errors rather than silent truncation.

Overview
Fixes incomplete ESP-IDF tool downloads when the connection drops before the full Content-Length is read—CPython can return an empty read instead of raising, which previously left corrupt partial files.

urlretrieve_ctx now loops up to 8 attempts: after a short read it warns, backs off (capped exponential sleep), and reopens the URL with an HTTP Range: bytes={read}- request, appending to the same output file. Resume requires a 206 response; otherwise it raises ContentTooShortError. File handle lifecycle is wrapped in try/finally so the destination file is always closed.

Adds urllib.request.Request for Range requests and expands the docstring to describe the behavior.

Reviewed by Cursor Bugbot for commit abf8a92. Bugbot is set up for automated code reviews on this repo. Configure here.

@CLAassistant

CLAassistant commented Jun 22, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot changed the title Update idf_tools.py Update idf_tools.py (IDFGH-17849) Jun 22, 2026
@espressif-bot espressif-bot added the Status: Opened Issue is new label Jun 22, 2026
@dobairoland

Copy link
Copy Markdown
Collaborator

@max-allan Thanks for the contribution. The linked issue hasn't been addressed because in the meantime idf_tools.py have been deprecated and the default installation method since v6.0 is EIM (https://docs.espressif.com/projects/esp-idf/en/release-v6.0/esp32/get-started/macos-setup.html). EIM can also be used for older v5.x releases if the issue occurs with idf_tools.py.

Honestly, I don't think it would be worth the effort (yours or ours) to make this merge-ready and backport (handle minimum required Python for each release branch - as you can see the pre-commit checks are not passing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Opened Issue is new

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Install Error on MacBook (IDFGH-14087)

4 participants