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

Improvements to CMakeLists #2313

Closed
wants to merge 15 commits into from
Closed

Improvements to CMakeLists #2313

wants to merge 15 commits into from

Conversation

m00nwtchr
Copy link

@m00nwtchr m00nwtchr commented May 16, 2023

This PR brings (as far as i can tell) all of the functionality of the Makefile into CMake, which makes it more automated and friendly to package maintainers, and it follows the typical CMake project convention.

Example build:

$ git clone <Hyprland>
$ cmake -B build -S Hyprland
$ cmake --build build

If desired, can be installed with cmake --install build

It should be ready to go as is.
Nvm, i found one issue

@m00nwtchr
Copy link
Author

m00nwtchr commented May 16, 2023

So, the issue is that while this works fine on its own, for some reason under Arch's package build system I get a build error: make[2]: *** No rule to make target 'wlroots/lib/libwlroots.so.12032', needed by 'Hyprland'. Stop.

Edit: Looks like its caused by the -j option in MAKEFLAGS.

@m00nwtchr
Copy link
Author

Ok, that fixed it

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@vaxerski
Copy link
Member

vaxerski commented May 25, 2023

Would be nice if it actually succeeded through the pipelines. If you do make clear and then make release it fails because ninja can't find libwlroots, unsure why

@vaxerski
Copy link
Member

Reverting my commits will not get you anywhere as this will not be merged without .pc installing to /usr/share.

@m00nwtchr
Copy link
Author

The Makefile will install to /usr/share now

cleaninstall:
echo -en "$(MAKE) cleaninstall has been DEPRECATED, you should avoid using it in the future.\nRunning $(MAKE) install instead...\n"
$(MAKE) install
cmake --install build --prefix "/usr"
Copy link
Member

Choose a reason for hiding this comment

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

fun of you to disallow commits from me on my own project.

Hyprland installs to /usr/local, .pc to /usr.

cmake --install build
if [ -d /usr/share/pkgconfig ]; then cp ./build/hyprland.pc /usr/share/pkgconfig 2>/dev/null || true; fi

if anything.

@vaxerski vaxerski force-pushed the main branch 2 times, most recently from 09bd49b to ce9c5fd Compare July 18, 2023 22:28
Copy link
Contributor

@stephan-cr stephan-cr left a comment

Choose a reason for hiding this comment

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

@m00nwtchr I had the same idea and then I found this PR. :-) Left some comments and hope they're helpful.

"protocols/")
set(WLROOTS_SOVERSION 12032)

find_program(MESON_EXE NAMES meson)
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be checked if find_program actually finds something. Since CMake 3.18 you can say:

Suggested change
find_program(MESON_EXE NAMES meson)
find_program(MESON_EXE NAMES meson REQUIRED)

https://cmake.org/cmake/help/latest/command/find_program.html

)

install(TARGETS Hyprland hyprctl DESTINATION bin)
Copy link
Contributor

@stephan-cr stephan-cr Jul 29, 2023

Choose a reason for hiding this comment

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

I wonder if it would make sense to include GNUInstallDirs and doing

install(TARGETS Hyprland hyprctl DESTINATION ${CMAKE_INSTALL_BINDIR})

instead?

ExternalProject_Add(wlroots_build
PREFIX wlroots
SOURCE_DIR ${CMAKE_SOURCE_DIR}/subprojects/wlroots
PATCH_COMMAND ${SED_EXE} -E -i -e "s/(soversion = 12)([^032]|$$)/soversion = ${WLROOTS_SOVERSION}/g" ${CMAKE_SOURCE_DIR}/subprojects/wlroots/meson.build
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the Sed script could be simplified to:

PATCH_COMMAND ${SED_EXE} -E -i -e "/^soversion = /s/12([^032]|$$)/${WLROOTS_SOVERSION}/" ${CMAKE_SOURCE_DIR}/subprojects/wlroots/meson.build

Copy link
Contributor

@stephan-cr stephan-cr left a comment

Choose a reason for hiding this comment

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

I think I know why the build fails currently.

INSTALL_COMMAND ${MESON_EXE} install --destdir ${CMAKE_CURRENT_BINARY_DIR}/wlroots
BUILD_BYPRODUCTS ${CMAKE_CURRENT_BINARY_DIR}/wlroots/lib/libwlroots.so.${WLROOTS_SOVERSION})

set(wlroots_INCLUDE_DIR ${CMAKE_SOURCE_DIR}/subprojects/wlroots/include/)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it should be something like:

ExternalProject_Get_property(wlroots_build INSTALL_DIR)
set(wlroots_INCLUDE_DIR ${INSTALL_DIR}/include/)

I think that is reason why the build currently fails. config.h is a generated file and is either in the build directory or in the final "installation" directory.

