Skip to content
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

#41, #36 #42

Merged
merged 7 commits into from
Apr 26, 2022
Merged

#41, #36 #42

merged 7 commits into from
Apr 26, 2022

Conversation

ivmarkov
Copy link
Collaborator

@ivmarkov ivmarkov commented Dec 9, 2021

As the subject says, this is fixing #41 and #36.

The changes should be relatively straightforward actually, with the non-trivial changes actually being part of the espidf module in embuild.

See this PR for the embuild-related changes:
esp-rs/embuild#29

build/common.rs Outdated Show resolved Hide resolved
build/common.rs Outdated Show resolved Hide resolved
@SergioGasquez
Copy link
Member

Hello! Just addressed @N3xed comments. Let me know your opinion @ivmarkov :)

@N3xed
Copy link
Collaborator

N3xed commented Apr 11, 2022

Okay, I think, at least the for the native installer, this is close to finished.
I still need to look at the pio installer.
Still needs some more testing though, and code reviews are appreciated.

I've refactored @ivmarkov 's code pretty heavily and also incorporated the changes I've made in embuild (though the author of the first commit is still, ivmarkov; hope you don't mind).

The biggest change is that EspIdf now uses a closure for the tools to be installed, this allows to decide dynamically which tools should be installed. This change together with the added EspIdfVersion that is parsed from the esp-idf/tools/cmake/version.cmake file now allows us easily to much more easily support multiple esp-idf versions.

I've also added a big match statement for selecting the esp-idf detected from the environment, the repo specified with IDF_PATH or everything managed by the installer.

Basically:

  • If activated esp-idf available and $ESP_IDF_TOOLS_INSTALL_DIR == "fromenv" or unset, use it
  • If $IDF_PATH is valid, use it as user provided esp-idf repository (but installer manages tools)
  • Otherwise the installer manages everything and respects ESP_IDF_REPOSITORY and ESP_IDF_VERSION.

There are also some errors and warnings for edge cases.

On windows, activated esp-idf detection currently doesn't work because the which crate does not search for the correct filename (harryfei/which-rs#56).

@N3xed N3xed marked this pull request as ready for review April 26, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support using ESP-IDF from the IDF_PATH variable Support for starting build in activated ESP-IDF environment
3 participants