[3/5] tools: moveconfig: simplify source tree switching

Message ID 1465968834-17361-4-git-send-email-yamada.masahiro@socionext.com
State Accepted
Commit f432c33f27898070718dd568feb104fc1870ca15
Headers show

Commit Message

Masahiro Yamada June 15, 2016, 5:33 a.m.
The subprocess.Popen() does not change the child process's working
directory if cwd=None is given.  Let's exploit this fact to refactor
the source directory handling.

We no longer have to pass "-C <reference_src_dir>" to the sub-process
because self.current_src_dir tracks the source tree against which we
want to run defconfig/autoconf.

The flag self.use_git_ref is not necessary either because we can know
the current state by checking whether the self.current_src_dir is a
valid string or None.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 tools/moveconfig.py | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

-- 
1.9.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Masahiro Yamada June 21, 2016, 1:53 a.m. | #1
2016-06-21 6:33 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
> On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada

> <yamada.masahiro@socionext.com> wrote:

>> The subprocess.Popen() does not change the child process's working

>> directory if cwd=None is given.  Let's exploit this fact to refactor

>> the source directory handling.

>>

>> We no longer have to pass "-C <reference_src_dir>" to the sub-process

>> because self.current_src_dir tracks the source tree against which we

>> want to run defconfig/autoconf.

>>

>> The flag self.use_git_ref is not necessary either because we can know

>> the current state by checking whether the self.current_src_dir is a

>> valid string or None.

>>

>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>> ---

>>

>>  tools/moveconfig.py | 22 +++++++++-------------

>>  1 file changed, 9 insertions(+), 13 deletions(-)

>>

>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py

>> index f4e2580..0e03751 100755

>> --- a/tools/moveconfig.py

>> +++ b/tools/moveconfig.py

>> @@ -645,7 +645,7 @@ class Slot:

>>

>>          self.defconfig = defconfig

>>          self.log = ''

>> -        self.use_git_ref = True if self.options.git_ref else False

>> +        self.current_src_dir = self.reference_src_dir

>>          self.do_defconfig()

>>          return True

>>

>> @@ -674,13 +674,13 @@ class Slot:

>>          if self.ps.poll() != 0:

>>              self.handle_error()

>>          elif self.state == STATE_DEFCONFIG:

>> -            if self.options.git_ref and not self.use_git_ref:

>> +            if self.reference_src_dir and not self.current_src_dir:

>>                  self.do_savedefconfig()

>>              else:

>>                  self.do_autoconf()

>>          elif self.state == STATE_AUTOCONF:

>> -            if self.use_git_ref:

>> -                self.use_git_ref = False

>> +            if self.current_src_dir:

>> +                self.current_src_dir = None

>

> This seems less clear to read. There is no current source dir? I think

> you need a different name.



Maybe,  self.subprocess_dir or something?




-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Masahiro Yamada June 21, 2016, 9:13 p.m. | #2
2016-06-22 1:25 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:
> On Mon, Jun 20, 2016 at 8:53 PM, Masahiro Yamada

> <yamada.masahiro@socionext.com> wrote:

>> 2016-06-21 6:33 GMT+09:00 Joe Hershberger <joe.hershberger@gmail.com>:

>>> On Wed, Jun 15, 2016 at 12:33 AM, Masahiro Yamada

>>> <yamada.masahiro@socionext.com> wrote:

>>>> The subprocess.Popen() does not change the child process's working

>>>> directory if cwd=None is given.  Let's exploit this fact to refactor

>>>> the source directory handling.

>>>>

>>>> We no longer have to pass "-C <reference_src_dir>" to the sub-process

>>>> because self.current_src_dir tracks the source tree against which we

>>>> want to run defconfig/autoconf.

>>>>

>>>> The flag self.use_git_ref is not necessary either because we can know

>>>> the current state by checking whether the self.current_src_dir is a

>>>> valid string or None.

>>>>

>>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>>>> ---

>>>>

>>>>  tools/moveconfig.py | 22 +++++++++-------------

>>>>  1 file changed, 9 insertions(+), 13 deletions(-)

>>>>

