[1292_2] address review comments

src/bin/bind10/bind10_src.py.in

@@ -640,22 +640,6 @@ class BoB:
         return self.start_process("b10-cmdctl", args, self.c_channel_env,
                                   self.cmdctl_port)
 
-    def start_xfrin(self):
-        # Set up the command arguments.
-        args = ['b10-xfrin']
-        if self.verbose:
-            args += ['-v']
-
-        return self.start_process("b10-xfrin", args, self.c_channel_env)
-
-    def start_xfrout(self):
-        # Set up the command arguments.
-        args = ['b10-xfrout']
-        if self.verbose:
-            args += ['-v']
-
-        return self.start_process("b10-xfrout", args, self.c_channel_env)
-
     def start_all_components(self):
         """
             Starts up all the components.  Any exception generated during the

src/bin/bind10/bob.spec

@@ -14,8 +14,8 @@
             "priority": 5,
             "kind": "dispensable"
           },
-          "b10-xfrin": { "special": "xfrin", "kind": "dispensable" },
-          "b10-xfrout": { "special": "xfrout", "kind": "dispensable" },
+          "b10-xfrin": { "address": "Xfrin", "kind": "dispensable" },
+          "b10-xfrout": { "address": "Xfrout", "kind": "dispensable" },
           "b10-zonemgr": { "address": "Zonemgr", "kind": "dispensable" },
           "b10-stats": { "address": "Stats", "kind": "dispensable" },
           "b10-stats-httpd": {

src/bin/bind10/tests/bind10_test.py.in

@@ -279,7 +279,9 @@ class MockBob(BoB):
                     'b10-stats-httpd': self.start_stats_httpd,
                     'b10-cmdctl': self.start_cmdctl,
                     'b10-dhcp6': self.start_dhcp6,
-                    'b10-dhcp4': self.start_dhcp4 }
+                    'b10-dhcp4': self.start_dhcp4,
+                    'b10-xfrin': self.start_xfrin,
+                    'b10-xfrout': self.start_xfrout }
         return procmap[name]()
 
     def start_xfrout(self):
@@ -474,8 +476,9 @@ class TestStartStopProcessesBob(unittest.TestCase):
         if start_auth:
             config['b10-auth'] = { 'kind': 'needed', 'special': 'auth' }
             config['b10-xfrout'] = { 'kind': 'dispensable',
-                                     'special': 'xfrout' }
-            config['b10-xfrin'] = { 'kind': 'dispensable', 'special': 'xfrin' }
+                                     'address': 'Xfrout' }
+            config['b10-xfrin'] = { 'kind': 'dispensable',
+                                    'address': 'Xfrin' }
             config['b10-zonemgr'] = { 'kind': 'dispensable',
                                       'address': 'Zonemgr' }
         if start_resolver:

src/lib/datasrc/Makefile.am

@@ -13,6 +13,7 @@ datasrc_config.h: datasrc_config.h.pre
 	$(SED) -e "s|@@PKGLIBEXECDIR@@|$(pkglibexecdir)|" datasrc_config.h.pre >$@
 
 CLEANFILES = *.gcno *.gcda datasrc_messages.h datasrc_messages.cc
+CLEANFILES += datasrc_config.h
 
 lib_LTLIBRARIES = libdatasrc.la
 libdatasrc_la_SOURCES = data_source.h data_source.cc

src/lib/datasrc/tests/Makefile.am

@@ -105,10 +105,12 @@ EXTRA_DIST += testdata/rwtest.sqlite3
 EXTRA_DIST += testdata/diffs.sqlite3
 
 # For the factory unit tests, we need to specify that we want
-# the libraries from the build tree, and not from the installation
-# directory. Therefore we build it into a separate binary,
-# and call that from check-local with B10_FROM_BUILD set.
-# Also, we only want to do this when static building is not used.
+# the loadable modules from the build tree, and not from the
+# installation directory. Therefore we build it into a separate
+# binary, and call that from check-local with B10_FROM_BUILD set.
+# Also, we only want to do this when static building is not used,
+# since it will cause various troubles with static link such as
+# "missing" symbols in the static object for the module.
 if !USE_STATIC_LINK
 noinst_PROGRAMS+=run_unittests_factory
 run_unittests_factory_SOURCES = $(common_sources)

src/lib/datasrc/tests/factory_unittest.cc

@@ -30,6 +30,43 @@ std::string SQLITE_DBFILE_EXAMPLE_ORG = TEST_DATA_DIR "/example.org.sqlite3";
 
 namespace {
 
+void
+pathtest_helper(const std::string& file, const std::string& expected_error) {
+    std::string error;
+    try {
+        DataSourceClientContainer(file, ElementPtr());
+    } catch (const DataSourceLibraryError& dsle) {
+        error = dsle.what();
+    }
+    EXPECT_EQ(expected_error, error);
+}
+
+TEST(FactoryTest, paths) {
+    // Test whether the paths are made absolute if they are not,
+    // by inspecting the error that is raised when they are wrong
+    const std::string error(": cannot open shared object file: "
+                            "No such file or directory");
+    // With the current implementation, we can safely assume this has
+    // been set for this test (as the loader would otherwise also fail
+    // unless the module happens to be installed)
+    const std::string builddir(getenv("B10_FROM_BUILD"));
+
+    // Absolute and ending with .so should have no change
+    pathtest_helper("/no_such_file.so", "/no_such_file.so" + error);
+
+    // If no ending in .so, it should get _ds.so
+    pathtest_helper("/no_such_file", "/no_such_file_ds.so" + error);
+
+    // If not starting with /, path should be added. For this test that
+    // means the build directory as set in B10_FROM_BUILD
+    pathtest_helper("no_such_file.so",
+                    builddir + "/src/lib/datasrc/.libs/no_such_file.so" +
+                    error);
+    pathtest_helper("no_such_file",
+                    builddir + "/src/lib/datasrc/.libs/no_such_file_ds.so" + 
+                    error);
+}
+
 TEST(FactoryTest, sqlite3ClientBadConfig) {
     // We start out by building the configuration data bit by bit,
     // testing each form of 'bad config', until we have a good one.

src/lib/python/isc/bind10/special_component.py

@@ -108,16 +108,6 @@ class CmdCtl(Component):
         Component.__init__(self, process, boss, kind, 'Cmdctl', None,
                            boss.start_cmdctl)
 
-class XfrIn(Component):
-    def __init__(self, process, boss, kind, address=None, params=None):
-        Component.__init__(self, process, boss, kind, 'Xfrin', None,
-                           boss.start_xfrin)
-
-class XfrOut(Component):
-    def __init__(self, process, boss, kind, address=None, params=None):
-        Component.__init__(self, process, boss, kind, 'Xfrout', None,
-                           boss.start_xfrout)
-
 class SetUID(BaseComponent):
     """
     This is a pseudo-component which drops root privileges when started
@@ -157,9 +147,6 @@ def get_specials():
         'auth': Auth,
         'resolver': Resolver,
         'cmdctl': CmdCtl,
-        # FIXME: Temporary workaround before #1292 is done
-        'xfrin': XfrIn,
-        'xfrout': XfrOut,
         # TODO: Remove when not needed, workaround before sockcreator works
         'setuid': SetUID
     }

src/lib/python/isc/bind10/tests/component_test.py

@@ -86,9 +86,6 @@ class BossUtils:
     def start_cmdctl(self):
         pass
 
-    def start_xfrin(self):
-        pass
-
 class ComponentTests(BossUtils, unittest.TestCase):
     """
     Tests for the bind10.component.Component class
@@ -430,7 +427,6 @@ class ComponentTests(BossUtils, unittest.TestCase):
                                isc.bind10.special_component.Auth,
                                isc.bind10.special_component.Resolver,
                                isc.bind10.special_component.CmdCtl,
-                               isc.bind10.special_component.XfrIn,
                                isc.bind10.special_component.SetUID]:
             component = component_type('none', self, 'needed')
             self.assertIsNone(component.pid())