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

Readme: Add Installation on Alpine Linux and Repology badge #42

Merged
merged 2 commits into from
Feb 6, 2022
Merged

Readme: Add Installation on Alpine Linux and Repology badge #42

merged 2 commits into from
Feb 6, 2022

Conversation

jirutka
Copy link
Contributor

@jirutka jirutka commented Feb 6, 2022

No description provided.

@jirutka jirutka changed the title Readme: Add Installation on Alpine Linux Readme: Add Installation on Alpine Linux and Repology badge Feb 6, 2022
@maximbaz
Copy link
Owner

maximbaz commented Feb 6, 2022

Thanks!

Your package is a golden mine of interesting stuff!

First of all, wow, it seems you managed to get it running with musl! I had some people asking about it, and my quick attempt failed. It would be cool to have necessary changes upstreamed, I'd also love to be able to provide pre-compiled musl variant as a Github release (not a big priority, a nice to have).

Ack for #43, cc @cyrinux, we'll have a look into it! If you already know of some better alternatives, please don't hesitate to share 😉

Using linked feature for ash is an interesting idea but won't this cause problems for people whose hardware doesn't support Vulkan? (#38 - the fix is not released yet...) Or if they lack the shared lib, but the code execution never reaches that point, it will be fine? In Arch there isn't really a consensus on having optional dependencies for shared libraries...

wayland-client/use_system_lib is interesting too! That I should consider doing in Arch package too 🤔 Probably not in the pre-built binaries for Github release though, as the prebuilt binary would just stop working when a new version of Vulkan is released... But on the other hand IIRC the Rust impl was one other place which caused problems with musl... It was late and I confused Vulkan and Wayland, sorry 😵‍💫

That entire # Optimize binary for size. section you have, is it something you do for every single rust app? Is there something that makes sense to upstream, or it really is more of a packaging preference? There isn't much in Arch/Rust package guidelines but that wiki is also relatively new, I'm on a lookout to learn new interesting tricks we could all benefit from 😉

@maximbaz
Copy link
Owner

maximbaz commented Feb 7, 2022

@jirutka FYI ed074b0 should remove the need for some of your patches (one notable difference: dlopen feature for wayland, which allows using the app on non-wayland as long as you use capturer=none).

To whoever is interested, regarding the above comments:

  • Linking to shared libs requires those libs to be present on user's system, or the app will fail to launch if it doesn't even use the functionality
  • ash links to vulkan ICD loader, which is driver-independent, so you can install just this one even if your system doesn't support Vulkan, but skip the driver and wluma will work as long as you dont use features that rely on Vulkan (specifically, don't use capturer=wlroots).
  • wayland-client now uses dlopen feature, so it just tries to load Wayland dynamically, and only if you actually use Wayland-specific features (capturer=wlroots)
  • Optimization flags I left to the packagers, e.g. on Arch we also enable LTO but in a different way.

@jirutka
Copy link
Contributor Author

jirutka commented Feb 7, 2022

FYI: I read your comment and I will respond, I’m just too busy now.

@jirutka jirutka deleted the patch-1 branch February 10, 2022 15:20
@jirutka
Copy link
Contributor Author

jirutka commented Feb 10, 2022

Your package is a golden mine of interesting stuff!

I’m glad you found it interesting. :)

…it seems you managed to get it running with musl!

Yes, but that wasn’t hard, the only problem was with v4l crate.

I had some people asking about it, and my quick attempt failed. … I'd also love to be able to provide pre-compiled musl variant as a Github release

There’s a difference between building a rust project on Alpine (with system’s rustc) and cross-compiling it on a different distro with stock rustc. Our rustc is patched, mainly to link system libraries dynamically by default, i.e. the same what rustc do by default on glibc and others, but not for musl. It’s not a problem to build wluma with musl as a dynamically linked executable (at least when you do it on Alpine), but building it as a static executable would be tricky.

If you already know of some better alternatives, please don't hesitate to share 😉

There’s also https://github.com/Gnurou/v4l2r, but I don’t know if it’s better, I don’t have any experience with these libraries.

Using linked feature for ash is an interesting idea but won't this cause problems for people whose hardware doesn't support Vulkan?

I thought that Vulkan is a hard dependency for wluma. If it’s used just for capturer=wlroots, then it makes sense to keep it optional.
libvulkan.so is just a loader for “Installable Client Driver”, the actual Vulkan driver for particular hardware, so it’s not a problem. It’s not a small dependency (420 kiB on x86_64) though, so I’m gonna reconsider this.

wayland-client/use_system_lib is interesting too!

The main motivation for this was to reduce number of bundled dependencies and the size of the wluma package, not that it wouldn’t work on Alpine without it.

That entire # Optimize binary for size. section you have, is it something you do for every single rust app?

Also every rust app, yes. We’re about to define it globally so we don't have to override it in every aport.

Is there something that makes sense to upstream…

You can read about them on https://doc.rust-lang.org/cargo/reference/profiles.html and decide.

I’d certainly use lto = true and consider codegen-units = 1 – these produce smaller and more optimized binary at the cost of higher resource requirements for the build (compilation takes longer and consumes more memory).

panic = "abort" also decreases size of the resulting binary, but at the cost of losing function names in the stack trace on panic.

FYI ed074b0 should remove the need for some of your patches (one notable difference: dlopen feature for wayland, which allows using the app on non-wayland as long as you use capturer=none).

I thought that wayland is a hard dependency for wluma…? It’s right at the beginning of the README:

A tool for wlroots-based compositors that automatically adjusts screen brightness…

Linking to shared libs requires those libs to be present on user's system, or the app will fail to launch if it doesn't even use the functionality

Yes.

ash links to vulkan ICD loader, which is driver-independent, so you can install just this one even if your system doesn't support Vulkan, but skip the driver and wluma will work as long as you dont use features that rely on Vulkan (specifically, don't use capturer=wlroots).

True.

Optimization flags I left to the packagers, e.g. on Arch we also enable LTO but in a different way.

You probably use Linker-plugin-LTO, right? We’re considering it for Alpine as well.

@maximbaz
Copy link
Owner

maximbaz commented Feb 10, 2022

There’s a difference between building a rust project on Alpine (with system’s rustc) and cross-compiling it on a different distro with stock rustc ... building it as a static executable would be tricky.

Oh I see, thanks for the clarification, makes sense.

There’s also https://github.com/Gnurou/v4l2r, but I don’t know if it’s better, I don’t have any experience with these libraries.

Is there any way you can recommend for me to easily check if the library will give you problem, before we spend a lot of time researching how to adapt code to it?

I thought that Vulkan is a hard dependency for wluma.
I thought that wayland is a hard dependency for wluma…? It’s right at the beginning of the README:

The answer is the same for both: I didn't realize that there would be so many people actually using capturer=none, I thought reacting on screen contents is the "killer feature" without which nobody would use wluma (and screen capturing is so far implemented for wlroots and uses Vulkan). This is now proven to be wrong, especially when wluma gained support for external screens, apparently people are happy to simply rely on ALS (i.e. behave like our phones do), or even rely simulated ALS (i.e. using webcam or even on time of the day). Will update the README now, good catch!

You probably use Linker-plugin-LTO, right? We’re considering it for Alpine as well.

It seems this was the way to go but now that you pointed it out, I dont actually see how this is enabled, I will investigate, thanks for the link 👍

You can read about them on https://doc.rust-lang.org/cargo/reference/profiles.html and decide.
I’d certainly use lto = true and consider codegen-units = 1 – these produce smaller and more optimized binary at the cost of higher resource requirements for the build (compilation takes longer and consumes more memory).

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants