-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add python windows #67 #68
Conversation
@@ -29,7 +30,7 @@ pub fn pyspy_backend(config: PyspyConfig) -> BackendImpl<BackendUninitialized> { | |||
#[derive(Debug, Clone)] | |||
pub struct PyspyConfig { | |||
/// Process to monitor | |||
pid: Option<i32>, | |||
pid: Option<Pid>, |
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.
Use the type remoteprocess::Pid
that is dynamic depending on the target platform.
@@ -7,7 +7,7 @@ edition = "2021" | |||
|
|||
[dependencies] | |||
interprocess = { version = "1.1.1", features = ["nonblocking"] } | |||
pyroscope = "0.5.3" | |||
pyroscope = { path = "../../" } |
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.
Ensure we're referencing the checked out version of pyroscope-rs.
@@ -74,11 +74,19 @@ mod check_err_tests { | |||
} | |||
|
|||
/// libc::epoll_ctl wrapper | |||
#[cfg(not(target_os = "windows"))] |
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.
If we're not on windows, don't do anything different.
pub fn pthread_self() -> Result<u64> { | ||
let thread_id = check_err(unsafe { libc::pthread_self() })? as u64; | ||
Ok(thread_id) | ||
} | ||
|
||
|
||
#[cfg(target_os = "windows")] |
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.
if we are on windows, use winapi
to get the thread id.
I have the windows build going now. I need to try installing it and see if it works. |
So, it's working on windows. I am able to send telemetry to https://pyroscope.cloud and everything :) |
Hi. Thanks a lot for contribution and sorry for long response. Thanks a lot! |
closes #67
I am looking into how to add support for Python on Windows.
The main issue seems to be a platform dependent type in a transitive dependency of
pyroscope_pyspy
. I'm not sure what the best method for addressing this is but I went with adding a direct dependency and pulling the type in from there.