-
Notifications
You must be signed in to change notification settings - Fork 222
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
gate the build with target_os = "macos" #506
base: main
Are you sure you want to change the base?
Conversation
Why is core-graphics-types being compiled on Linux? |
This is due to the peculiarities of how the Debian resolution of source packaged work, I'll try to explain why I packaged it. Problem statementI would like to package the font-kit package, and that depends on core-graphics that depend on core-graphics-types. A deeper look into whyEach rust crates becomes one source package in Debian, so that the copyright + license information can be reviewed, and in according to the Debian policy of not vendoring dependencies. Each dependency from Cargo.toml becomes one item in the Debian Since the Then the option becomes to either patch the font-kit package to remove all unneeded dependencies, or to package those into their own debian packages. I prefer to reduce the amount of debian specific patches if that is possible, so I packaged core-graphics-types, that turned out to still require a patch, since the code didn't even build on linux, but I thought I could at least provide that patch upstream. Sorry for the wall of text, but I hope that this explains the background of the patch. |
Does every foundational crate used by a cross-platform rust library require patches like this? |
I think many build cross platform without trouble as it is today, we also package windows api crates for example: https://packages.debian.org/sid/librust-winapi-dev I don't know if all of them have gotten patches from some debian package maintainer at some point in time or not :) |
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 don't know anything about the Debian depends
translation, but the fact that it wants to use non-unix dependencies sounds to me more like a bug in that? I mean, font-kit
's Cargo.toml
clearly describes that these dependencies are macOS/iOS only (same goes for winapi
, it shouldn't have to download that extra 1MB of useless data).
But, even barring that, what if we at some point to support a Linux version of Core Graphics like GNUStep's Opal (not entirely theoretical, I might want to do that at some point)? Then your Debian build would start failing again, since we try to link to something that doesn't exist. And then you would need to package Opal, and all of it's sub-dependencies, even though none of it is ever used in font-kit
.
core-graphics-types/src/lib.rs
Outdated
#[cfg(target_os = "macos")] | ||
pub mod base; | ||
#[cfg(target_os = "macos")] | ||
pub mod geometry; |
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.
This would remove support for iOS and tvOS (and watchOS), those should allowed as well.
☔ The latest upstream changes (presumably b009c87) made this pull request unmergeable. Please resolve the merge conflicts. |
so that we don't get build errors on linux.
Ref: https://salsa.debian.org/rust-team/debcargo-conf/-/blob/master/src/core-graphics-types/debian/patches/cnf-macos.patch