build.rs: if found more than one candidate, filter on arch #1626

alonblade · 2021-05-24T12:16:38Z

Hi,

This is my first RP on pyo3. Thanks for the great work, I'm using this right now to convert a library from python to rust for a raspberry pi running in 32 bit mode, that is armhf architecture, using ubuntu 20.04 as the target distribution. I have both the host x86_64 python3.8 sysconfigdata and the foreign architecture armhf in /usr/lib/python3.8, which results in more than one candidate during search_lib_dir, and an error of "Detected multiple possible python versions".

I've conservatively added a filter in that case to keep only the paths with cross.arch in their name (to_lossy_string.contains(&config.arch))

There is already an existing filter on config.arch but only in another branch, so I didn't want to pull it in to the first branch (Ok(f) if starts_with(f, "_sysconfigdata") && ends_with(f, "py")).

I added a line in CHANGELOG, I think this is a bug so it should not change the documentation.

I've run clippy (no errors), run fmt (no changes), "cargo test" fails but on unrelated code (I can attach, fails in examples).

Alon

davidhewitt

Hi @alonblade, thanks for the PR!

We literally just merged a change to the build infrastructure, so I had to rebase and force-push your PR. Hope that's ok.

Let's see how CI does with this branch, also I have one thought...

davidhewitt · 2021-05-25T06:22:38Z

pyo3-build-config/src/impl_.rs

+    // If we got more than one file, only take those that contain the arch name.
+    // For ubuntu 20.04 with host architecture x86_64 and a foreign architecture of armhf
+    // this reduces the number of candidates to 1:
+    //
+    // $ find /usr/lib/python3.8/ -name '_sysconfigdata*.py' -not -lname '*'
+    //  /usr/lib/python3.8/_sysconfigdata__x86_64-linux-gnu.py
+    //  /usr/lib/python3.8/_sysconfigdata__arm-linux-gnueabihf.py
+    if sysconfig_paths.len() > 1 {
+        sysconfig_paths = sysconfig_paths
+            .iter()
+            .filter(|p| p.to_string_lossy().contains(&cross.arch))
+            .map(|p| p.clone())
+            .collect::<Vec<PathBuf>>();
+    }


The only potential issue I see is that if the arch doesn't match any sysconfig file (e.g. maybe the arch is spelt differently for some reason?) then this could reduce the number of candidates to zero.

In that case, it might be worth keeping the original full set of paths instead of listing zero paths and failing?

alonblade · 2021-05-25T07:15:28Z

ok, I made a mess by accidentally pushing with force my updated change, so now I'm redoing the rebase you did for me.

…

On Tue, May 25, 2021 at 9:23 AM David Hewitt ***@***.***> wrote: ***@***.**** commented on this pull request. Hi @alonblade <https://github.com/alonblade>, thanks for the PR! We literally just merged a change to the build infrastructure, so I had to rebase and force-push your PR. Hope that's ok. Let's see how CI does with this branch, also I have one thought... ------------------------------ In pyo3-build-config/src/impl_.rs <#1626 (comment)>: > + // If we got more than one file, only take those that contain the arch name. + // For ubuntu 20.04 with host architecture x86_64 and a foreign architecture of armhf + // this reduces the number of candidates to 1: + // + // $ find /usr/lib/python3.8/ -name '_sysconfigdata*.py' -not -lname '*' + // /usr/lib/python3.8/_sysconfigdata__x86_64-linux-gnu.py + // /usr/lib/python3.8/_sysconfigdata__arm-linux-gnueabihf.py + if sysconfig_paths.len() > 1 { + sysconfig_paths = sysconfig_paths + .iter() + .filter(|p| p.to_string_lossy().contains(&cross.arch)) + .map(|p| p.clone()) + .collect::<Vec<PathBuf>>(); + } The only potential issue I see is that if the arch doesn't match *any* sysconfig file (e.g. maybe the arch is spelt differently for some reason?) then this could reduce the number of candidates to zero. In that case, it might be worth keeping the original full set of paths instead of listing zero paths and failing? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1626 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASHRLMN5LNDHBWJO3TDWOLLTPM67VANCNFSM45NEBKDQ> .

alonblade · 2021-05-25T07:22:37Z

@davidhewitt hmm, now that I rebased on latest I get a different error from my use case (pi armhf target, x86_64 host) - Trying to debug it.

   Compiling pyo3 v0.13.2 (/home/alon/catkin_ws/mpu6050/sub/pyo3)
error: failed to run custom build command for `pyo3 v0.13.2 (/home/alon/catkin_ws/mpu6050/sub/pyo3)`

Caused by:
  process didn't exit successfully: `/home/alon/catkin_ws/mpu6050/target/release/build/pyo3-6c31d3e862b113d6/build-script-build` (exit status: 1)
  --- stderr
  error: Your Rust target architecture (32-bit) does not match your python interpreter (64-bit)

davidhewitt · 2021-05-25T07:33:42Z

@alonblade yikes, I'm pretty sure this is my fault; I have a guess at what's going on.

I'll try to open a PR to fix tonight.

alonblade · 2021-05-25T08:01:37Z

@davidhewitt ok, I think I found the problem: the new refactoring of pyo3-build-config out of build.rs means that when
pyo3-build-config/src/impl_.rs is run the value of the cargo target environment variables is actually the host, not the real target. So cross.arch is actually x86_64, and I get a false error. To confirm, I commented out ensure_target_architecture and tested the resulting binary and it is the correct architecture.

