Bug 9881 - Samba doesn't check for system libtevent
Summary: Samba doesn't check for system libtevent
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.6
Classification: Unclassified
Component: Build environment (show other bugs)
Version: 3.6.15
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-14 08:24 UTC by Andreas Schneider
Modified: 2018-04-27 21:21 UTC (History)
3 users (show)

See Also:

Attachments
v3-6-test patch (30.11 KB, patch)
2013-05-23 09:13 UTC, Andreas Schneider 		no flags Details
v4-0-test patch (28.12 KB, patch)
2013-05-23 10:01 UTC, Andreas Schneider 		obnox: review+
ddiss: review+
Details
v3-6-test patch (31.05 KB, patch)
2013-05-23 10:07 UTC, Andreas Schneider 		obnox: review+
ddiss: review+
bjacke: review-
Details
additional v3-6-test patch (1.22 KB, patch)
2013-06-17 09:54 UTC, Andreas Schneider 		kseeger: review+
obnox: review+
Details
additional v3-6-test (1.23 KB, patch)
2013-06-18 09:50 UTC, Karolin Seeger 		asn: review+
obnox: review+
Details
additional v3-6-test patch (1.30 KB, patch)
2013-06-18 09:51 UTC, Karolin Seeger 		asn: review+
obnox: review+
Details
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Schneider 2013-05-14 08:24:00 UTC 
Samba always builds and links against the in-tree version of libtevent. It should check for the system version of libtevent and use it if detected. The following patch checks the for the system library and uses it if detected. You can force it to check for the external version or only use the internal version.
Comment 1 Andreas Schneider 2013-05-23 09:13:59 UTC 
Created attachment 8909 [details]
v3-6-test patch
Comment 2 Andreas Schneider 2013-05-23 10:01:44 UTC 
Created attachment 8910 [details]
v4-0-test patch
Comment 3 Andreas Schneider 2013-05-23 10:07:21 UTC 
Created attachment 8911 [details]
v3-6-test patch
Comment 4 David Disseldorp 2013-06-06 10:18:59 UTC 
Comment on attachment 8910 [details]
v4-0-test patch

looks good.
Comment 5 David Disseldorp 2013-06-06 10:20:31 UTC 
Karolin, please queue for 4.0.next and 3.6.next.
Comment 6 Karolin Seeger 2013-06-06 18:53:43 UTC 
Pushed to v3-6-test and autobuild-v4-0-test.
Comment 7 Karolin Seeger 2013-06-10 19:21:15 UTC 
Pushed to v4-0-test.
Closing out bug report.

Thanks!
Comment 8 Michael Adam 2013-06-13 14:19:31 UTC 
Comment on attachment 8911 [details]
v3-6-test patch

sorry, late ack
Comment 9 Michael Adam 2013-06-13 14:19:45 UTC 
Comment on attachment 8910 [details]
v4-0-test patch

sorry, late ack...
Comment 10 Björn Jacke 2013-06-17 09:43:17 UTC 
it seem like the patch throws out the static libs from the build process. this way only builds with shared tevent libs work.

we build withot shared libtevent libs:

        --with-libtalloc=no \
        --with-libtdb=no \
        --with-libnetapi=no \
        --enable-shared-libs=no \
        --enable-static=yes


and thus get this error with this patch applied:


Linking shared library bin/libsmbclient.so.0
utils/dbwrap_tool.o(.text+0x5d9): In function `main':
utils/dbwrap_tool.c:295: undefined reference to `tevent_context_init'
../lib/util/tevent_unix.o(.text+0x23): In function
`tevent_req_is_unix_error':
../lib/util/tevent_unix.c:32: undefined reference to `tevent_req_is_error'
../lib/util/tevent_ntstatus.o(.text+0x35): In function
`_tevent_req_nterror':
../lib/util/tevent_ntstatus.c:45: undefined reference to
`_tevent_req_error'


So currently we should revert this patch for 3.6 as this would be a regression.
Comment 11 Björn Jacke 2013-06-17 09:44:40 UTC 
Comment on attachment 8911 [details]
v3-6-test patch

as mentioned above: this patch is not okay
Comment 12 Andreas Schneider 2013-06-17 09:54:47 UTC 
Created attachment 8976 [details]
additional v3-6-test patch
Comment 13 Andreas Schneider 2013-06-17 10:03:31 UTC 
asn@magrathea:~/workspace/projects/samba/master/source3> make bin/smbiconv
Compiling torture/smbiconv.c
torture/smbiconv.c: In function ‘process_block’:
torture/smbiconv.c:36:10: warning: variable ‘n’ set but not used [-Wunused-but-set-variable]
Linking bin/smbiconv
Comment 14 Karolin Seeger 2013-06-18 09:50:30 UTC 
Created attachment 8978 [details]
additional v3-6-test

Additional patch needed for v3-6-test
Comment 15 Karolin Seeger 2013-06-18 09:51:47 UTC 
Created attachment 8979 [details]
additional v3-6-test patch

Additional patch needed for v3-6-test.
Comment 16 Karolin Seeger 2013-06-18 09:54:21 UTC 
(In reply to comment #11)
> Comment on attachment 8911 [details]
> v3-6-test patch
> 
> as mentioned above: this patch is not okay

Björn, after adding the three additional patches and using --with-libtevent=no,
everything builds. So I thinnk, the patch is fine. Do you agree?
Comment 17 Karolin Seeger 2013-06-18 09:56:18 UTC 
My question is, shall I

a) revert the first patch and ship 3.6.16 without it or
b) add the three last minute patches for the release tomorow.

Comments?
Comment 18 Karolin Seeger 2013-06-18 09:59:02 UTC 
Need to be checked for 4.0.
Comment 19 Karolin Seeger 2013-06-18 10:05:39 UTC 
(In reply to comment #18)
> Need to be checked for 4.0.

Patches do not apply to v4-0-test and LIBTEVENT has already been added for dbwrap_torture and dbwrap_tool, smbiconv does not exit.
So I think it's not an issue here.
Comment 20 Andreas Schneider 2013-06-18 14:32:40 UTC 
If it compiles with the patches, please ship with them. If not then add them to the next version.

It isn't really that important right now.
Comment 21 Michael Adam 2013-06-18 14:54:24 UTC 
Together with the three additional patches, I think the patchset is good for shipping.
Comment 22 Karolin Seeger 2013-06-19 08:28:02 UTC 
Pushed the 3 additional patches to v3-6-test.
Will be included in Samba 3.6.16.
Closing out bug report.

Thanks!