-
Notifications
You must be signed in to change notification settings - Fork 920
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 installation instructions to top-level docs #4010
base: master
Are you sure you want to change the base?
Conversation
55d397b
to
a8d090a
Compare
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.
@rust-windowing/winit Please review this one.
src/lib.rs
Outdated
//! For the Wayland backend, the following Ubuntu packages or their equivalents | ||
//! must be installed. | ||
//! | ||
//! - `libwayland-dev` | ||
//! - `libxcbcommon-dev` |
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.
@kchibisov Are there any additional systems dependencies needed on Wayland? I'm not sure about this.
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.
you'd need libfontconfig/freetype
once you enable sctk-adwaita
feature.
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.
Indicated as such
src/lib.rs
Outdated
//! The other backends (Windows, macOS, etc) do not have any dependencies on system libraries | ||
//! that don't already come with the operating system. However, note that the Windows backend | ||
//! only supports Windows 10 and above. |
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.
@madsmtm @MarijnS95 I know we support a specific Android version and up, but I'm not clear on macOS.
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.
See winit::platform::macos
;)
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.
Indicated as such
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.
The latest master
shows a different version though, so now the two are out of sync :/ (my bad for giving you the wrong link)
a8d090a
to
63bcbbb
Compare
src/lib.rs
Outdated
//! ## Dependencies | ||
//! | ||
//! Dependencies on non-system libraries is managed through Cargo. For the X11 | ||
//! backend, the following Ubuntu packages or their equivalents must be installed. | ||
//! | ||
//! - `libx11-dev` | ||
//! - `libxcb1-dev` | ||
//! - `libxi-dev` | ||
//! - `libxcbcommon-dev` | ||
//! - `libxcbcommon-x11-dev` | ||
//! | ||
//! For the Wayland backend, the following Ubuntu packages or their equivalents | ||
//! must be installed. | ||
//! | ||
//! - `libwayland-dev` | ||
//! - `libxcbcommon-dev` | ||
//! | ||
//! The "dev" packages are only needed for building binaries that use `winit`. On | ||
//! deployed system the non-`dev` equivalents need to be installed. | ||
//! | ||
//! The other backends (Windows, macOS, etc) do not have any dependencies on system libraries | ||
//! that don't already come with the operating system. However, note that the Windows backend | ||
//! only supports Windows 10 and above. |
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.
Nit: Slight preference for putting this information in winit::platform::*
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.
And then linking to that, if you really want to
//! ## Dependencies | |
//! | |
//! Dependencies on non-system libraries is managed through Cargo. For the X11 | |
//! backend, the following Ubuntu packages or their equivalents must be installed. | |
//! | |
//! - `libx11-dev` | |
//! - `libxcb1-dev` | |
//! - `libxi-dev` | |
//! - `libxcbcommon-dev` | |
//! - `libxcbcommon-x11-dev` | |
//! | |
//! For the Wayland backend, the following Ubuntu packages or their equivalents | |
//! must be installed. | |
//! | |
//! - `libwayland-dev` | |
//! - `libxcbcommon-dev` | |
//! | |
//! The "dev" packages are only needed for building binaries that use `winit`. On | |
//! deployed system the non-`dev` equivalents need to be installed. | |
//! | |
//! The other backends (Windows, macOS, etc) do not have any dependencies on system libraries | |
//! that don't already come with the operating system. However, note that the Windows backend | |
//! only supports Windows 10 and above. | |
//! ## Dependencies | |
//! | |
//! Dependencies on non-system libraries is managed through Cargo. | |
//! See the [`platform`] documentation for information on system dependencies. |
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'd like to avoid burying this information if we can. Everyone who uses winit should know what system libraries need to be installed.
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.
How about putting it in the top-level docs on winit::platform
, then (along with the Tier 1 and Tier 2 target lists)?
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.
Idk., I just kinda think we're giving undue weight to Linux/other free unix needs here? Like, system libraries (and the OS version, since it's the same as rustc
) aren't a problem on macOS/iOS, but there's a bunch of other very important concerns you need to be aware of there. Even more so with Android. But we still put this sort of information in winit::platform
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 is largely motivated by having to deal with persistent issues (read: NixOS) where people don't have packages set up properly on Linux. I'd like to have this information in as visible of a place as possible.
63bcbbb
to
19320b4
Compare
This commit adds installation instructions to the top-level documentation for lib.rs. The main goal is to give the dependencies that X11/Wayland use in a formal place, since it's not written down anywhere to my knowledge. Signed-off-by: John Nunley <[email protected]>
91bab7f
to
19477d3
Compare
//! | ||
//! ## Dependencies | ||
//! | ||
//! Dependencies on non-system libraries is managed through Cargo. For the X11 |
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.
What does this mean, exactly?
//! $ cargo add winit --no-default-features --features wayland | ||
//! ``` | ||
//! | ||
//! These features have no effect on systems that are not Free Unix. |
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.
Should this spend a few more words explaining that:
- Disabling default features on other systems/targets has no effect, since they only have one "implementation" that is unconditionally compiled in;
- Enabling
x11
/wayland
is a no-op on non-Free-Unix systems, i.e. enabling it while also building for Windows or Android doesn't suddenly break the build?
//! 10.14 and above. | ||
//! | ||
//! [^unix]: Unix systems outside of Android and Apple, like Linux or FreeBSD. | ||
//! [^must]: This is not a "must" when the "dlopen" features are enabled |
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.
//! [^must]: This is not a "must" when the "dlopen" features are enabled | |
//! [^must]: This is not a "must" when the "dlopen" features are enabled. |
To match the other footnote?
This commit adds installation instructions to the top-level
documentation for lib.rs. The main goal is to give the dependencies that
X11/Wayland use in a formal place, since it's not written down anywhere
to my knowledge.
changelog
module if knowledge of this change could be valuable to users