I am not sure how to proceed - one option would be to pass the real target variables to pyo3-build-config in a different set of environment variables (i.e. CARGO_CFG_TARGET_ARCH becomes PYO3_CARGO_CFG_TARGET_ARCH)

What do you think?

alonblade · 2021-05-25T08:09:38Z

@alonblade yikes, I'm pretty sure this is my fault; I have a guess at what's going on.

I'll try to open a PR to fix tonight.

Thanks!

…refactor PyO3#1626 (comment)

davidhewitt · 2021-05-25T22:34:41Z

(Apologies, I've run out of time to try to fix this tonight. I'm going to attempt to find time again soon, hopefully on Thursday evening.)

alonblade · 2021-05-26T06:29:58Z

Sweet. No worries.

…

On Wed, May 26, 2021 at 1:34 AM David Hewitt ***@***.***> wrote: (Apologies, I've run out of time to try to fix this tonight. I'm going to attempt to find time again soon, hopefully on Thursday evening.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1626 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASHRLMORTYEDPDLJ6VE3YILTPQQZBANCNFSM45NEBKDQ> .

If we got more then one file, only take those that contain the arch name. For ubuntu 20.04 with host architecture x86_64 and a foreign architecture of armhf this reduces the number of candidates to 1: $ find /usr/lib/python3.8/ -name '_sysconfigdata*.py' -not -lname '*' /usr/lib/python3.8/_sysconfigdata__x86_64-linux-gnu.py /usr/lib/python3.8/_sysconfigdata__arm-linux-gnueabihf.py CHANGELOG.md: add entry for cross-sysconfigdata filter on arch commit changelog: 1. initial 2. if filtered list is empty, use pre filtered. 3. clippy is_empty and cloned

…refactor PyO3#1626 (comment)

davidhewitt · 2021-06-05T08:28:54Z

I'm going to merge this and rebase my cross-compilation fix on it. Thanks for implementing this!

alonblade · 2021-06-05T13:40:48Z

Super, thanks!

…

On Sat, Jun 5, 2021 at 11:29 AM David Hewitt ***@***.***> wrote: Merged #1626 <#1626> into main. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1626 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASHRLMNCRDDVAJLVD6FZCQDTRHN5XANCNFSM45NEBKDQ> .

Same as PyO3/pyo3#1626

alonblade force-pushed the cross-sysconfig branch 2 times, most recently from aeb578f to ee492ca Compare May 24, 2021 12:18

davidhewitt force-pushed the cross-sysconfig branch from ee492ca to 16e297f Compare May 25, 2021 06:20

davidhewitt reviewed May 25, 2021

View reviewed changes

alonblade force-pushed the cross-sysconfig branch from 16e297f to ba19e7c Compare May 25, 2021 07:13

alonblade force-pushed the cross-sysconfig branch from ba19e7c to 95cb323 Compare May 25, 2021 07:21

alonblade added a commit to alonblade/pyo3 that referenced this pull request May 25, 2021

TEMP disable ensure_target_architecture false positive from build.rs …

8636c6b

…refactor PyO3#1626 (comment)

alonblade force-pushed the cross-sysconfig branch from 95cb323 to 96a6739 Compare May 25, 2021 10:04

davidhewitt mentioned this pull request May 25, 2021

0.14 Release #1570

Closed

7 tasks

alonblade force-pushed the cross-sysconfig branch from 96a6739 to e5542f2 Compare May 30, 2021 07:09

alonblade added a commit to alonblade/pyo3 that referenced this pull request May 30, 2021

TEMP disable ensure_target_architecture false positive from build.rs …

aa34490

…refactor PyO3#1626 (comment)

davidhewitt merged commit 97d6f15 into PyO3:main Jun 5, 2021

messense added a commit to messense/maturin that referenced this pull request Jun 9, 2021

Filter target architecture for cross compiling sysconfigdata candidates

f9306c7

Same as PyO3/pyo3#1626

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build.rs: if found more than one candidate, filter on arch #1626

build.rs: if found more than one candidate, filter on arch #1626

alonblade commented May 24, 2021

davidhewitt left a comment

davidhewitt May 25, 2021

alonblade commented May 25, 2021 via email

alonblade commented May 25, 2021

davidhewitt commented May 25, 2021

alonblade commented May 25, 2021

alonblade commented May 25, 2021

davidhewitt commented May 25, 2021

alonblade commented May 26, 2021 via email

davidhewitt commented Jun 5, 2021

alonblade commented Jun 5, 2021 via email

build.rs: if found more than one candidate, filter on arch #1626

build.rs: if found more than one candidate, filter on arch #1626

Conversation

alonblade commented May 24, 2021

davidhewitt left a comment

Choose a reason for hiding this comment

davidhewitt May 25, 2021

Choose a reason for hiding this comment

alonblade commented May 25, 2021 via email

alonblade commented May 25, 2021

davidhewitt commented May 25, 2021

alonblade commented May 25, 2021

alonblade commented May 25, 2021

davidhewitt commented May 25, 2021

alonblade commented May 26, 2021 via email

davidhewitt commented Jun 5, 2021

alonblade commented Jun 5, 2021 via email