CONFIGURE_COMMAND ${MESON_EXE} setup --prefix "/" . ${CMAKE_SOURCE_DIR}/subprojects/wlroots
BUILD_COMMAND ${NINJA_EXE} -C .
INSTALL_COMMAND ${MESON_EXE} install --destdir ${CMAKE_CURRENT_BINARY_DIR}/wlroots
BUILD_BYPRODUCTS ${CMAKE_CURRENT_BINARY_DIR}/wlroots/lib/libwlroots.so.${WLROOTS_SOVERSION})
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: wrong indention?

@@ -153,35 +172,53 @@ include(CPack)
message(STATUS "Setting link libraries")

function(protocol protoPath protoName external)
if (external)
if(external)
execute_process(

Choose a reason for hiding this comment

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

using add_custom_command() here to generate these files into CMAKE_CURRENT_BINARY_DIR would be nice. this way we could add them to an interface library target that any dependents can target_link_libraries() to.

@m00nwtchr
Copy link
Author

Note to anyone still looking at this: I gave up on having this upstreamed due to the above conversations. But if you still want to continue this you're welcome to fork my fork.

@majestrate
Copy link

Note to anyone still looking at this: I gave up on having this upstreamed due to the above conversations. But if you still want to continue this you're welcome to fork my fork.

i think the underlying pain point comes from the lack of separation in the dev branches and any packaging fixups. when packaging logic gets woven so deeply into the main development branch 100% of the time package maintainers have their fixups for their own distros mucking up things where it does not have to.
debian has spinoff branches for packages for this very reason, instead of mucking about in the dev process of everything they package they periodically rebase spinoff branches for applying small commits for JUST packaging. packaging is a full time job and making the life of the package maintainer is just as important as the people working on the bulk of the meat. when package maintainers and core software developers have to care about divergences from any distro getting in their way something is wrong with how packaging is done. if hyprland could do something more manageable like spinoff branches for nix flakes, RPMs, ebuilds, debs or whatever ever packaging is required i think that this problem would go away and everyone ends up happy as a result.
i would personally like to feel out how people in hyprland feel about that as i think that everyone who wants to contribute but doesnt is influenced by the build system being so convoluted by being intertwined with packaging right now.

@m00nwtchr
Copy link
Author

m00nwtchr commented Sep 29, 2023

Note to anyone still looking at this: I gave up on having this upstreamed due to the above conversations. But if you still want to continue this you're welcome to fork my fork.

i think the underlying pain point comes from the lack of separation in the dev branches and any packaging fixups. when packaging logic gets woven so deeply into the main development branch 100% of the time package maintainers have their fixups for their own distros mucking up things where it does not have to.
debian has spinoff branches for packages for this very reason, instead of mucking about in the dev process of everything they package they periodically rebase spinoff branches for applying small commits for JUST packaging. packaging is a full time job and making the life of the package maintainer is just as important as the people working on the bulk of the meat. when package maintainers and core software developers have to care about divergences from any distro getting in their way something is wrong with how packaging is done. if hyprland could do something more manageable like spinoff branches for nix flakes, RPMs, ebuilds, debs or whatever ever packaging is required i think that this problem would go away and everyone ends up happy as a result.
i would personally like to feel out how people in hyprland feel about that as i think that everyone who wants to contribute but doesnt is influenced by the build system being so convoluted by being intertwined with packaging right now.

That's what I tried to do, the upstream shouldn't care about the target distribution at all, and it should be left to the packagers, but they insisted on supporting distributions when installing to system with cmake, which is just moronic (and makes downstream packaging more difficult)

@majestrate
Copy link

Note to anyone still looking at this: I gave up on having this upstreamed due to the above conversations. But if you still want to continue this you're welcome to fork my fork.

i think the underlying pain point comes from the lack of separation in the dev branches and any packaging fixups. when packaging logic gets woven so deeply into the main development branch 100% of the time package maintainers have their fixups for their own distros mucking up things where it does not have to.
debian has spinoff branches for packages for this very reason, instead of mucking about in the dev process of everything they package they periodically rebase spinoff branches for applying small commits for JUST packaging. packaging is a full time job and making the life of the package maintainer is just as important as the people working on the bulk of the meat. when package maintainers and core software developers have to care about divergences from any distro getting in their way something is wrong with how packaging is done. if hyprland could do something more manageable like spinoff branches for nix flakes, RPMs, ebuilds, debs or whatever ever packaging is required i think that this problem would go away and everyone ends up happy as a result.
i would personally like to feel out how people in hyprland feel about that as i think that everyone who wants to contribute but doesnt is influenced by the build system being so convoluted by being intertwined with packaging right now.

That's what I tried to do, the upstream shouldn't care about the target distribution at all, and it should be left to the packagers, but they insisted on supporting distributions when installing to system with cmake, which is just moronic (and makes downstream packaging more difficult)

i personally would like to reduce the friction on this somehow so hyprland can eventually be packaged in debian.

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.

4 participants