Skip to content

meson: Add build-lsfd option and make rt dependency optional #2879

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

meson: Add build-lsfd option and make rt dependency optional #2879

wants to merge 2 commits into from

Conversation

jwillikers
Copy link
Contributor

@jwillikers jwillikers commented Mar 29, 2024 •
edited
Loading

A dependency on the rt library is unnecessarily required when checking for the clock_gettime and timer_create functions.
This causes the build to fail if the rt library is not found. This should not fail the build as rt is only required for the checks. Additionally, the lsfd executable and some tests require rt. There is currently no option to toggle building lsfd.

This PR makes it possible to build without the rt library. Function checks no longer require rt for the build. The function checks for the rt library only run when rt is available. This PR adds an option to allow building without lsfd. This makes it possible to build without the executable that requires rt. To not require rt for the test, an additional check has been added. The effected tests won't be built unless rt has been found.

Fixes #2878 and possibly #2874

@jwillikers jwillikers mentioned this pull request Mar 29, 2024
Closed
@karelzak
Copy link
Collaborator

karelzak commented Apr 1, 2024

@t-8ch can you review, please?

t-8ch
t-8ch reviewed Apr 1, 2024
View reviewed changes
meson.build Outdated
@@ -2711,19 +2717,20 @@ errnos_h = custom_target('errnos.h',
command : ['tools/all_errnos', cc.cmd_array()]
)

mq_libs = []
mq_libs += cc.find_library('rt', required : true)
lib_rt = cc.find_library('rt', required : get_option('build-lsfd'))
Copy link
Member

Choose a reason for hiding this comment

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

This fails if librt is not present but also not needed. (Same as in current master)
Could you change this to reuse the results from the checks above?

Copy link
Contributor Author

@jwillikers jwillikers Apr 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Because it doesn't check for it at all because it is using an option here. Got it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I have the cc.find_library call happen in one place now instead of three. I used a require call on the build-lsfd feature option to require the rt library be found.

@jwillikers
meson: Add build-lsfd option and make rt dependency optional
06b2705 
A dependency on the rt library is unnecessarily required when checking
for the clock_gettime and timer_create functions.
This causes the build to fail if the rt library is not found.
This should not fail the build as rt is only required for the checks.
Additionally, the lsfd executable and some tests require rt.
There is currently no option to toggle building lsfd.

This PR makes it possible to build without the rt library.
Function checks no longer require rt for the build.
The function checks for the rt library only run when rt is available.
This PR adds an option to allow building without lsfd.
This makes it possible to build without the executable that requires rt.
To not require rt for the test, a additional check has been added.
The effected tests won't be built unless rt has been found.

Signed-off-by: Jordan Williams <jordan@jwillikers.com>
@jwillikers
meson: Only pick up the rt library once
2439502 
Require the rt library for the build-lsfd feature.

Signed-off-by: Jordan Williams <jordan@jwillikers.com>
@karelzak karelzak added the MESON meson related label Apr 2, 2024
@jwillikers jwillikers requested a review from t-8ch April 2, 2024 12:44
@karelzak
Copy link
Collaborator

karelzak commented Apr 5, 2024

@t-8ch I guess this one is ready to merge. Right?

@karelzak
Copy link
Collaborator

karelzak commented Apr 5, 2024

Merged, not sure why github see itr as unmerged,

$ git pull --log https://github.com/jwillikers/util-linux.git meson-optional-rt
From https://github.com/jwillikers/util-linux
 * branch                meson-optional-rt -> FETCH_HEAD
Auto-merging meson.build
Auto-merging meson_options.txt
Merge made by the 'ort' strategy.
 meson.build       | 32 ++++++++++++++++++--------------
 meson_options.txt |  2 ++
 2 files changed, 20 insertions(+), 14 deletions(-)

@karelzak karelzak closed this Apr 5, 2024
@t-8ch
Copy link
Member

t-8ch commented Apr 5, 2024

not sure why github see itr as unmerged

It seems to be broken: https://www.githubstatus.com/

@jwillikers
Copy link
Contributor Author

Interesting. Thanks for the reviews!

@jwillikers jwillikers deleted the meson-optional-rt branch April 8, 2024 12:59
@jwillikers
Copy link
Contributor Author

@karelzak Would it be possible to backport this commit to the stable v2.40 branch?

@jwillikers jwillikers mentioned this pull request Apr 8, 2024
Closed
@karelzak
Copy link
Collaborator

karelzak commented Apr 9, 2024

@karelzak Would it be possible to backport this commit to the stable v2.40 branch?

We can backport more meson-related things. It would be ideal to create a separate pull request and cherry-pick all important meson things for stable/v2.40. For .1 releases, I'm usually very open to changes :-)

@jwillikers
Copy link
Contributor Author

@karelzak Would it be possible to backport this commit to the stable v2.40 branch?

We can backport more meson-related things. It would be ideal to create a separate pull request and cherry-pick all important meson things for stable/v2.40. For .1 releases, I'm usually very open to changes :-)

Got it, thanks!

@jwillikers jwillikers mentioned this pull request Apr 9, 2024
Merged
jwillikers added a commit to jwillikers/util-linux that referenced this pull request Apr 16, 2024
@jwillikers
meson: Remove lingering mq_libs variable
550a957 
The cherry-pick of util-linux#2879 in PR util-linux#2941 to the stable/v2.40 branch didn't
include the removal of mq_libs like it should have.
I must have resolved a conflict incorrectly in the cherry-pick.
This just removes the lingering definitions which are no longer used.
The extra find_library call for rt is a problem.

Signed-off-by: Jordan Williams <jordan@jwillikers.com>
@jwillikers jwillikers mentioned this pull request Apr 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-8ch t-8ch t-8ch approved these changes

Assignees
No one assigned
Labels
MESON meson related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

meson: Do not require the rt library when checking for a function's existence
3 participants
@jwillikers @karelzak @t-8ch