| Message ID | 1465968834-17361-4-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Accepted |
| Commit | f432c33f27898070718dd568feb104fc1870ca15 |
| Headers | show |
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
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
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)]
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