Merge lp:~zyga/checkbox/fix-1328903 into lp:checkbox

[Zygmunt Krynicki]

7547a44 plainbox:unit:job: add JobDefinition.flags
e298a86 plainbox:unit:job: add JobDefinition.get_flag_set()
3e24bd8 plainbox:ctrl: add support for 'preserve-locale' flag
c589d44 plainbox:ctrl: fix typo in a comment

[Daniel Manrique (roadmr)]

We need to document the new flags: field, and document possible values since this is handled magically in the code; otherwise this risks becoming an arcane feature nobody will be able to use (like environ: which is rather obscure).

But code-wise, this is OK, so let's go ahead.

review: Approve

[Zygmunt Krynicki (zyga)]

Good point. I'll work on documenting this

On Wed, Jun 11, 2014 at 6:40 PM, Daniel Manrique <
<email address hidden>> wrote:

> The proposal to merge lp:~zkrynicki/checkbox/fix-1328903 into lp:checkbox
> has been updated.
>
> Status: Needs review => Approved
>
> For more details, see:
> https://code.launchpad.net/~zkrynicki/checkbox/fix-1328903/+merge/222835
> --
> https://code.launchpad.net/~zkrynicki/checkbox/fix-1328903/+merge/222835
> You are the owner of lp:~zkrynicki/checkbox/fix-1328903.
>

=== modified file 'plainbox/plainbox/impl/ctrl.py'
--- plainbox/plainbox/impl/ctrl.py	2014-06-10 12:51:38 +0000
+++ plainbox/plainbox/impl/ctrl.py	2014-06-11 16:22:48 +0000
@@ -512,13 +512,15 @@
         """
         # Get a proper environment
         env = dict(os.environ)
-        # Use non-internationalized environment
-        env['LANG'] = 'C.UTF-8'
-        if 'LANGUAGE' in env:
-            del env['LANGUAGE']
-        for name in list(env.keys()):
-            if name.startswith("LC_"):
-                del env[name]
+        # Neuter locale unless 'preserve-locale' flag is set
+        if 'preserve-locale' not in job.get_flag_set():
+            # Use non-internationalized environment
+            env['LANG'] = 'C.UTF-8'
+            if 'LANGUAGE' in env:
+                del env['LANGUAGE']
+            for name in list(env.keys()):
+                if name.startswith("LC_"):
+                    del env[name]
         # Use PATH that can lookup checkbox scripts
         if job.provider.extra_PYTHONPATH:
             env['PYTHONPATH'] = os.pathsep.join(
 
=== modified file 'plainbox/plainbox/impl/test_ctrl.py'
--- plainbox/plainbox/impl/test_ctrl.py	2014-06-10 12:51:38 +0000
+++ plainbox/plainbox/impl/test_ctrl.py	2014-06-11 16:22:48 +0000
@@ -533,6 +533,8 @@
                 extra_PYTHONPATH=None,
                 CHECKBOX_SHARE='CHECKBOX_SHARE',
                 data_dir='data_dir', units_dir='units_dir'))
+        # Mock the default flags (empty set)
+        self.job.get_flag_set.return_value = frozenset()
         # Create mocked config.
         # Put an empty dictionary of environment overrides
         # that is expected by get_execution_environment()
@@ -655,9 +657,18 @@
         # Call the tested method
         env = self.ctrl.get_execution_environment(
             self.job, self.config, self.NEST_DIR)
-        # Ensure that LANG is rese to C.UTF-8
+        # Ensure that LANG is reset to C.UTF-8
         self.assertEqual(env['LANG'], 'C.UTF-8')
 
+    @mock.patch.dict('os.environ', clear=True, LANG='fake_LANG')
+    def test_get_execution_environment_preserves_LANG_if_requested(self):
+        self.job.get_flag_set.return_value = {'preserve-locale'}
+        # Call the tested method
+        env = self.ctrl.get_execution_environment(
+            self.job, self.config, self.NEST_DIR)
+        # Ensure that LANG is what we mocked it to be
+        self.assertEqual(env['LANG'], 'fake_LANG')
+
     @mock.patch.dict('os.environ', clear=True, PYTHONPATH='PYTHONPATH')
     def test_get_execution_environment_keeps_PYTHONPATH(self):
         # Call the tested method
@@ -756,6 +767,8 @@
                 data_dir="data_dir-generator",
                 units_dir="units_dir-generator",
                 CHECKBOX_SHARE='CHECKBOX_SHARE-generator'))
+        # Mock the default flags (empty set)
+        self.job.origin.source.job.get_flag_set.return_value = frozenset()
         PATH = os.pathsep.join([self.NEST_DIR, 'vanilla-path'])
         expected = [
             'pkexec', '--user', self.job.user,
 
=== modified file 'plainbox/plainbox/impl/unit/job.py'
--- plainbox/plainbox/impl/unit/job.py	2014-05-23 13:44:32 +0000
+++ plainbox/plainbox/impl/unit/job.py	2014-06-11 16:22:48 +0000
@@ -309,6 +309,10 @@
         return self.get_record_value('user')
 
     @property
+    def flags(self):
+        return self.get_record_value('flags')
+
+    @property
     def shell(self):
         """
         Shell that is used to interpret the command
@@ -368,6 +372,15 @@
         else:
             return set()
 
+    def get_flag_set(self):
+        """
+        Return a set of flags associated with this job
+        """
+        if self.flags is not None:
+            return {flag for flag in re.split('[\s,]+', self.flags)}
+        else:
+            return set()
+
     @property
     def automated(self):
         """
 
=== modified file 'plainbox/plainbox/impl/unit/test_job.py'
--- plainbox/plainbox/impl/unit/test_job.py	2014-05-23 13:44:32 +0000
+++ plainbox/plainbox/impl/unit/test_job.py	2014-06-11 16:22:48 +0000
@@ -67,18 +67,21 @@
             'environ': 'environ-value',
             'user': 'user-value',
             'shell': 'shell-value',
+            'flags': 'flags-value',
         }, raw_data={
             'plugin': 'plugin-raw',
             'command': 'command-raw',
             'environ': 'environ-raw',
             'user': 'user-raw',
             'shell': 'shell-raw',
+            'flags': 'flags-raw',
         })
         self.assertEqual(job.plugin, "plugin-value")
         self.assertEqual(job.command, "command-value")
         self.assertEqual(job.environ, "environ-value")
         self.assertEqual(job.user, "user-value")
         self.assertEqual(job.shell, "shell-value")
+        self.assertEqual(job.flags, "flags-value")
 
     def test_properties_default_values(self):
         """
@@ -90,6 +93,7 @@
         self.assertEqual(job.environ, None)
         self.assertEqual(job.user, None)
         self.assertEqual(job.shell, 'bash')
+        self.assertEqual(job.flags, None)
 
     def test_checksum_smoke(self):
         job1 = JobDefinition({'plugin': 'plugin', 'user': 'root'})
@@ -112,6 +116,14 @@
         job3 = JobDefinition({'environ': 'a,b,c'})
         self.assertEqual(job3.get_environ_settings(), set(['a', 'b', 'c']))
 
+    def test_get_flag_set(self):
+        job1 = JobDefinition({})
+        self.assertEqual(job1.get_flag_set(), set())
+        job2 = JobDefinition({'flags': 'a b c'})
+        self.assertEqual(job2.get_flag_set(), set(['a', 'b', 'c']))
+        job3 = JobDefinition({'flags': 'a,b,c'})
+        self.assertEqual(job3.get_flag_set(), set(['a', 'b', 'c']))
+
 
 class JobDefinitionParsingTests(TestCaseWithParameters):