Skip to content

Commit

Permalink
Nix - support - rebase #353 (#356)
Browse files Browse the repository at this point in the history 
* build cargo_driver: update embuild to support non-git `IDF_PATH`

* Leave a TODO that we need to cleanup the code that deals with an activated ESP IDF environment

---------

Co-authored-by: Denbeigh Stevens <[email protected]>
  • Loading branch information
@ivmarkov @denbeigh2000
ivmarkov and denbeigh2000 authored Dec 22, 2024
1 parent 45c2881 commit 55bd1da
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 44 deletions.
3 changes: 1 addition & 2 deletions build/common.rs
View file
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
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

0 comments on commit 55bd1da

Please sign in to comment.