BIND 10 #1246: socketcreator breaks "FROM_SOURCE" environment

[BIND 10 Development]

#1246: socketcreator breaks "FROM_SOURCE" environment
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20111025
                   Priority:  major  |            Resolution:
                  Component:  Boss   |             Sensitive:  0
  of BIND                            |           Sub-Project:  Core
                   Keywords:         |  Estimated Difficulty:  2
            Defect Severity:  High   |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:12 jelte]:

 > > - If possible I'd avoid embedding the hardcoded check for
 B10_FROM_SOURCE
 > >   in bind10_src.py or at least unify the point of this check (e.g. via
 > >   a command line option).
 >
 > I did not make it a command-line option (yet, we can consider this,
 > but it could be part of a different approach to supporting 'in-source'
 > running)
 >
 > However, I did unify it in the sense that it'll now set a global
 > variable in the 'big' check right at the start (so at least the
 > environment variable name is only used once). Updated the several
 > checks for the environment variable to use that one.

 Okay, but the LD PATH hack is expected to be a very short term workaround,
 and when it's resolved the "AND_LD" part in ADD_LIBEXEC_AND_LD_PATH will
 be meaningless.  I'd rename it to something a bit generic, e.g.
 ADD_INSTALLED_PATHS.  Or we might simply say ADD_LIBEXEC_PATH and
 add a comment to the "LD" part that we abuse this variable name for a
 short term hack.

 > > - isn't this test self-contained?
 > > {{{
 > >     def test_unchanged_environment(self):
 > >         # Check whether the environment has not been changed
 > >         self.assertEqual(original_os_environ, os.environ)
 > > }}}
 > >   i.e., it seems to assume that other tests could modify os.environ
 > >   without the change introduced in this branch.  I'd make sure this
 > >   test itself performs start_all_process() or something.
 >
 > since this test is last, if they actually change the environment, it
 > would show (note that both the deepcopy and the environment check fix
 > this, so to see the test failing you'd need to revert all changes).
 >
 > But if the order isn't guaranteed, or if we feel this is relying too
 > much on unittest framework implementation details, we could also make
 > this a helper function and add it to the end of all tests. Would you
 > prefer that?

 I don't know if the order is guaranteed or not by the unittest module
 specification, but I'd generally avoid relyin on test orders.  We might
 want to disable other part of the tests or we might want to reorder
 the tests, and I'd like to make it less susceptible to such events.

 > > - do we need a changelog entry?  This is certainly a visible bug, but
 > >   on the other hand only core developers would care, so mostly
 irrelevant
 > >   to end users.  I'd leave it to you.
 >
 > I'll ask jeremy. In the case we do, I propose this one:
 > XXX. [defect] jelte
 > The main bind10 script now no longer runs processes from an installed
 > version when it is run from the source tree with run_bind10.sh. It
 > will correctly use the source tree build.

 This looks okay.  I'd leave it to you and Jeremy whether to actually
 include it.

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