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

Fix error building without CMAKE_BUILD_TYPE being set #3590

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

ideasman42
Copy link
Contributor

This resolves the error building without a CMAKE_BUILD_TYPE.

CMake Error at CMakeLists.txt:36 (string):                                                                                                                                                          
  string no output variable specified

Describe your PR, what does it fix/add?

Resolve a build error with CMake.

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

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

It's ready.

This resolves the error building without a CMAKE_BUILD_TYPE.
CMake Error at CMakeLists.txt:36 (string):                                                                                                                                                          
  string no output variable specified
@q234rty
Copy link
Contributor

q234rty commented Oct 17, 2023

It would be better to also fix other valid CMAKE_BUILD_TYPE s as well, i.e. translate None to meson empty, RelWithDebInfo to meson debugoptimized, MinSizeRel to meson minsize .

@vaxerski
Copy link
Member

true that. Also, why is it _INIT?

@ideasman42
Copy link
Contributor Author

ideasman42 commented Oct 17, 2023

true that. Also, why is it _INIT?

This is used to set the initial build type, setting CMAKE_BUILD_TYPE would override the build type set that was already set, preventing other kinds of builds, "Debug" "RelWithDebInfo"... etc.

Strangely CMAKE_BUILD_TYPE_INIT is undocumented, but it is used. See:


If you wanted to avoid relying on an undocumented variable you could always do something like this:

if(NOT CMAKE_BUILD_TYPE)
  set(CMAKE_BUILD_TYPE "Release" CACHE STRING)
endif()

An alternative solution could be to support an empty CMAKE_BUILD_TYPE since CMake it's self allows this, e.g.

if(CMAKE_BUILD_TYPE)
    string(TOLOWER ${CMAKE_BUILD_TYPE} BUILDTYPE_LOWER)
else()
    set(BUILDTYPE_LOWER "release")
endif()

@vaxerski
Copy link
Member

vaxerski commented Oct 17, 2023

latter seems more reasonable (supporting empty)

Fix error when the CMAKE_BUILD_TYPE variable isn't set & properly convert the build type to mesons build type.
@ideasman42
Copy link
Contributor Author

Updated to convert the CMake build type to meson build types (with support for empty build type).

@q234rty
Copy link
Contributor

q234rty commented Oct 18, 2023

cmake None i.e. meson's empty seems to be missing

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@vaxerski vaxerski merged commit d994e6a into hyprwm:main Oct 19, 2023
10 checks passed
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