>>>> diff --git a/tools/moveconfig.py b/tools/moveconfig.py

>>>> index f4e2580..0e03751 100755

>>>> --- a/tools/moveconfig.py

>>>> +++ b/tools/moveconfig.py

>>>> @@ -645,7 +645,7 @@ class Slot:

>>>>

>>>>          self.defconfig = defconfig

>>>>          self.log = ''

>>>> -        self.use_git_ref = True if self.options.git_ref else False

>>>> +        self.current_src_dir = self.reference_src_dir

>>>>          self.do_defconfig()

>>>>          return True

>>>>

>>>> @@ -674,13 +674,13 @@ class Slot:

>>>>          if self.ps.poll() != 0:

>>>>              self.handle_error()

>>>>          elif self.state == STATE_DEFCONFIG:

>>>> -            if self.options.git_ref and not self.use_git_ref:

>>>> +            if self.reference_src_dir and not self.current_src_dir:

>>>>                  self.do_savedefconfig()

>>>>              else:

>>>>                  self.do_autoconf()

>>>>          elif self.state == STATE_AUTOCONF:

>>>> -            if self.use_git_ref:

>>>> -                self.use_git_ref = False

>>>> +            if self.current_src_dir:

>>>> +                self.current_src_dir = None

>>>

>>> This seems less clear to read. There is no current source dir? I think

>>> you need a different name.

>>

>>

>> Maybe,  self.subprocess_dir or something?

>

> How about something like self.alternate_src_dir?

>


So,
reference_src_dir is still
alternate_src_dir moves


This is not clear to me from the variable names.


My first choice "current" means
it is a moving directory.

I can live with subprocess_dir, though.



-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index f4e2580..0e03751 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -645,7 +645,7 @@  class Slot:
 
         self.defconfig = defconfig
         self.log = ''
-        self.use_git_ref = True if self.options.git_ref else False
+        self.current_src_dir = self.reference_src_dir
         self.do_defconfig()
         return True
 
@@ -674,13 +674,13 @@  class Slot:
         if self.ps.poll() != 0:
             self.handle_error()
         elif self.state == STATE_DEFCONFIG:
-            if self.options.git_ref and not self.use_git_ref:
+            if self.reference_src_dir and not self.current_src_dir:
                 self.do_savedefconfig()
             else:
                 self.do_autoconf()
         elif self.state == STATE_AUTOCONF:
-            if self.use_git_ref:
-                self.use_git_ref = False
+            if self.current_src_dir:
+                self.current_src_dir = None
                 self.do_defconfig()
             else:
                 self.do_savedefconfig()
@@ -706,11 +706,9 @@  class Slot:
 
         cmd = list(self.make_cmd)
         cmd.append(self.defconfig)
-        if self.use_git_ref:
-            cmd.append('-C')
-            cmd.append(self.reference_src_dir)
         self.ps = subprocess.Popen(cmd, stdout=self.devnull,
-                                   stderr=subprocess.PIPE)
+                                   stderr=subprocess.PIPE,
+                                   cwd=self.current_src_dir)
         self.state = STATE_DEFCONFIG
 
     def do_autoconf(self):
@@ -728,11 +726,9 @@  class Slot:
             cmd.append('CROSS_COMPILE=%s' % self.cross_compile)
         cmd.append('KCONFIG_IGNORE_DUPLICATES=1')
         cmd.append('include/config/auto.conf')
-        if self.use_git_ref:
-            cmd.append('-C')
-            cmd.append(self.reference_src_dir)
         self.ps = subprocess.Popen(cmd, stdout=self.devnull,
-                                   stderr=subprocess.PIPE)
+                                   stderr=subprocess.PIPE,
+                                   cwd=self.current_src_dir)
         self.state = STATE_AUTOCONF
 
     def do_savedefconfig(self):
@@ -934,7 +930,7 @@  def move_config(configs, options):
         reference_src = ReferenceSource(options.git_ref)
         reference_src_dir = reference_src.get_dir()
     else:
-        reference_src_dir = ''
+        reference_src_dir = None
 
     if options.defconfigs:
         defconfigs = [line.strip() for line in open(options.defconfigs)]