Skip to content

Nix - support - rebase https://github.com/esp-rs/esp-idf-sys/pull/353 #356

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

Merged
ivmarkov merged 2 commits into from
Dec 22, 2024
+58 −44
Merged

Nix - support - rebase https://github.com/esp-rs/esp-idf-sys/pull/353 #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3b534e
build cargo_driver: update embuild to support non-git `IDF_PATH`
denbeigh2000 Nov 26, 2024
19afe7e
Leave a TODO that we need to cleanup the code that deals with an acti…
ivmarkov Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions build/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,7 @@ impl InstallDir {
/// Get the install directory from the [`ESP_IDF_TOOLS_INSTALL_DIR_VAR`] env variable.
///
/// If this env variable is unset or empty uses `default_install_dir` instead.
/// On success returns `(install_dir as InstallDir, is_default as bool)`.
pub fn try_from(location: Option<&str>) -> Result<InstallDir> {
pub fn try_from(location: Option<&str>) -> Result<Self> {
let (location, path) = match &location {
None => (crate::config::DEFAULT_TOOLS_INSTALL_DIR, None),
Some(val) => {
Expand Down
99 changes: 57 additions & 42 deletions build/native/cargo_driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use config::{ESP_IDF_REPOSITORY_VAR, ESP_IDF_VERSION_VAR};
use embuild::cargo::IntoWarning;
use embuild::cmake::file_api::codemodel::Language;
use embuild::cmake::file_api::ObjKind;
use embuild::espidf::{EspIdfOrigin, EspIdfRemote, FromEnvError, DEFAULT_ESP_IDF_REPOSITORY};
use embuild::espidf::{
EspIdfOrigin, EspIdfRemote, FromEnvError, NotActivatedError, SourceTree,
DEFAULT_ESP_IDF_REPOSITORY,
};
use embuild::fs::copy_file_if_different;
use embuild::utils::{OsStrExt, PathExt};
use embuild::{bindgen, build, cargo, cmake, espidf, git, kconfig, path_buf};
Expand Down Expand Up @@ -49,7 +52,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
if let Ok(chip) = Chip::from_str(mcu) {
if !supported_chips.iter().any(|sc| *sc == chip) {
bail!(
"Specified MCU '{chip}' is not amongst the MCUs ([{}]) supported by the build target ('{target}')",
"Specified MCU '{chip}' is not amongst the MCUs ([{}]) supported by the build target ('{target}')",
supported_chips.iter().map(|chip| format!("{chip}")).collect::<Vec<_>>().join(", ")
);
}
Expand Down Expand Up @@ -82,13 +85,13 @@ pub fn build() -> Result<EspIdfBuildOutput> {
let cmake_generator = config.native.esp_idf_cmake_generator();

// A closure to specify which tools `idf-tools.py` should install.
let make_tools = move |repo: &git::Repository,
let make_tools = move |tree: &SourceTree,
version: &Result<espidf::EspIdfVersion>|
-> Result<Vec<espidf::Tools>> {
eprintln!(
"Using esp-idf {} at '{}'",
espidf::EspIdfVersion::format(version),
repo.worktree().display()
tree.path().display(),
);

let mut tools = vec![];
Expand Down Expand Up @@ -127,38 +130,37 @@ pub fn build() -> Result<EspIdfBuildOutput> {
// we're using msys/cygwin.
// But this variable is also present when using git-bash.
env::remove_var("MSYSTEM");

// Install the esp-idf and its tools.
let (idf, tools_install_dir) = {
// Get the install dir location from the build config, or use
// [`crate::config::DEFAULT_TOOLS_INSTALL_DIR`] if unset.
let (install_dir, is_default_install_dir) = config.esp_idf_tools_install_dir()?;
let (idf_tools_install_dir, is_default_install_dir) = config.esp_idf_tools_install_dir()?;
// EspIdf must come from the environment if `esp_idf_tools_install_dir` == `fromenv`.
let require_from_env = install_dir.is_from_env();
let require_from_env = idf_tools_install_dir.is_from_env();
let maybe_from_env = require_from_env || is_default_install_dir;

// Closure to install the esp-idf using `embuild::espidf::Installer`.
let install = |esp_idf_origin: EspIdfOrigin| -> Result<(espidf::EspIdf, InstallDir)> {
match &esp_idf_origin {
EspIdfOrigin::Custom(repo) => {
eprintln!(
"Using custom user-supplied esp-idf repository at '{}' (detected from env variable `{}`)",
repo.worktree().display(),
espidf::IDF_PATH_VAR
);
"Using custom user-supplied esp-idf repository at '{}' (detected from env variable `{}`)",
repo.path().display(),
espidf::IDF_PATH_VAR
);
if let Some(custom_url) = &config.native.esp_idf_repository {
cargo::print_warning(format_args!(
"Ignoring configuration setting `{ESP_IDF_REPOSITORY_VAR}=\"{custom_url}\"`: \
custom esp-idf repository detected via ${}",
espidf::IDF_PATH_VAR
));
"Ignoring configuration setting `{ESP_IDF_REPOSITORY_VAR}=\"{custom_url}\"`: \
custom esp-idf repository detected via ${}",
espidf::IDF_PATH_VAR
));
}
if let Some(custom_version) = &config.native.esp_idf_version {
cargo::print_warning(format_args!(
"Ignoring configuration setting `{ESP_IDF_VERSION_VAR}` ({custom_version}): \
custom esp-idf repository detected via ${}",
espidf::IDF_PATH_VAR
));
"Ignoring configuration setting `{ESP_IDF_VERSION_VAR}` ({custom_version}): \
custom esp-idf repository detected via ${}",
espidf::IDF_PATH_VAR
));
}
}
EspIdfOrigin::Managed(remote) => {
Expand All @@ -167,13 +169,21 @@ pub fn build() -> Result<EspIdfBuildOutput> {
};

let idf = espidf::Installer::new(esp_idf_origin)
.install_dir(install_dir.path().map(Into::into))
.install_dir(idf_tools_install_dir.path().map(Into::into))
.with_tools(make_tools)
.install()
.context("Could not install esp-idf")?;
Ok((idf, install_dir.clone()))
Ok((idf, idf_tools_install_dir.clone()))
};

// TODO: This is a bit of a mess.
// We should probably refactor this and the lines below to make it more readable.
//
// For one, it is still unclear to me when this path should be used and when not.
// It seems an option is to also completely retire specifying the IDF PATH in the cargo-metadata config,
// see: https://github.com/esp-rs/esp-idf-sys/pull/353#issuecomment-2543179482
let idf_path = config.native.idf_path.as_deref();

// 1. Try to use the activated esp-idf environment if `esp_idf_tools_install_dir`
// is `fromenv` or unset.
// 2. Use a custom esp-idf repository specified by `$IDF_PATH`/`idf_path` if
Expand All @@ -185,29 +195,29 @@ pub fn build() -> Result<EspIdfBuildOutput> {
eprintln!(
"Using activated esp-idf {} environment at '{}'",
espidf::EspIdfVersion::format(&idf.version),
idf.repository.worktree().display()
idf.esp_idf_dir.path().display()
);

(idf, InstallDir::FromEnv)
},
(Ok(idf), false) => {
cargo::print_warning(format_args!(
"Ignoring activated esp-idf environment: {ESP_IDF_TOOLS_INSTALL_DIR_VAR} != {}", InstallDir::FromEnv
));
install(EspIdfOrigin::Custom(idf.repository))?
cargo::print_warning(format_args!(
"Ignoring activated esp-idf environment: {ESP_IDF_TOOLS_INSTALL_DIR_VAR} != {}", InstallDir::FromEnv
));
install(EspIdfOrigin::Custom(idf.esp_idf_dir))?
},
(Err(FromEnvError::NotActivated { source: err, .. }), true) |
(Err(FromEnvError::NotActivated(NotActivatedError { source: err, .. })), true) |
(Err(FromEnvError::NoRepo(err)), true) if require_from_env => {
return Err(err.context(
format!("activated esp-idf environment not found but required by {ESP_IDF_TOOLS_INSTALL_DIR_VAR} == {install_dir}")
format!("activated esp-idf environment not found but required by {ESP_IDF_TOOLS_INSTALL_DIR_VAR} == {idf_tools_install_dir}")
))
}
(Err(FromEnvError::NotActivated { esp_idf_repo, .. }), _) => {
install(EspIdfOrigin::Custom(esp_idf_repo))?
(Err(FromEnvError::NotActivated(NotActivatedError { esp_idf_dir, .. })), _) => {
install(EspIdfOrigin::Custom(esp_idf_dir))?
},
(Err(FromEnvError::NoRepo(_)), _) => {
let origin = match &config.native.idf_path {
Some(idf_path) => EspIdfOrigin::Custom(git::Repository::open(idf_path)?),
let origin = match idf_path {
Some(idf_path) => EspIdfOrigin::Custom(SourceTree::Plain(idf_path.to_path_buf())),
None => EspIdfOrigin::Managed(EspIdfRemote {
git_ref: config.native.esp_idf_version(),
repo_url: config.native.esp_idf_repository.clone()
Expand Down Expand Up @@ -284,9 +294,13 @@ pub fn build() -> Result<EspIdfBuildOutput> {
let patch_set = match idf.version.as_ref().map(|v| (v.major, v.minor, v.patch)) {
// master branch
_ if {
let default_branch = idf.repository.get_default_branch()?;
let curr_branch = idf.repository.get_branch_name()?;
default_branch == curr_branch && default_branch.is_some()
if let SourceTree::Git(repository) = &idf.esp_idf_dir {
let default_branch = repository.get_default_branch()?;
let curr_branch = repository.get_branch_name()?;
default_branch == curr_branch && default_branch.is_some()
} else {
false
}
} =>
{
cargo::print_warning(
Expand All @@ -313,10 +327,11 @@ pub fn build() -> Result<EspIdfBuildOutput> {
}
};

// Apply patches, only if the patches were not previously applied and if the esp-idf repo is managed.
// Apply patches, only if the patches were not previously applied and if the esp-idf dir is a managed GIT repo.
if idf.is_managed_espidf && !patch_set.is_empty() {
idf.repository
.apply_once(patch_set.iter().map(|p| manifest_dir.join(p)))?;
if let SourceTree::Git(repository) = &idf.esp_idf_dir {
repository.apply_once(patch_set.iter().map(|p| manifest_dir.join(p)))?;
}
}

// "PATH" is anyway passed to the CMake generator, but if we don't set it here, we get the following warnings from CMake:
Expand Down Expand Up @@ -427,7 +442,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
)?;

let cmake_toolchain_file = path_buf![
&idf.repository.worktree(),
&idf.esp_idf_dir.path(),
"tools",
"cmake",
chip.cmake_toolchain_file()
Expand All @@ -443,7 +458,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
cmake::cmake(),
"-P",
extractor_script.as_ref().as_os_str();
env=("IDF_PATH", &idf.repository.worktree().as_os_str()))
env=("IDF_PATH", idf.esp_idf_dir.path()))
.stdout()?;

let mut vars = cmake::process_script_variables_extractor_output(output)?;
Expand Down Expand Up @@ -484,7 +499,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
.cxxflag(cxx_flags)
.env("IDF_COMPONENT_MANAGER", idf_comp_manager)
.env("EXTRA_COMPONENT_DIRS", extra_component_dirs)
.env("IDF_PATH", idf.repository.worktree())
.env("IDF_PATH", idf.esp_idf_dir.path())
.env("PATH", &idf.exported_path)
.env("SDKCONFIG_DEFAULTS", defaults_files)
.env("IDF_TARGET", &chip_name)
Expand Down Expand Up @@ -540,7 +555,7 @@ pub fn build() -> Result<EspIdfBuildOutput> {
.context("Could not determine the compiler from cmake")?;

let build_info = espidf::EspIdfBuildInfo {
esp_idf_dir: idf.repository.worktree().to_owned(),
esp_idf_dir: idf.esp_idf_dir.path().to_owned(),
exported_path_var: idf.exported_path.try_to_str()?.to_owned(),
venv_python: idf.venv_python,
build_dir: cmake_build_dir.clone(),
Expand Down