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      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:10 jinmei]:
 > It basically looks okay.  Some minor comments:
 >
 > - 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.

 > - 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?

 > - 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.

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