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 options to use the system wide wlroots library #552

Closed
wants to merge 1 commit into from

Conversation

epsilon-0
Copy link

Describe your PR, what does it fix/add?

a similar option is used in wayfire
this reduces the burden on recompilation of the whole
subproject when recompiling hyprland, and multiple
wayland compositors are installed

Signed-off-by: Aisha Tammy floss@bsd.ac

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

The default values for the new options makes it behave identical to how it is right now.

Is it ready for merging, or does it need work?

Should be ready to merge.

a similar option is used in wayfire
this reduces the burden on recompilation of the whole
subproject when recompiling hyprland, and multiple
wayland compositors are installed

Signed-off-by: Aisha Tammy <floss@bsd.ac>
@vaxerski
Copy link
Member

There is a reason #302 got closed as not planned.

This is a terrible idea. Why?

this reduces the burden on recompilation of the whole

wlroots takes like 10% of the compile time.

multiple wayland compositors are installed

Hyprland does not require, overwrite or conflict with any other wlroots installation.

@epsilon-0
Copy link
Author

It conflicts on Gentoo (which I am trying to add hyprland into) and also on Arch. This is evidenced by the last comment on the issue linked.

Also, I made the PR while keeping the original behaviour as the default one. I'm hoping that this just gives an option to others to make it easily packagable for other operating systems, while still trying to make it in line with your vision.

@viperML
Copy link
Contributor

viperML commented Aug 20, 2022

You can just patch the sources if you need to

https://github.com/hyprwm/Hyprland/blob/main/nix/meson-build.patch

Hyprland/nix/default.nix

Lines 74 to 77 in 9513031

patches = [
# make meson use the provided wlroots instead of the git submodule
./meson-build.patch
];

@vaxerski
Copy link
Member

vaxerski commented Aug 20, 2022

It conflicts on Gentoo (which I am trying to add hyprland into) and also on Arch. This is evidenced by the last comment on the issue linked.

The comment referenced a bug that was fixed months ago, it no longer does that.

Hyprland will build wlroots from the subproject and NOT install it. The only file it will install is libwlroots.so.11032 which will not conflict with any wlroots installation, and if built with meson, it's statically linked, so no .so is ever installed.

Also, I made the PR while keeping the original behaviour as the default one. I'm hoping that this just gives an option to others to make it easily packagable for other operating systems, while still trying to make it in line with your vision.

The solution you're proposing is inane though - Hyprland will NOT build with any wlroots older than ~ 10-20 commits (aka. any tagged release). By keeping it a subproject, we eliminate conflicts. To build Hyprland with the system wlroots, they'd have to match the locked commit - which would INTRODUCE and not eliminate conflicts.

The only thing your option would introduce is a wave of people coming to the issues saying "oMg I have wlroots 0.15.1 and no buildy why :( :( :("

If you are thinking I am a dickhead and lying, see the wlroots 0.16.0 meta and divide the amount of bullet points by the amount of days since the last tag, about 210, and you get a breaking change every other week.

@epsilon-0
Copy link
Author

You can just patch the sources if you need to

https://github.com/hyprwm/Hyprland/blob/main/nix/meson-build.patch

Hyprland/nix/default.nix

Lines 74 to 77 in 9513031

patches = [
# make meson use the provided wlroots instead of the git submodule
./meson-build.patch
];

Yes, this is what I am doing currently for the gentoo package 😸

The only thing your option would introduce is a wave of people coming to the issues saying "oMg I have wlroots 0.15.1 and no buildy why :( :( :("

If you are thinking I am a dickhead and lying, see the wlroots 0.16.0 meta and divide the amount of bullet points by the amount of days since the last tag, about 210, and you get a breaking change every other week.

Got it. I'll just keep the patch locally for gentoo for now. Maybe when wlroots is stabler with its API this can be done :)

@vaxerski
Copy link
Member

vaxerski commented Aug 20, 2022

If it works for gentoo, feel free, but I will not keep it in the repo here.

Every distro is free to patch the meson / cmake / whatever to their needs. With the parent repo, however, I try to keep things as versatile and safe as possible.

Furthermore, tagged hyprland releases exist, so I mean you can always use those.

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.

None yet

3 participants