From 4a0c316b234036f3019712d1c2022dd29a208ee8 Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Tue, 24 Mar 2020 11:45:12 -0400 Subject: [PATCH 1/5] make test.sh fail if any command fails --- ci/actions/test.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/actions/test.sh b/ci/actions/test.sh index 6f0145859ed..0b2db4cf038 100755 --- a/ci/actions/test.sh +++ b/ci/actions/test.sh @@ -1,6 +1,8 @@ #!/bin/bash -cargo test --features "$FEATURES num-bigint num-complex" +set -e -u -o pipefail + +cargo test --features "${FEATURES:-} num-bigint num-complex" (cd pyo3-derive-backend; cargo test) for example_dir in examples/*; do From 139a1e6e6ba2b2a62cb916350e87a7c7d9c8725e Mon Sep 17 00:00:00 2001 From: Yuji Kanagawa Date: Wed, 25 Mar 2020 11:28:33 +0900 Subject: [PATCH 2/5] Use checkout@v2 --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 27c565e0413..941f4e3fb3b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,7 +17,7 @@ jobs: ] steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v2 - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v1 with: From f3876a90b34b92641157cc0af047139f896e968e Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Mon, 23 Mar 2020 13:01:59 -0400 Subject: [PATCH 3/5] use struct.calcsize("P") rather than platform.machine() platform.machine() gives the wrong answer if you're running 32-bit Python on a 64-bit machine. The reason we don't use platform.architecture() here is that it's not reliable on macOS. See https://stackoverflow.com/a/1405971/823869. Similarly, sys.maxsize is not reliable on Windows. See https://stackoverflow.com/questions/1405913/how-do-i-determine-if-my-python-shell-is-executing-in-32bit-or-64bit-mode-on-os/1405971#comment6209952_1405971 and https://stackoverflow.com/a/3411134/823869. Also use CARGO_CFG_TARGET_POINTER_WIDTH rather than inferring the Rust target pointer width from CARGO_CFG_TARGET_ARCH. --- build.rs | 58 ++++++++++++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/build.rs b/build.rs index 493c8b0b74f..6be47e7cbff 100644 --- a/build.rs +++ b/build.rs @@ -32,7 +32,7 @@ struct InterpreterConfig { /// Prefix used for determining the directory of libpython base_prefix: String, executable: String, - machine: String, + calcsize_pointer: Option, } #[derive(Deserialize, Debug, Clone, PartialEq)] @@ -180,7 +180,7 @@ fn load_cross_compile_info() -> Result<(InterpreterConfig, HashMap Result<(InterpreterConfig, HashMap Result { let script = r#" +import json +import platform +import struct import sys import sysconfig -import platform -import json PYPY = platform.python_implementation() == "PyPy" @@ -456,7 +457,7 @@ print(json.dumps({ "base_prefix": base_prefix, "shared": PYPY or bool(sysconfig.get_config_var('Py_ENABLE_SHARED')), "executable": sys.executable, - "machine": platform.machine() + "calcsize_pointer": struct.calcsize("P"), })) "#; let json = run_python_script(interpreter, script)?; @@ -475,7 +476,7 @@ fn configure(interpreter_config: &InterpreterConfig) -> Result { } } - check_target_architecture(&interpreter_config.machine)?; + check_target_architecture(interpreter_config)?; let is_extension_module = env::var_os("CARGO_FEATURE_EXTENSION_MODULE").is_some(); if !is_extension_module || cfg!(target_os = "windows") { @@ -517,32 +518,39 @@ fn configure(interpreter_config: &InterpreterConfig) -> Result { Ok(flags) } -fn check_target_architecture(python_machine: &str) -> Result<()> { +fn check_target_architecture(interpreter_config: &InterpreterConfig) -> Result<()> { // Try to check whether the target architecture matches the python library - let target_arch = match env::var("CARGO_CFG_TARGET_ARCH") - .as_ref() - .map(|e| e.as_str()) - { - Ok("x86_64") => Some("64-bit"), - Ok("x86") => Some("32-bit"), - _ => None, // It might be possible to recognise other architectures, this will do for now. + let rust_target = match env::var("CARGO_CFG_TARGET_POINTER_WIDTH")?.as_str() { + "64" => "64-bit", + "32" => "32-bit", + x => bail!("unexpected Rust target pointer width: {}", x), }; - let python_arch = match python_machine { - "AMD64" | "x86_64" => Some("64-bit"), - "i686" | "x86" => Some("32-bit"), - _ => None, // It might be possible to recognise other architectures, this will do for now. + // The reason we don't use platform.architecture() here is that it's not + // reliable on macOS. See https://stackoverflow.com/a/1405971/823869. + // Similarly, sys.maxsize is not reliable on Windows. See + // https://stackoverflow.com/questions/1405913/how-do-i-determine-if-my-python-shell-is-executing-in-32bit-or-64bit-mode-on-os/1405971#comment6209952_1405971 + // and https://stackoverflow.com/a/3411134/823869. + let python_target = match interpreter_config.calcsize_pointer { + Some(8) => "64-bit", + Some(4) => "32-bit", + None => { + // Unset, e.g. because we're cross-compiling. Don't check anything + // in this case. + return Ok(()); + } + Some(n) => bail!("unexpected Python calcsize_pointer value: {}", n), }; - match (target_arch, python_arch) { - // If we could recognise both, and they're different, fail. - (Some(t), Some(p)) if p != t => bail!( + if rust_target != python_target { + bail!( "Your Rust target architecture ({}) does not match your python interpreter ({})", - t, - p - ), - _ => Ok(()), + rust_target, + python_target + ); } + + Ok(()) } fn check_rustc_version() -> Result<()> { From 9e23476221fb07da86973434b0da114dff766a4a Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Wed, 25 Mar 2020 12:09:25 -0400 Subject: [PATCH 4/5] avoid using platform.architecture() to detect 32-bit-ness in datetime tests Same reasoning as the previous commit. --- .../rustapi_module/tests/test_datetime.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/examples/rustapi_module/tests/test_datetime.py b/examples/rustapi_module/tests/test_datetime.py index 4e4ae3d51dc..fa859936d1a 100644 --- a/examples/rustapi_module/tests/test_datetime.py +++ b/examples/rustapi_module/tests/test_datetime.py @@ -1,6 +1,7 @@ import datetime as pdt -import sys import platform +import struct +import sys import pytest import rustapi_module.datetime as rdt @@ -40,16 +41,27 @@ def tzname(self, dt): MAX_MICROSECONDS = int(pdt.timedelta.max.total_seconds() * 1e6) MIN_MICROSECONDS = int(pdt.timedelta.min.total_seconds() * 1e6) -IS_X86 = platform.architecture()[0] == "32bit" +# The reason we don't use platform.architecture() here is that it's not +# reliable on macOS. See https://stackoverflow.com/a/1405971/823869. Similarly, +# sys.maxsize is not reliable on Windows. See +# https://stackoverflow.com/questions/1405913/how-do-i-determine-if-my-python-shell-is-executing-in-32bit-or-64bit-mode-on-os/1405971#comment6209952_1405971 +# and https://stackoverflow.com/a/3411134/823869. +_pointer_size = struct.calcsize("P") +if _pointer_size == 8: + IS_32_BIT = False +elif _pointer_size == 4: + IS_32_BIT = True +else: + raise RuntimeError("unexpected pointer size: " + repr(_pointer_size)) IS_WINDOWS = sys.platform == "win32" if IS_WINDOWS: MIN_DATETIME = pdt.datetime(1970, 1, 2, 0, 0) - if IS_X86: + if IS_32_BIT: MAX_DATETIME = pdt.datetime(3001, 1, 19, 4, 59, 59) else: MAX_DATETIME = pdt.datetime(3001, 1, 19, 7, 59, 59) else: - if IS_X86: + if IS_32_BIT: # TS ±2147483648 (2**31) MIN_DATETIME = pdt.datetime(1901, 12, 13, 20, 45, 52) MAX_DATETIME = pdt.datetime(2038, 1, 19, 3, 14, 8) From d2c07a87d28dba50ecba08ca356f056d14fdec70 Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Fri, 27 Mar 2020 11:29:49 -0400 Subject: [PATCH 5/5] xfail a couple of datetime tests on Python 3.5 + macOS --- examples/rustapi_module/tests/test_datetime.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/examples/rustapi_module/tests/test_datetime.py b/examples/rustapi_module/tests/test_datetime.py index fa859936d1a..2f6e4019314 100644 --- a/examples/rustapi_module/tests/test_datetime.py +++ b/examples/rustapi_module/tests/test_datetime.py @@ -7,7 +7,6 @@ import rustapi_module.datetime as rdt from hypothesis import given, example from hypothesis import strategies as st -from hypothesis.strategies import dates, datetimes # Constants @@ -78,6 +77,11 @@ def tzname(self, dt): reason="Date bounds were not checked in the C constructor prior to version 3.6", ) +xfail_macos_datetime_bounds = pytest.mark.xfail( + sys.version_info < (3, 6) and platform.system() == "Darwin", + reason="Unclearly failing. See https://github.com/PyO3/pyo3/pull/830 for more.", +) + # Tests def test_date(): @@ -98,6 +102,7 @@ def test_invalid_date_fails(): rdt.make_date(2017, 2, 30) +@xfail_macos_datetime_bounds @given(d=st.dates(MIN_DATETIME.date(), MAX_DATETIME.date())) def test_date_from_timestamp(d): if PYPY and d < pdt.date(1900, 1, 1): @@ -237,6 +242,7 @@ def test_datetime_typeerror(): rdt.make_datetime("2011", 1, 1, 0, 0, 0, 0) +@xfail_macos_datetime_bounds @given(dt=st.datetimes(MIN_DATETIME, MAX_DATETIME)) @example(dt=pdt.datetime(1970, 1, 2, 0, 0)) def test_datetime_from_timestamp(dt):