-
Notifications
You must be signed in to change notification settings - Fork 431
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
Fix clippy warnings #679
Fix clippy warnings #679
Conversation
andrewjcg
commented
Sep 3, 2024
``` warning: unexpected `cfg` condition name: `unwind` --> src/python_spy.rs:38:11 | 38 | #[cfg(unwind)] | ^^^^^^ help: found config with similar value: `panic = "unwind"` | = help: consider using a Cargo feature instead = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint: [lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(unwind)'] } = help: or consider adding `println!("cargo::rustc-check-cfg=cfg(unwind)");` to the top of the `build.rs` = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration ```
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 for the PR - this is really helpful!
As a note, I approached this quite differently in zanieb@045846a (and zanieb@224a783) — the downside to that is that it requires a newer Cargo version in your cross-build images. |
@zanieb nice! that does seem like a simpler change - updating the docker cross-build images is something we should do at some point, but I'm inclined to get this PR merged to unblock some other necessary changes. |
Definitely whatever works best for you. The cross-build images merge with the upstream without conflict. I'm having some trouble building them though (mostly due to lack of understanding of how they're intended to be built). |
also try adding the x86_64-apple-darwin target
I usually build with:
(which is a subset of the upstream project, but all I need for py-spy). |
I saw the build script but I'm on an aarch64 mac so I can't run it locally. I'm building them in GitHub instead. I think I've got it mostly figured out now. I can open a pull request if you open your repository to contributions. (Sorry for the unrelated discussion on this PR!) |
I would appreciate a PR here - thanks for updating this! |
Also unrelated - but the windows build CI succeeded on this PR https://github.com/benfred/py-spy/actions/runs/11264051989/job/31323278597 but then immediately failed on the main branch =( https://github.com/benfred/py-spy/actions/runs/11264590823/job/31324954658 . |