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.

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

Jump to bottom

Change search_lib_dir's return type to Result #4043

Merged
merged 3 commits into from
Jun 22, 2024
Merged

Change search_lib_dir's return type to Result #4043

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
feba308
Change `search_lib_dir`'s return type to Result
reswqa Apr 3, 2024
f7f91c7
add changelog
reswqa Apr 3, 2024
bc864bc
add coverage and a hint to `PYO3_CROSS_LIB_DIR`
davidhewitt Jun 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
1 change: 1 addition & 0 deletions newsfragments/4043.fixed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Don't panic when `PYO3_CROSS_LIB_DIR` is set to a missing path.
24 changes: 13 additions & 11 deletions pyo3-build-config/src/impl_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,7 @@ fn ends_with(entry: &DirEntry, pat: &str) -> bool {
/// Returns `None` if the library directory is not available, and a runtime error
/// when no or multiple sysconfigdata files are found.
fn find_sysconfigdata(cross: &CrossCompileConfig) -> Result<Option<PathBuf>> {
let mut sysconfig_paths = find_all_sysconfigdata(cross);
let mut sysconfig_paths = find_all_sysconfigdata(cross)?;
if sysconfig_paths.is_empty() {
if let Some(lib_dir) = cross.lib_dir.as_ref() {
bail!("Could not find _sysconfigdata*.py in {}", lib_dir.display());
Expand Down Expand Up @@ -1274,11 +1274,11 @@ fn find_sysconfigdata(cross: &CrossCompileConfig) -> Result<Option<PathBuf>> {
///
/// Returns an empty vector when the target Python library directory
/// is not set via `PYO3_CROSS_LIB_DIR`.
pub fn find_all_sysconfigdata(cross: &CrossCompileConfig) -> Vec<PathBuf> {
pub fn find_all_sysconfigdata(cross: &CrossCompileConfig) -> Result<Vec<PathBuf>> {
let sysconfig_paths = if let Some(lib_dir) = cross.lib_dir.as_ref() {
search_lib_dir(lib_dir, cross)
search_lib_dir(lib_dir, cross)?
} else {
return Vec::new();
return Ok(Vec::new());
};

let sysconfig_name = env_var("_PYTHON_SYSCONFIGDATA_NAME");
Expand All @@ -1296,7 +1296,7 @@ pub fn find_all_sysconfigdata(cross: &CrossCompileConfig) -> Vec<PathBuf> {
sysconfig_paths.sort();
sysconfig_paths.dedup();

sysconfig_paths
Ok(sysconfig_paths)
}

fn is_pypy_lib_dir(path: &str, v: &Option<PythonVersion>) -> bool {
Expand Down Expand Up @@ -1327,17 +1327,19 @@ fn is_cpython_lib_dir(path: &str, v: &Option<PythonVersion>) -> bool {
}

/// recursive search for _sysconfigdata, returns all possibilities of sysconfigdata paths
fn search_lib_dir(path: impl AsRef<Path>, cross: &CrossCompileConfig) -> Vec<PathBuf> {
fn search_lib_dir(path: impl AsRef<Path>, cross: &CrossCompileConfig) -> Result<Vec<PathBuf>> {
let mut sysconfig_paths = vec![];
for f in fs::read_dir(path).expect("Path does not exist") {
for f in fs::read_dir(path)
.context("failed to search the lib dir, please check your path and permissions.")?
Copy link
Member

Choose a reason for hiding this comment

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

I think we should name the relevant environment variables here or possibly add more targeted calls to context at the call sites of search_lib_dir so the user knows what to check exactly.

Copy link
Member

Choose a reason for hiding this comment

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

Added a reference to PYO3_CROSS_LIB_DIR

{
sysconfig_paths.extend(match &f {
// Python 3.7+ sysconfigdata with platform specifics
Ok(f) if starts_with(f, "_sysconfigdata_") && ends_with(f, "py") => vec![f.path()],
Ok(f) if f.metadata().map_or(false, |metadata| metadata.is_dir()) => {
let file_name = f.file_name();
let file_name = file_name.to_string_lossy();
if file_name == "build" || file_name == "lib" {
search_lib_dir(f.path(), cross)
search_lib_dir(f.path(), cross)?
} else if file_name.starts_with("lib.") {
// check if right target os
if !file_name.contains(&cross.target.operating_system.to_string()) {
Expand All @@ -1347,12 +1349,12 @@ fn search_lib_dir(path: impl AsRef<Path>, cross: &CrossCompileConfig) -> Vec<Pat
if !file_name.contains(&cross.target.architecture.to_string()) {
continue;
}
search_lib_dir(f.path(), cross)
search_lib_dir(f.path(), cross)?
} else if is_cpython_lib_dir(&file_name, &cross.version)
|| is_pypy_lib_dir(&file_name, &cross.version)
|| is_graalpy_lib_dir(&file_name, &cross.version)
{
search_lib_dir(f.path(), cross)
search_lib_dir(f.path(), cross)?
} else {
continue;
}
Expand Down Expand Up @@ -1381,7 +1383,7 @@ fn search_lib_dir(path: impl AsRef<Path>, cross: &CrossCompileConfig) -> Vec<Pat
}
}

sysconfig_paths
Ok(sysconfig_paths)
}

/// Find cross compilation information from sysconfigdata file
Expand Down
Loading