Bug 4543 - Broken POSIX ACL detection in 3.0.25rc2
Broken POSIX ACL detection in 3.0.25rc2
Status: RESOLVED FIXED
Product: Samba 3.0
Classification: Unclassified
Component: Build environment
3.0.25
Other FreeBSD
: P3 normal
: none
Assigned To: Michael Adam
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-24 19:10 UTC by Timur Bakeyev
Modified: 2007-08-22 05:07 UTC (History)
0 users

See Also:


Attachments
Fixes for the POSIX ACL detection and more consistend diagnostic output (4.83 KB, patch)
2007-04-24 19:12 UTC, Timur Bakeyev
no flags Details
Dirty FreeBSD hack to make things work. (640 bytes, patch)
2007-04-24 19:18 UTC, Timur Bakeyev
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Timur Bakeyev 2007-04-24 19:10:14 UTC
I've tried to compile latest RC2 under FreeBSD 6.2 and failed to get POSIX ACL working(actually - compiled in).

After looking into configure.in I realised that in the FreeBSD branch of the ACL checks there is an omission of adding vfs_posixacl module to the list of static modules.

At that point I came to the idea that FreeBSD, starting from version 5 implements full set of POSIX ACL API, so there is no actual need to have a specific OS dependant check for it and generic POSIX ACL check should be enough. After removing FreeBSD specific check I got configure to use generic POSIX ACL check section, but it still didn't work due hardcoded inclusion of libacl in it. And FreeBSD has all ACL API implemented in libc. Seems, this code never was actually checked before. So, with some aditional adjustments I got it to work and giving nice and verbose diagnostics during confugure checks.

Patch is attached to the report.
Comment 1 Timur Bakeyev 2007-04-24 19:12:17 UTC
Created attachment 2408 [details]
Fixes for the POSIX ACL detection and more consistend diagnostic output
Comment 2 Timur Bakeyev 2007-04-24 19:17:45 UTC
There is one more problem with POSIX ACLs under FreeBSD vs. Linux implementation.

FreeBSD implements acl_get_perm_np() call, rather than acl_get_perm() in the Linux. I'm not sure, which one is more POSIX compliant, but amasingly enough there is a check in configure for the NP variant of the function. The only problem with the code in RC2 is that defined macro isn't used anywhere and acl_get_perm() called unconditionly.

I attach the patch from the FreeBSD Samba3 port for this matter, but obviously it have to be implemented in a portable way, either through the wrapper function or #ifdef's.
Comment 3 Timur Bakeyev 2007-04-24 19:18:26 UTC
Created attachment 2409 [details]
Dirty FreeBSD hack to make things work.
Comment 4 Timur Bakeyev 2007-04-24 19:21:42 UTC
Yet another note on ACLs detection. Linux check and generic POSIX one share 99% of code and the only difference between them is additional inclusion of libattr to the list of ACL_LIBS. On other side, FreeBSD doesn't need any libs, as everything resides in libc. By generalizing this checks it's possible to have one POSIX ACL compliant systems check, which would be easier to maintain.

With best regards,
Timur.
Comment 5 Michael Adam 2007-04-25 03:39:23 UTC
Hi Timur, thanks for testing and reporting.
I will check things once I have found time to
set up a FreeBSD (vmware) machine. 
Comment 6 Timur Bakeyev 2007-05-09 19:57:56 UTC
Hi, Michael!

(In reply to comment #5)
> Hi Timur, thanks for testing and reporting.
> I will check things once I have found time to
> set up a FreeBSD (vmware) machine. 

It'll be nice to get that into the release, as otherwise it is broken for FreeBSD entirely. What do you need exactly to test?

Almost all the changes are in the configure only and I can ensure you on 99% that they are right for FreeBSD:)

The real question is to test them against Linux and come with portable check, that would work for both(and maybe more) platforms.

If you need any help with FreeBSD - feel free to ask.

With best regards,
Timur
Comment 7 Michael Adam 2007-05-10 03:37:07 UTC
Hi Timur,

(In reply to comment #6)
> It'll be nice to get that into the release, as otherwise it is broken for
> FreeBSD entirely. What do you need exactly to test?

Honestly, time is what I need... :-)

I managed to set up a freebsd machine so far.

I hope to find some time for testing and possibly adapting your patch 
in the next couple of days!

> Almost all the changes are in the configure only and I can ensure you on 99%
> that they are right for FreeBSD:)
> 
> The real question is to test them against Linux and come with portable check,
> that would work for both(and maybe more) platforms.
> 
> If you need any help with FreeBSD - feel free to ask.

Ok thanks, I will gladly come back to that if necessary.

Best, Michael
Comment 8 Michael Adam 2007-05-10 08:36:11 UTC
Timur,

i just committed a quick fix in r22777.
Please check!

The unification of the configure checks is not in there.
But it vfs_posixacl is now built and statically linked 
on FreeBSD and use is made of the HAVE_ACL_GET_PERM_NP
macro. I built and successfully tested acl support on 
FreeBSD 6.2.

Michael
Comment 9 Michael Adam 2007-06-08 15:46:22 UTC
Hi Timur,

I have made a modification to the acl detection code in r23387 and r23389, 
similar to the one you suggested in comment #1. Bugzilla was not available,
but I think I have come up with almost the same patch as yours. :-)
It is in fact the natural thing to do.

Furthermore, I have switched autodetection of ACL support on.

I'll wait for a couple of more hosts in the build farm and then
merge this to 3_0_26. 

Tim: Could you assign the bug to me, please?
So that I can change the status...

Cheers, Michael
Comment 10 Michael Adam 2007-08-22 05:07:13 UTC
Thanks Günther, for assigning the bug to me... :-)

Timur: Since I have not heard from you anymore about this, I am marking the bug as fixed..

...Michael