Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

notgull
Copy link
Member

@notgull notgull commented Nov 27, 2024

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.

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@notgull notgull force-pushed the notgull/install-docs branch from 55d397b to a8d090a Compare November 27, 2024 03:45
@notgull notgull requested review from madsmtm and kchibisov and removed request for madsmtm November 27, 2024 03:45
Copy link
Member Author

@notgull notgull left a 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
Comment on lines 38 to 42
//! For the Wayland backend, the following Ubuntu packages or their equivalents
//! must be installed.
//!
//! - `libwayland-dev`
//! - `libxcbcommon-dev`
Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member Author

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
Comment on lines 47 to 49
//! 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.
Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indicated as such

Copy link
Member

@madsmtm madsmtm Dec 3, 2024

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)

@notgull notgull added S - docs Awareness, docs, examples, etc. C - nominated Nominated for discussion in the next meeting labels Nov 27, 2024
@notgull notgull force-pushed the notgull/install-docs branch from a8d090a to 63bcbbb Compare November 27, 2024 03:47
src/lib.rs Outdated
Comment on lines 27 to 49
//! ## 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.
Copy link
Member

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::*

Copy link
Member

@madsmtm madsmtm Nov 27, 2024

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

Suggested change
//! ## 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.

Copy link
Member Author

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.

Copy link
Member

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)?

Copy link
Member

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

Copy link
Member Author

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.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@notgull notgull force-pushed the notgull/install-docs branch from 63bcbbb to 19320b4 Compare December 3, 2024 04:18
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]>
@notgull notgull force-pushed the notgull/install-docs branch from 91bab7f to 19477d3 Compare December 8, 2024 16:30
//!
//! ## Dependencies
//!
//! Dependencies on non-system libraries is managed through Cargo. For the X11
Copy link
Member

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.
Copy link
Member

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//! [^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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - nominated Nominated for discussion in the next meeting S - docs Awareness, docs, examples, etc.
Development

Successfully merging this pull request may close these issues.

4 participants