-
Notifications
You must be signed in to change notification settings - Fork 1
Make it work on more platforms #16
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
Conversation
| # depends on the headers through cpython-sys | ||
| rule="$objs: cpython-sys \$(srcdir)/Cargo.toml \$(srcdir)/Cargo.lock \$(srcdir)/$srcdir/$manifest $prefixed_srcs \$(PYTHON_HEADERS)" | ||
| rule="$rule; cargo build --lib --locked --package ${mods} --profile \$(CARGO_PROFILE)" | ||
| rule="$rule; CARGO_TARGET_DIR=\$(abs_builddir)/target PYTHON_BUILD_DIR=\$(abs_builddir) cargo build --lib --locked --package ${mods} --profile \$(CARGO_PROFILE) --manifest-path \$(srcdir)/Cargo.toml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this because some platforms build CPython outside the main source tree
| #[cfg(py_gil_disabled)] | ||
| pub const PyObject_HEAD_INIT: PyObject = { | ||
| let mut obj: PyObject = unsafe { std::mem::MaybeUninit::zeroed().assume_init() }; | ||
| obj.ob_flags = _Py_STATICALLY_ALLOCATED_FLAG as _; | ||
| obj | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A temporary work around to make it build for free-threaded build
| def list_rust_modules(names: set[str]) -> None: | ||
| if not os.path.isdir(MODULES_PATH): | ||
| return | ||
| for entry in os.scandir(MODULES_PATH): | ||
| if not entry.is_dir(): | ||
| continue | ||
| if entry.name == "cpython-sys": | ||
| continue | ||
| cargo_toml = os.path.join(entry.path, "Cargo.toml") | ||
| if os.path.isfile(cargo_toml): | ||
| names.add(entry.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don’t actually need this in this PR, but we’ll eventually need something like it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be fine! We can always modify it later as needed.
emmatyping
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
No description provided.