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

meson: Correct capability check for SunRPC on *BSD #1768

Merged
merged 1 commit into from
Nov 24, 2024

Conversation

rdmark
Copy link
Member

@rdmark rdmark commented Nov 21, 2024

For the SunRPC / libquota check, look for quota_open() instead of the obsolete getfsquota()

Don't compile the bundled nfsquota module when using libquota.

@rdmark rdmark linked an issue Nov 21, 2024 that may be closed by this pull request
@rdmark rdmark force-pushed the 1225-netbsd-using-compatibility-layer-for-quota branch 3 times, most recently from b161247 to cb02a80 Compare November 22, 2024 06:48
@hfath
Copy link

hfath commented Nov 22, 2024

Two comments:

I wonder if there should be a have_libquota meson selector, capturing support for the NetBSD libquota(3) / FreeBSD quotafile(3) API, instead of conditionalizing on the OS.

The quota.c #ifdef __NetBSD__ should not be necessary, because that code is #ifdef'ed out with libquota support.

@rdmark
Copy link
Member Author

rdmark commented Nov 22, 2024

Two comments:

I wonder if there should be a have_libquota meson selector, capturing support for the NetBSD libquota(3) / FreeBSD quotafile(3) API, instead of conditionalizing on the OS.

Easily done! I actually had that in an earlier revision, if you look at the force push diff.

The way you do this in Meson, is to define a boolean variable somewhere in the global scope, i.e. have_libquota = false, then set the flag to true in the branch that check for that dependency.

It's recommended to define the flag early and in the global scope first to avoid errors for certain branches.

The quota.c #ifdef __NetBSD__ should not be necessary, because that code is #ifdef'ed out with libquota support.

I think we need a new capability check, either to replace or supplement the current libquota check. The current libquota check is false positive for NetBSD. Do you have an idea?

@rdmark rdmark marked this pull request as ready for review November 23, 2024 21:32
@rdmark
Copy link
Member Author

rdmark commented Nov 23, 2024

@hfath Scratch the above. You were right.

It builds cleanly on all OSes now.

@rdmark rdmark force-pushed the 1225-netbsd-using-compatibility-layer-for-quota branch from ccbdbdf to e9b6ea1 Compare November 23, 2024 21:45
@rdmark rdmark changed the title meson: Check for SunRPC function quota_open() meson: Correct capability check for SunRPC on *BSD Nov 23, 2024
@rdmark rdmark merged commit 4d75489 into main Nov 24, 2024
29 checks passed
@rdmark rdmark deleted the 1225-netbsd-using-compatibility-layer-for-quota branch November 24, 2024 12:35
@hfath
Copy link

hfath commented Nov 25, 2024

I have a related patch which

  1. checks for libquota first - should work both on Net- and FreeBSD
  2. sets quota_provider accordingly (is this functional, or mainly eye candy?)

-- builds and runs here. I still need to set up a test bed for the actual quota functionality, though, both local and on nfs volumes.

@hfath
Copy link

hfath commented Nov 25, 2024

meson_build.txt

(github's import filter sucks boulders through a straw...)

@rdmark
Copy link
Member Author

rdmark commented Nov 25, 2024

Thanks for the patch! Do you have any interest whatsoever to file a GitHub PR for this?

A PR would give you proper credits in the commit log and the next release notes. :)

@rdmark
Copy link
Member Author

rdmark commented Nov 25, 2024

I have a related patch which

1. checks for libquota first - should work both on Net- and FreeBSD

2. sets quota_provider accordingly (is this functional, or mainly eye candy?)

Eye candy primarily, with the added benefit of informing ignorant netatalk maintainers which library was detected on which OS.

@hfath
Copy link

hfath commented Nov 25, 2024

@pr -- I should get some practice, I guess... give me a couple of days, too many balls in the air right now

@rdmark
Copy link
Member Author

rdmark commented Nov 30, 2024

BTW, you can define a code block delineated by a pair of three backticks like this:

$NetBSD$

--- meson.build.orig	2024-11-24 17:34:19.000000000 +0000
+++ meson.build
@@ -1012,40 +1012,43 @@ endif
 if not enable_quota
     have_quota = false
     cdata.set('NO_QUOTA_SUPPORT', 1)
-else
-    have_quota = tirpc.found() and cc.has_header('rpcsvc/rquota.h')
-    if have_quota
-        cdata.set('NEED_RQUOTA', 1)
-        quota_deps += tirpc
-        quota_provider += 'libtirpc'
-        if cc.compiles(
-            '''
-                    #include <rpcsvc/rquota.h>
-                    int main(void) {
-                        enum qr_status foo;
-                        foo = Q_OK;
-                        return 0;
-                    }
-                ''',
-            dependencies: tirpc,
+elif quota.found() and cc.has_function('quota_open')
+    # FreeBSD provides libquota API in libc
+    have_libquota = true
+    have_quota = true
+    quota_provider += 'libquota'
+    cdata.set('HAVE_LIBQUOTA', 1)
+elif quota.found() and cc.has_function('quota_open', dependencies: [quota, prop])
+    have_libquota = true
+    have_quota = true
+    quota_provider += 'libquota'
+    quota_deps += [quota, prop]
+    cdata.set('HAVE_LIBQUOTA', 1)
+elif tirpc.found() and cc.has_header('rpcsvc/rquota.h')
+    cdata.set('NEED_RQUOTA', 1)
+    quota_deps += tirpc
+    quota_provider += 'libtirpc'
+    if cc.compiles(
+        '''
+        #include <rpcsvc/rquota.h>
+        int main(void) {
+        enum qr_status foo;
+        foo = Q_OK;
+        return 0;
+        }
+        ''',
+        dependencies: tirpc,
         )
-            cdata.set('HAVE_RQUOTA_H_QR_STATUS', 1)
-        endif
-    else
-        have_quota = rpcsvc.found() and rpc_headers_ok
-        if have_quota
-            quota_deps += rpcsvc
-            quota_provider += 'SunRPC'
-            if quota.found() and cc.has_function('quota_open', dependencies: [quota, prop, rpcsvc])
-                have_libquota = true
-                quota_deps += [quota, prop]
-                cdata.set('HAVE_LIBQUOTA', 1)
-            endif
-        else
-            have_quota = false
-            cdata.set('NO_QUOTA_SUPPORT', 1)
-        endif
+        cdata.set('HAVE_RQUOTA_H_QR_STATUS', 1)
     endif
+    have_quota = true
+elif rpcsvc.found() and rpc_headers_ok
+    quota_deps += rpcsvc
+    quota_provider += 'SunRPC'
+    have_quota = true
+else
+    have_quota = false
+    cdata.set('NO_QUOTA_SUPPORT', 1)
 endif
 
 if enable_quota and not have_quota

Just in case you weren’t aware. It’s a handy way to share code snippets or patches.

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.

NetBSD using compatibility layer for quota
2 participants