BIND 10 #1292: Xfrin retransfer shared object "sqlite3_ds.so" not found

[BIND 10 Development]

#1292: Xfrin retransfer shared object "sqlite3_ds.so" not found
-------------------------------------+-------------------------------------
                   Reporter:  jreed  |                 Owner:  jinmei
                       Type:         |                Status:  reviewing
  defect                             |             Milestone:
                   Priority:         |  Sprint-20111206
  critical                           |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  4
            Defect Severity:  High   |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  5      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Thanks for looking at it on such short notice :)

 Replying to [comment:21 jinmei]:
 > Replying to [comment:20 jelte]:
 >
 > First, this point:
 >
 > > BTW, I have not checked, but if this works, i suspect it solves
 several other related tickets as well (#1385 and #1421, maybe more)
 >
 > For #1421, probably.  I suspect #1385 is a separate issue.
 >

 he hasn't answered my question there yet, but i have seen this problem
 too; when i had an older installed version that wasn't
 binary-compatible with recent changes. Exact same symptoms, caused by
 dlopen picking the wrong one. So if that is the case for him too, the
 root cause is of course different (we need version check), but the
 system should fall in that trap a lot less (theoretically, never,
 even, unless you start mixing versions).

 > About the branch:
 >
 > '''general'''
 >
 > I'm not very happy about introducing yet another hardcode of things
 > like B10_FROM_BUILD in our main source code.  Is the plan to have this
 > as a short to middle term workaround and eventually replace it with a
 > more generic framework?  If so, I can live with that (but please make
 > that clear in comments).  If not...we should discuss it:-)
 >

 Actually, I just don't see another way (yet? perhaps you have an
 idea). Even if we have a configuration value, we'll still need to
 override the default when running from source tree (either by the
 caller or the callee, but i'd prefer to put it in as central a
 location as possible). And not only for tests, also for run_bind10.sh.
 I did not want to add yet another env var, and the location should
 always be fixed from the build directory, so if we do need one, this
 one seems best to me. This is a very similar problem to the database
 file one.

 > '''datasrc_config.h.in'''
 > - We need a header guard for this file.

 ack

 > - I'd avoid using macro whenever possible, and I think in this case
 > we can avoid that (it should be able to be a const char* const). I'd
 > also put the definition in a namespace isc::datasrc.

 ack

 > - We need documentation for MODULE_PATH.

 ack

 > - Although this is independent from the topic of this ticket, I
 >   personally think @prefix@/lib is not the right place for loadable
 >   modules for two reasons: 1. they are not really "libraries", 2. they
 >   are quite BIND 10 specific.  So I'd suggest changing it to
 >   @localstatedir@/@PACKAGE@/modules (which would be expanded to
 >   something like /usr/local/libexec/bind10-devel/modules), either
 >   within this ticket or in a new ticket after creating it.
 >

 ah, yes i asked around for better suggestions but hadn't gotten any,
 so I defaulted to what we were already using. The annoying thing about
 localstatedir is that it expands to a shell variable, not an absolute
 directory (for that matter, even if it would be prefix/lib it
 should've been libdir which does the same, the reason being that
 make is supposed to be able to modify it as well, after configure has
 run).

 Oh, localstate wouldn't expand to libexec, but to var, btw. I like
 libexec a bit more though (not that it's exec, but it's not really
 var/ material either, and this way it is closer to the binary).
 Anyway, I have no really strong opinion, and now that it is expanded
 correctly, we can easily change it.


 > '''bind10_src.py.in'''
 >
 > Why did you reintroduce start_zonemgr and start_stats?  Also, although
 > this is a subject of #1388, we should actually even be able to
 > obsolete (i.e. remove) start_xfrout() and start_xfrin() and remove
 > XfrIn/XfrOut from isc.bind10.special_component.py (and these would
 > also require updating bob.spec).
 >

 Oops. Merge error (started out in the existing branch, then figured
 out there was no reason for that, and move changes to master. That
 brought along some old code. Reapplying cleanly now).

 > We could leave this task to #1388 as originally planned, or complete
 > that part within this ticket.
 >

 Actually I prefer doing it here, so we can show that it works.

 > '''factory.cc'''
 > - maybe a matter of taste in the case of std::string, but I suggest
 >   using empty() instead of length() == 0:
 > {{{
 >     if (type.length() == 0) {
 > }}}

 ok

 > - I'd also move the definition of lib_file just before its first use:
 > {{{
 >     std::string lib_file = type;
 >     const int ext_pos = lib_file.rfind(".so");
 > }}}
 > - Don't we need a test for getDataSourceLibFile?

 did we decide on a good approach for testing
 anonymous-namespace-functions? We can of course make it a function in
 isc::datasrc otherwise.
 (i.e. have not done this yet)

 >
 > '''datasrc/tests/Makefile.am'''
 > - Why was this commented out, not simply removed?
 > {{{
 > -run_unittests_SOURCES += factory_unittest.cc
 > +#run_unittests_SOURCES += factory_unittest.cc
 > }}}
 > - Regarding run_unittests_factory, if the intent is as commented:
 > {{{
 > # Also, we only want to do this when static building is not used.
 > }}}
 >   Shouldn't we wrap this with "if !USE_STATIC_LINK/endif"?
 >

 yes, sorry, forgot about this in my hurry to get you to take a look at
 it :p Moved the if check and removed the old part.

 > '''lettuce bind10_control.py'''
 >
 > I've not confirmed this by running it, but is this okay?
 >
 > {{{#!python
 >     result = bindctl.wait()
 >     (stdout, stderr) = bindctl.communicate()
 > }}}
 >
 > According to the documentation communicate() seems to have the
 > functionality of wait().

 It works (communicate calls wait() again, which immediately returns),
 but it is probably cleaner to only use communicate, then check the
 returncode attribute, so I've changed that.

 >
 > '''lettuce xfrin_bind10.feature'''
 >
 > I'm afraid I'll need to learn more about lettuce to understand this
 > change.  I'm willing to do that, but it will take some time.  For now,
 > maybe it's better to have someone more familiar with lettuce (if any)
 > quickly review and approve it.

 Ok, will do.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1292#comment:24>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development