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.

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

use struct.calcsize("P") rather than platform.machine() #830

Merged
merged 5 commits into from
Mar 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
58 changes: 33 additions & 25 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ struct InterpreterConfig {
/// Prefix used for determining the directory of libpython
base_prefix: String,
executable: String,
machine: String,
calcsize_pointer: Option<u32>,
}

#[derive(Deserialize, Debug, Clone, PartialEq)]
Expand Down Expand Up @@ -180,7 +180,7 @@ fn load_cross_compile_info() -> Result<(InterpreterConfig, HashMap<String, Strin
ld_version: "".to_string(),
base_prefix: "".to_string(),
executable: "".to_string(),
machine: "".to_string(),
calcsize_pointer: None,
};

Ok((interpreter_config, fix_config_map(config_map)))
Expand Down Expand Up @@ -433,10 +433,11 @@ fn find_interpreter_and_get_config() -> Result<(InterpreterConfig, HashMap<Strin
/// Extract compilation vars from the specified interpreter.
fn get_config_from_interpreter(interpreter: &str) -> Result<InterpreterConfig> {
let script = r#"
import json
import platform
import struct
import sys
import sysconfig
import platform
import json

PYPY = platform.python_implementation() == "PyPy"

Expand All @@ -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)?;
Expand All @@ -475,7 +476,7 @@ fn configure(interpreter_config: &InterpreterConfig) -> Result<String> {
}
}

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") {
Expand Down Expand Up @@ -517,32 +518,39 @@ fn configure(interpreter_config: &InterpreterConfig) -> Result<String> {
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<()> {
Expand Down
4 changes: 3 additions & 1 deletion ci/actions/test.sh
Original file line number Diff line number Diff line change
@@ -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
Expand Down
28 changes: 23 additions & 5 deletions examples/rustapi_module/tests/test_datetime.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import datetime as pdt
import sys
import platform
import struct
import sys

import pytest
import rustapi_module.datetime as rdt
from hypothesis import given, example
from hypothesis import strategies as st
from hypothesis.strategies import dates, datetimes


# Constants
Expand Down Expand Up @@ -40,16 +40,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)
Expand All @@ -66,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():
Expand All @@ -86,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):
Expand Down Expand Up @@ -225,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):
Expand Down