All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] moveconfig: Add a new --git-ref option
@ 2015-05-29 21:23 Joe Hershberger
  2015-06-02 13:42 ` Joe Hershberger
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Joe Hershberger @ 2015-05-29 21:23 UTC (permalink / raw)
  To: u-boot

This option allows the 'make *_defconfig' step to run against a former
repo state, while the savedefconfig step runs against the current repo
state. This is convenient for the case where something in the Kconfig
has changed such that the defconfig is no longer complete with the new
Kconfigs. This feature allows the .config to be built assuming those old
Kconfigs, but then savedefconfig based on the new state of the Kconfigs.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

 tools/moveconfig.py     | 60 +++++++++++++++++++++++++++++++++++++++++++++----
 tools/patman/gitutil.py | 15 +++++++++++++
 2 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 496c90a..eeb9c0e 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -153,6 +153,10 @@ Available options
    Specify the number of threads to run simultaneously.  If not specified,
    the number of threads is the same as the number of CPU cores.
 
+ -r, --git-ref
+   Specify the git ref to clone for the make *_defconfig step. If unspecified
+   use the CWD.
+
  -v, --verbose
    Show any build errors as boards are built
 
@@ -173,6 +177,12 @@ import sys
 import tempfile
 import time
 
+# Bring in the patman libraries
+our_path = os.path.dirname(os.path.realpath(__file__))
+sys.path.append(os.path.join(our_path, 'patman'))
+
+import gitutil
+
 SHOW_GNU_MAKE = 'scripts/show-gnu-make'
 SLEEP_TIME=0.03
 
@@ -526,7 +536,7 @@ class Slot:
     for faster processing.
     """
 
-    def __init__(self, config_attrs, options, devnull, make_cmd):
+    def __init__(self, config_attrs, options, devnull, make_cmd, defconfig_src_dir):
         """Create a new process slot.
 
         Arguments:
@@ -540,6 +550,7 @@ class Slot:
         self.build_dir = tempfile.mkdtemp()
         self.devnull = devnull
         self.make_cmd = (make_cmd, 'O=' + self.build_dir)
+        self.defconfig_src_dir = defconfig_src_dir
         self.parser = KconfigParser(config_attrs, options, self.build_dir)
         self.state = STATE_IDLE
         self.failed_boards = []
@@ -576,6 +587,9 @@ class Slot:
             return False
         cmd = list(self.make_cmd)
         cmd.append(defconfig)
+        if self.options.git_ref:
+            cmd.append('-C')
+            cmd.append(self.defconfig_src_dir)
         self.ps = subprocess.Popen(cmd, stdout=self.devnull,
                                    stderr=subprocess.PIPE)
         self.defconfig = defconfig
@@ -658,6 +672,9 @@ class Slot:
         cmd.append('include/config/auto.conf')
         """This will be screen-scraped, so be sure the expected text will be
         returned consistently on every machine by setting LANG=C"""
+        if self.options.git_ref:
+            cmd.append('-C')
+            cmd.append(self.defconfig_src_dir)
         self.ps = subprocess.Popen(cmd, stdout=self.devnull,
                                    env=dict(os.environ, LANG='C'),
                                    stderr=subprocess.PIPE)
@@ -673,7 +690,7 @@ class Slots:
 
     """Controller of the array of subprocess slots."""
 
-    def __init__(self, config_attrs, options):
+    def __init__(self, config_attrs, options, defconfig_src_dir):
         """Create a new slots controller.
 
         Arguments:
@@ -686,7 +703,8 @@ class Slots:
         devnull = get_devnull()
         make_cmd = get_make_cmd()
         for i in range(options.jobs):
-            self.slots.append(Slot(config_attrs, options, devnull, make_cmd))
+            self.slots.append(Slot(config_attrs, options, devnull, make_cmd,
+                                   defconfig_src_dir))
 
     def add(self, defconfig, num, total):
         """Add a new subprocess if a vacant slot is found.
@@ -743,6 +761,24 @@ class Slots:
                 for board in failed_boards:
                     f.write(board + '\n')
 
+class WorkDir:
+    def __init__(self):
+        """Create a new working directory."""
+        self.work_dir = tempfile.mkdtemp()
+
+    def __del__(self):
+        """Delete the working directory
+
+        This function makes sure the temporary directory is cleaned away
+        even if Python suddenly dies due to error.  It should be done in here
+        because it is guaranteed the destructor is always invoked when the
+        instance of the class gets unreferenced.
+        """
+        shutil.rmtree(self.work_dir)
+
+    def get(self):
+        return self.work_dir
+
 def move_config(config_attrs, options):
     """Move config options to defconfig files.
 
@@ -755,6 +791,20 @@ def move_config(config_attrs, options):
         print 'Nothing to do. exit.'
         sys.exit(0)
 
+    defconfig_src_dir = ''
+
+    if options.git_ref:
+        work_dir = WorkDir()
+        defconfig_src_dir = work_dir.get()
+        cwd = os.getcwd()
+        print 'Cloning git repo for \'make *_defconfig\' step...'
+        gitutil.Clone(cwd, defconfig_src_dir)
+        print 'Checkout \'%s\' to find original configs.' % \
+            gitutil.CommitHash(options.git_ref)
+        os.chdir(defconfig_src_dir)
+        gitutil.Checkout(options.git_ref)
+        os.chdir(cwd)
+
     print 'Move the following CONFIG options (jobs: %d)' % options.jobs
     for config_attr in config_attrs:
         print '  %s (type: %s, default: %s)' % (config_attr['config'],
@@ -777,7 +827,7 @@ def move_config(config_attrs, options):
             for filename in fnmatch.filter(filenames, '*_defconfig'):
                 defconfigs.append(os.path.join(dirpath, filename))
 
-    slots = Slots(config_attrs, options)
+    slots = Slots(config_attrs, options, defconfig_src_dir)
 
     # Main loop to process defconfig files:
     #  Add a new subprocess into a vacant slot.
@@ -887,6 +937,8 @@ def main():
                       help='only cleanup the headers')
     parser.add_option('-j', '--jobs', type='int', default=cpu_count,
                       help='the number of jobs to run simultaneously')
+    parser.add_option('-r', '--git-ref', type='string',
+                      help='the git ref to clone for the make *_defconfig step')
     parser.add_option('-v', '--verbose', action='store_true', default=False,
                       help='show any build errors as boards are built')
     parser.usage += ' recipe_file\n\n' + \
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 9e739d8..138f989 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -61,6 +61,21 @@ def CountCommitsToBranch():
     patch_count = int(stdout)
     return patch_count
 
+def CommitHash(commit_ref):
+    """Gets the hash for a commit
+
+    Args:
+        commit_ref: Commit ref to look up
+
+    Return:
+        Hash of revision, if any, else None
+    """
+    pipe = ['git', 'rev-parse', '--short', commit_ref]
+    stdout = command.RunPipe([pipe], capture=True, oneline=True).stdout
+
+    hash = stdout.strip()
+    return hash
+
 def NameRevision(commit_hash):
     """Gets the revision name for a commit
 
-- 
1.7.11.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH] moveconfig: Add a new --git-ref option
  2015-05-29 21:23 [U-Boot] [PATCH] moveconfig: Add a new --git-ref option Joe Hershberger
@ 2015-06-02 13:42 ` Joe Hershberger
  2015-06-05  4:54 ` Masahiro Yamada
  2016-06-02  3:30 ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Joe Hershberger
  2 siblings, 0 replies; 19+ messages in thread
From: Joe Hershberger @ 2015-06-02 13:42 UTC (permalink / raw)
  To: u-boot

Hey guys,

On Fri, May 29, 2015 at 4:23 PM, Joe Hershberger <joe.hershberger@ni.com> wrote:
> This option allows the 'make *_defconfig' step to run against a former
> repo state, while the savedefconfig step runs against the current repo
> state. This is convenient for the case where something in the Kconfig
> has changed such that the defconfig is no longer complete with the new
> Kconfigs. This feature allows the .config to be built assuming those old
> Kconfigs, but then savedefconfig based on the new state of the Kconfigs.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---

I forgot the Cc's on this...

>  tools/moveconfig.py     | 60 +++++++++++++++++++++++++++++++++++++++++++++----
>  tools/patman/gitutil.py | 15 +++++++++++++
>  2 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/tools/moveconfig.py b/tools/moveconfig.py
> index 496c90a..eeb9c0e 100755
> --- a/tools/moveconfig.py
> +++ b/tools/moveconfig.py
> @@ -153,6 +153,10 @@ Available options
>     Specify the number of threads to run simultaneously.  If not specified,
>     the number of threads is the same as the number of CPU cores.
>
> + -r, --git-ref
> +   Specify the git ref to clone for the make *_defconfig step. If unspecified
> +   use the CWD.
> +
>   -v, --verbose
>     Show any build errors as boards are built
>
> @@ -173,6 +177,12 @@ import sys
>  import tempfile
>  import time
>
> +# Bring in the patman libraries
> +our_path = os.path.dirname(os.path.realpath(__file__))
> +sys.path.append(os.path.join(our_path, 'patman'))
> +
> +import gitutil
> +
>  SHOW_GNU_MAKE = 'scripts/show-gnu-make'
>  SLEEP_TIME=0.03
>
> @@ -526,7 +536,7 @@ class Slot:
>      for faster processing.
>      """
>
> -    def __init__(self, config_attrs, options, devnull, make_cmd):
> +    def __init__(self, config_attrs, options, devnull, make_cmd, defconfig_src_dir):
>          """Create a new process slot.
>
>          Arguments:
> @@ -540,6 +550,7 @@ class Slot:
>          self.build_dir = tempfile.mkdtemp()
>          self.devnull = devnull
>          self.make_cmd = (make_cmd, 'O=' + self.build_dir)
> +        self.defconfig_src_dir = defconfig_src_dir
>          self.parser = KconfigParser(config_attrs, options, self.build_dir)
>          self.state = STATE_IDLE
>          self.failed_boards = []
> @@ -576,6 +587,9 @@ class Slot:
>              return False
>          cmd = list(self.make_cmd)
>          cmd.append(defconfig)
> +        if self.options.git_ref:
> +            cmd.append('-C')
> +            cmd.append(self.defconfig_src_dir)
>          self.ps = subprocess.Popen(cmd, stdout=self.devnull,
>                                     stderr=subprocess.PIPE)
>          self.defconfig = defconfig
> @@ -658,6 +672,9 @@ class Slot:
>          cmd.append('include/config/auto.conf')
>          """This will be screen-scraped, so be sure the expected text will be
>          returned consistently on every machine by setting LANG=C"""
> +        if self.options.git_ref:
> +            cmd.append('-C')
> +            cmd.append(self.defconfig_src_dir)
>          self.ps = subprocess.Popen(cmd, stdout=self.devnull,
>                                     env=dict(os.environ, LANG='C'),
>                                     stderr=subprocess.PIPE)
> @@ -673,7 +690,7 @@ class Slots:
>
>      """Controller of the array of subprocess slots."""
>
> -    def __init__(self, config_attrs, options):
> +    def __init__(self, config_attrs, options, defconfig_src_dir):
>          """Create a new slots controller.
>
>          Arguments:
> @@ -686,7 +703,8 @@ class Slots:
>          devnull = get_devnull()
>          make_cmd = get_make_cmd()
>          for i in range(options.jobs):
> -            self.slots.append(Slot(config_attrs, options, devnull, make_cmd))
> +            self.slots.append(Slot(config_attrs, options, devnull, make_cmd,
> +                                   defconfig_src_dir))
>
>      def add(self, defconfig, num, total):
>          """Add a new subprocess if a vacant slot is found.
> @@ -743,6 +761,24 @@ class Slots:
>                  for board in failed_boards:
>                      f.write(board + '\n')
>
> +class WorkDir:
> +    def __init__(self):
> +        """Create a new working directory."""
> +        self.work_dir = tempfile.mkdtemp()
> +
> +    def __del__(self):
> +        """Delete the working directory
> +
> +        This function makes sure the temporary directory is cleaned away
> +        even if Python suddenly dies due to error.  It should be done in here
> +        because it is guaranteed the destructor is always invoked when the
> +        instance of the class gets unreferenced.
> +        """
> +        shutil.rmtree(self.work_dir)
> +
> +    def get(self):
> +        return self.work_dir
> +
>  def move_config(config_attrs, options):
>      """Move config options to defconfig files.
>
> @@ -755,6 +791,20 @@ def move_config(config_attrs, options):
>          print 'Nothing to do. exit.'
>          sys.exit(0)
>
> +    defconfig_src_dir = ''
> +
> +    if options.git_ref:
> +        work_dir = WorkDir()
> +        defconfig_src_dir = work_dir.get()
> +        cwd = os.getcwd()
> +        print 'Cloning git repo for \'make *_defconfig\' step...'
> +        gitutil.Clone(cwd, defconfig_src_dir)
> +        print 'Checkout \'%s\' to find original configs.' % \
> +            gitutil.CommitHash(options.git_ref)
> +        os.chdir(defconfig_src_dir)
> +        gitutil.Checkout(options.git_ref)
> +        os.chdir(cwd)
> +
>      print 'Move the following CONFIG options (jobs: %d)' % options.jobs
>      for config_attr in config_attrs:
>          print '  %s (type: %s, default: %s)' % (config_attr['config'],
> @@ -777,7 +827,7 @@ def move_config(config_attrs, options):
>              for filename in fnmatch.filter(filenames, '*_defconfig'):
>                  defconfigs.append(os.path.join(dirpath, filename))
>
> -    slots = Slots(config_attrs, options)
> +    slots = Slots(config_attrs, options, defconfig_src_dir)
>
>      # Main loop to process defconfig files:
>      #  Add a new subprocess into a vacant slot.
> @@ -887,6 +937,8 @@ def main():
>                        help='only cleanup the headers')
>      parser.add_option('-j', '--jobs', type='int', default=cpu_count,
>                        help='the number of jobs to run simultaneously')
> +    parser.add_option('-r', '--git-ref', type='string',
> +                      help='the git ref to clone for the make *_defconfig step')
>      parser.add_option('-v', '--verbose', action='store_true', default=False,
>                        help='show any build errors as boards are built')
>      parser.usage += ' recipe_file\n\n' + \
> diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
> index 9e739d8..138f989 100644
> --- a/tools/patman/gitutil.py
> +++ b/tools/patman/gitutil.py
> @@ -61,6 +61,21 @@ def CountCommitsToBranch():
>      patch_count = int(stdout)
>      return patch_count
>
> +def CommitHash(commit_ref):
> +    """Gets the hash for a commit
> +
> +    Args:
> +        commit_ref: Commit ref to look up
> +
> +    Return:
> +        Hash of revision, if any, else None
> +    """
> +    pipe = ['git', 'rev-parse', '--short', commit_ref]
> +    stdout = command.RunPipe([pipe], capture=True, oneline=True).stdout
> +
> +    hash = stdout.strip()
> +    return hash
> +
>  def NameRevision(commit_hash):
>      """Gets the revision name for a commit
>
> --
> 1.7.11.5
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH] moveconfig: Add a new --git-ref option
  2015-05-29 21:23 [U-Boot] [PATCH] moveconfig: Add a new --git-ref option Joe Hershberger
  2015-06-02 13:42 ` Joe Hershberger
@ 2015-06-05  4:54 ` Masahiro Yamada
  2015-06-10 14:16   ` Joe Hershberger
  2016-06-02  3:30 ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Joe Hershberger
  2 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2015-06-05  4:54 UTC (permalink / raw)
  To: u-boot

Hi Joe,


2015-05-30 6:23 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
> This option allows the 'make *_defconfig' step to run against a former
> repo state, while the savedefconfig step runs against the current repo
> state. This is convenient for the case where something in the Kconfig
> has changed such that the defconfig is no longer complete with the new
> Kconfigs. This feature allows the .config to be built assuming those old
> Kconfigs, but then savedefconfig based on the new state of the Kconfigs.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>


This idea seems nice, but I have some comments about the implementation.



> +    defconfig_src_dir = ''
> +
> +    if options.git_ref:
> +        work_dir = WorkDir()
> +        defconfig_src_dir = work_dir.get()
> +        cwd = os.getcwd()
> +        print 'Cloning git repo for \'make *_defconfig\' step...'

You can use signle quotes without escape sequences inside "...", and vice versa.





> diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
> index 9e739d8..138f989 100644
> --- a/tools/patman/gitutil.py
> +++ b/tools/patman/gitutil.py
> @@ -61,6 +61,21 @@ def CountCommitsToBranch():
>      patch_count = int(stdout)
>      return patch_count
>
> +def CommitHash(commit_ref):
> +    """Gets the hash for a commit
> +
> +    Args:
> +        commit_ref: Commit ref to look up
> +
> +    Return:
> +        Hash of revision, if any, else None
> +    """
> +    pipe = ['git', 'rev-parse', '--short', commit_ref]
> +    stdout = command.RunPipe([pipe], capture=True, oneline=True).stdout
> +
> +    hash = stdout.strip()
> +    return hash
> +
>  def NameRevision(commit_hash):
>      """Gets the revision name for a commit



Finally, this tool is going to depend on patman.  I am afraid this
tool is getting messy.

gitutils.py depends on command.py, and then command.py depends on
cros_subprocess.py.

Do you really need to invoke such a chain of libraries
to run a sub-process?


For example, you can get a hash in a single line like this:

subprocess.check_output(['git', 'rev-parse', '--short', 'HEAD'])





-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH] moveconfig: Add a new --git-ref option
  2015-06-05  4:54 ` Masahiro Yamada
@ 2015-06-10 14:16   ` Joe Hershberger
  0 siblings, 0 replies; 19+ messages in thread
From: Joe Hershberger @ 2015-06-10 14:16 UTC (permalink / raw)
  To: u-boot

Hi Masahiro-san,

On Thu, Jun 4, 2015 at 11:54 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Joe,
>
>
> 2015-05-30 6:23 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
>> This option allows the 'make *_defconfig' step to run against a former
>> repo state, while the savedefconfig step runs against the current repo
>> state. This is convenient for the case where something in the Kconfig
>> has changed such that the defconfig is no longer complete with the new
>> Kconfigs. This feature allows the .config to be built assuming those old
>> Kconfigs, but then savedefconfig based on the new state of the Kconfigs.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
>
> This idea seems nice, but I have some comments about the implementation.
>
>
>
>> +    defconfig_src_dir = ''
>> +
>> +    if options.git_ref:
>> +        work_dir = WorkDir()
>> +        defconfig_src_dir = work_dir.get()
>> +        cwd = os.getcwd()
>> +        print 'Cloning git repo for \'make *_defconfig\' step...'
>
> You can use signle quotes without escape sequences inside "...", and vice versa.

OK

>> diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
>> index 9e739d8..138f989 100644
>> --- a/tools/patman/gitutil.py
>> +++ b/tools/patman/gitutil.py
>> @@ -61,6 +61,21 @@ def CountCommitsToBranch():
>>      patch_count = int(stdout)
>>      return patch_count
>>
>> +def CommitHash(commit_ref):
>> +    """Gets the hash for a commit
>> +
>> +    Args:
>> +        commit_ref: Commit ref to look up
>> +
>> +    Return:
>> +        Hash of revision, if any, else None
>> +    """
>> +    pipe = ['git', 'rev-parse', '--short', commit_ref]
>> +    stdout = command.RunPipe([pipe], capture=True, oneline=True).stdout
>> +
>> +    hash = stdout.strip()
>> +    return hash
>> +
>>  def NameRevision(commit_hash):
>>      """Gets the revision name for a commit
>
>
>
> Finally, this tool is going to depend on patman.  I am afraid this
> tool is getting messy.
>
> gitutils.py depends on command.py, and then command.py depends on
> cros_subprocess.py.
>
> Do you really need to invoke such a chain of libraries
> to run a sub-process?

Why does that matter?

> For example, you can get a hash in a single line like this:
>
> subprocess.check_output(['git', 'rev-parse', '--short', 'HEAD'])

I certainly can do that, but it seems better to reuse the common
functionality. This tool doesn't seem so useful elsewhere that a
person would want it to work easily outside of u-boot. In u-boot,
patman exists. Simon did the same thing for buildman. I can change it
to call the git commands directly sine what I'm doing is simple, but I
don't see any value in doing it.

Simon, what do you think?

Thanks,
-Joe

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear
  2015-05-29 21:23 [U-Boot] [PATCH] moveconfig: Add a new --git-ref option Joe Hershberger
  2015-06-02 13:42 ` Joe Hershberger
  2015-06-05  4:54 ` Masahiro Yamada
@ 2016-06-02  3:30 ` Joe Hershberger
  2016-06-02  3:30   ` [U-Boot] [PATCH v2 2/2] tools: moveconfig: Add a new --git-ref option Joe Hershberger
  2016-06-08  2:47   ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Masahiro Yamada
  2 siblings, 2 replies; 19+ messages in thread
From: Joe Hershberger @ 2016-06-02  3:30 UTC (permalink / raw)
  To: u-boot

Make the processing of a slot more linear code compared to how it
executes.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

---

Changes in v2:
- New for v2

 tools/moveconfig.py | 59 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 2d29e1b..01350ce 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -197,9 +197,10 @@ CROSS_COMPILE = {
 }
 
 STATE_IDLE = 0
-STATE_DEFCONFIG = 1
-STATE_AUTOCONF = 2
-STATE_SAVEDEFCONFIG = 3
+STATE_INIT = 1
+STATE_DEFCONFIG = 2
+STATE_AUTOCONF = 3
+STATE_SAVEDEFCONFIG = 4
 
 ACTION_MOVE = 0
 ACTION_NO_ENTRY = 1
@@ -633,12 +634,9 @@ class Slot:
         """
         if self.state != STATE_IDLE:
             return False
-        cmd = list(self.make_cmd)
-        cmd.append(defconfig)
-        self.ps = subprocess.Popen(cmd, stdout=self.devnull,
-                                   stderr=subprocess.PIPE)
+        self.ps = None
         self.defconfig = defconfig
-        self.state = STATE_DEFCONFIG
+        self.state = STATE_INIT
         self.log = ''
         return True
 
@@ -661,6 +659,14 @@ class Slot:
         if self.state == STATE_IDLE:
             return True
 
+        if self.state == STATE_INIT:
+            cmd = list(self.make_cmd)
+            cmd.append(self.defconfig)
+            self.ps = subprocess.Popen(cmd, stdout=self.devnull,
+                                       stderr=subprocess.PIPE)
+            self.state = STATE_DEFCONFIG
+            return False
+
         if self.ps.poll() == None:
             return False
 
@@ -673,6 +679,24 @@ class Slot:
             self.finish(False)
             return True
 
+        if self.state == STATE_DEFCONFIG:
+            self.cross_compile = self.parser.get_cross_compile()
+            if self.cross_compile is None:
+                self.log += color_text(self.options.color, COLOR_YELLOW,
+                                       "Compiler is missing.  Do nothing.\n")
+                self.finish(False)
+                return True
+
+            cmd = list(self.make_cmd)
+            if self.cross_compile:
+                cmd.append('CROSS_COMPILE=%s' % self.cross_compile)
+            cmd.append('KCONFIG_IGNORE_DUPLICATES=1')
+            cmd.append('include/config/auto.conf')
+            self.ps = subprocess.Popen(cmd, stdout=self.devnull,
+                                       stderr=subprocess.PIPE)
+            self.state = STATE_AUTOCONF
+            return False
+
         if self.state == STATE_AUTOCONF:
             (updated, log) = self.parser.update_dotconfig()
             self.log += log
@@ -708,22 +732,9 @@ class Slot:
             self.finish(True)
             return True
 
-        self.cross_compile = self.parser.get_cross_compile()
-        if self.cross_compile is None:
-            self.log += color_text(self.options.color, COLOR_YELLOW,
-                                   "Compiler is missing.  Do nothing.\n")
-            self.finish(False)
-            return True
-
-        cmd = list(self.make_cmd)
-        if self.cross_compile:
-            cmd.append('CROSS_COMPILE=%s' % self.cross_compile)
-        cmd.append('KCONFIG_IGNORE_DUPLICATES=1')
-        cmd.append('include/config/auto.conf')
-        self.ps = subprocess.Popen(cmd, stdout=self.devnull,
-                                   stderr=subprocess.PIPE)
-        self.state = STATE_AUTOCONF
-        return False
+        # Undefined state!
+        self.finish(False)
+        return True
 
     def finish(self, success):
         """Display log along with progress and go to the idle state.
-- 
1.7.11.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v2 2/2] tools: moveconfig: Add a new --git-ref option
  2016-06-02  3:30 ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Joe Hershberger
@ 2016-06-02  3:30   ` Joe Hershberger
  2016-06-08  2:56     ` Masahiro Yamada
  2016-06-10 19:53     ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Joe Hershberger
  2016-06-08  2:47   ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Masahiro Yamada
  1 sibling, 2 replies; 19+ messages in thread
From: Joe Hershberger @ 2016-06-02  3:30 UTC (permalink / raw)
  To: u-boot

This option allows the 'make *_defconfig' step to run against a former
repo state, while the savedefconfig step runs against the current repo
state. This is convenient for the case where something in the Kconfig
has changed such that the defconfig is no longer complete with the new
Kconfigs. This feature allows the .config to be built assuming those old
Kconfigs, but then savedefconfig based on the new state of the Kconfigs.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

Changes in v2:
- Stop reusing functions from patman
- Rebase on top of the latest moveconfig series

 tools/moveconfig.py | 101 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 84 insertions(+), 17 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 01350ce..7d9d31e 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -143,6 +143,10 @@ Available options
    Specify the number of threads to run simultaneously.  If not specified,
    the number of threads is the same as the number of CPU cores.
 
+ -r, --git-ref
+   Specify the git ref to clone for the make *_defconfig step. If unspecified
+   use the CWD.
+
  -v, --verbose
    Show any build errors as boards are built
 
@@ -412,6 +416,9 @@ class KconfigParser:
         self.options = options
         self.dotconfig = os.path.join(build_dir, '.config')
         self.autoconf = os.path.join(build_dir, 'include', 'autoconf.mk')
+        if options.git_ref:
+            self.autoconf_orig = os.path.join(build_dir, 'include',
+                                              'autoconf.orig.mk')
         self.config_autoconf = os.path.join(build_dir, 'include', 'config',
                                             'auto.conf')
         self.defconfig = os.path.join(build_dir, 'defconfig')
@@ -464,14 +471,6 @@ class KconfigParser:
         """
         not_set = '# %s is not set' % config
 
-        for line in dotconfig_lines:
-            line = line.rstrip()
-            if line.startswith(config + '=') or line == not_set:
-                old_val = line
-                break
-        else:
-            return (ACTION_NO_ENTRY, config)
-
         for line in autoconf_lines:
             line = line.rstrip()
             if line.startswith(config + '='):
@@ -480,6 +479,14 @@ class KconfigParser:
         else:
             new_val = not_set
 
+        for line in dotconfig_lines:
+            line = line.rstrip()
+            if line.startswith(config + '=') or line == not_set:
+                old_val = line
+                break
+        else:
+            return (ACTION_NO_ENTRY, config)
+
         if old_val == new_val:
             return (ACTION_NO_CHANGE, new_val)
 
@@ -515,8 +522,14 @@ class KconfigParser:
         with open(self.dotconfig) as f:
             dotconfig_lines = f.readlines()
 
-        with open(self.autoconf) as f:
-            autoconf_lines = f.readlines()
+
+        if self.options.git_ref:
+            # Pull the target value from the original autoconf.mk
+            with open(self.autoconf_orig) as f:
+                autoconf_lines = f.readlines()
+        else:
+            with open(self.autoconf) as f:
+                autoconf_lines = f.readlines()
 
         for config in self.configs:
             result = self.parse_one_config(config, dotconfig_lines,
@@ -585,7 +598,7 @@ class Slot:
     for faster processing.
     """
 
-    def __init__(self, configs, options, progress, devnull, make_cmd):
+    def __init__(self, configs, options, progress, devnull, make_cmd, defconfig_src_dir):
         """Create a new process slot.
 
         Arguments:
@@ -600,8 +613,11 @@ class Slot:
         self.build_dir = tempfile.mkdtemp()
         self.devnull = devnull
         self.make_cmd = (make_cmd, 'O=' + self.build_dir)
+        self.defconfig_src_dir = defconfig_src_dir
         self.parser = KconfigParser(configs, options, self.build_dir)
         self.state = STATE_IDLE
+        if self.options.git_ref:
+            self.use_git_ref = True
         self.failed_boards = []
 
     def __del__(self):
@@ -609,7 +625,7 @@ class Slot:
 
         This function makes sure the temporary directory is cleaned away
         even if Python suddenly dies due to error.  It should be done in here
-        because it is guranteed the destructor is always invoked when the
+        because it is guaranteed the destructor is always invoked when the
         instance of the class gets unreferenced.
 
         If the subprocess is still running, wait until it finishes.
@@ -638,6 +654,7 @@ class Slot:
         self.defconfig = defconfig
         self.state = STATE_INIT
         self.log = ''
+        self.use_git_ref = True
         return True
 
     def poll(self):
@@ -662,6 +679,9 @@ class Slot:
         if self.state == STATE_INIT:
             cmd = list(self.make_cmd)
             cmd.append(self.defconfig)
+            if self.options.git_ref and self.use_git_ref:
+                cmd.append('-C')
+                cmd.append(self.defconfig_src_dir)
             self.ps = subprocess.Popen(cmd, stdout=self.devnull,
                                        stderr=subprocess.PIPE)
             self.state = STATE_DEFCONFIG
@@ -692,12 +712,22 @@ class Slot:
                 cmd.append('CROSS_COMPILE=%s' % self.cross_compile)
             cmd.append('KCONFIG_IGNORE_DUPLICATES=1')
             cmd.append('include/config/auto.conf')
+            if self.options.git_ref and self.use_git_ref:
+                cmd.append('-C')
+                cmd.append(self.defconfig_src_dir)
             self.ps = subprocess.Popen(cmd, stdout=self.devnull,
                                        stderr=subprocess.PIPE)
             self.state = STATE_AUTOCONF
             return False
 
         if self.state == STATE_AUTOCONF:
+            if self.options.git_ref and self.use_git_ref:
+                shutil.move(os.path.join(self.build_dir, 'include/autoconf.mk'),
+                            os.path.join(self.build_dir, 'include/autoconf.orig.mk'))
+                self.state = STATE_INIT
+                self.use_git_ref = False
+                return False
+
             (updated, log) = self.parser.update_dotconfig()
             self.log += log
 
@@ -724,7 +754,7 @@ class Slot:
             updated = not filecmp.cmp(orig_defconfig, new_defconfig)
 
             if updated:
-                self.log += color_text(self.options.color, COLOR_LIGHT_GREEN,
+                self.log += color_text(self.options.color, COLOR_LIGHT_BLUE,
                                        "defconfig was updated.\n")
 
             if not self.options.dry_run and updated:
@@ -771,7 +801,7 @@ class Slots:
 
     """Controller of the array of subprocess slots."""
 
-    def __init__(self, configs, options, progress):
+    def __init__(self, configs, options, progress, defconfig_src_dir):
         """Create a new slots controller.
 
         Arguments:
@@ -785,7 +815,7 @@ class Slots:
         make_cmd = get_make_cmd()
         for i in range(options.jobs):
             self.slots.append(Slot(configs, options, progress, devnull,
-                                   make_cmd))
+                                   make_cmd, defconfig_src_dir))
 
     def add(self, defconfig):
         """Add a new subprocess if a vacant slot is found.
@@ -842,6 +872,24 @@ class Slots:
                 for board in failed_boards:
                     f.write(board + '\n')
 
+class WorkDir:
+    def __init__(self):
+        """Create a new working directory."""
+        self.work_dir = tempfile.mkdtemp()
+
+    def __del__(self):
+        """Delete the working directory
+
+        This function makes sure the temporary directory is cleaned away
+        even if Python suddenly dies due to error.  It should be done in here
+        because it is guaranteed the destructor is always invoked when the
+        instance of the class gets unreferenced.
+        """
+        shutil.rmtree(self.work_dir)
+
+    def get(self):
+        return self.work_dir
+
 def move_config(configs, options):
     """Move config options to defconfig files.
 
@@ -853,11 +901,28 @@ def move_config(configs, options):
         if options.force_sync:
             print 'No CONFIG is specified. You are probably syncing defconfigs.',
         else:
-            print 'Neigher CONFIG nor --force-sync is specified. Nothing will happen.',
+            print 'Neither CONFIG nor --force-sync is specified. Nothing will happen.',
     else:
         print 'Move ' + ', '.join(configs),
     print '(jobs: %d)\n' % options.jobs
 
+    defconfig_src_dir = ''
+
+    if options.git_ref:
+        work_dir = WorkDir()
+        defconfig_src_dir = work_dir.get()
+        cwd = os.getcwd()
+        print "Cloning git repo for 'make *_defconfig' step..."
+	subprocess.check_output(['git', 'clone', cwd, '.'], \
+				cwd=defconfig_src_dir)
+        print "Checkout '%s' to find original configs." % \
+            subprocess.check_output(['git', 'rev-parse', '--short', \
+				    options.git_ref]).strip()
+        os.chdir(defconfig_src_dir)
+	subprocess.check_output(['git', 'checkout', options.git_ref],
+                                stderr=subprocess.STDOUT)
+        os.chdir(cwd)
+
     if options.defconfigs:
         defconfigs = [line.strip() for line in open(options.defconfigs)]
         for i, defconfig in enumerate(defconfigs):
@@ -875,7 +940,7 @@ def move_config(configs, options):
                 defconfigs.append(os.path.join(dirpath, filename))
 
     progress = Progress(len(defconfigs))
-    slots = Slots(configs, options, progress)
+    slots = Slots(configs, options, progress, defconfig_src_dir)
 
     # Main loop to process defconfig files:
     #  Add a new subprocess into a vacant slot.
@@ -917,6 +982,8 @@ def main():
                       help='only cleanup the headers')
     parser.add_option('-j', '--jobs', type='int', default=cpu_count,
                       help='the number of jobs to run simultaneously')
+    parser.add_option('-r', '--git-ref', type='string',
+                      help='the git ref to clone for the make *_defconfig step')
     parser.add_option('-v', '--verbose', action='store_true', default=False,
                       help='show any build errors as boards are built')
     parser.usage += ' CONFIG ...'
-- 
1.7.11.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear
  2016-06-02  3:30 ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Joe Hershberger
  2016-06-02  3:30   ` [U-Boot] [PATCH v2 2/2] tools: moveconfig: Add a new --git-ref option Joe Hershberger
@ 2016-06-08  2:47   ` Masahiro Yamada
  1 sibling, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-08  2:47 UTC (permalink / raw)
  To: u-boot

Hi Joe.


2016-06-02 12:30 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
> Make the processing of a slot more linear code compared to how it
> executes.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>

This patch moves "make defconfig"
from the .add() method to the .poll() method,
but it did not update the comment block.

So, the comment below does not match the code any more.

 def add(self, defconfig):
     """Assign a new subprocess for defconfig and add it to the slot.


But, anyway I do not like to split
the current STATE_IDLE into STATE_IDLE and STATE_INIT.

See my comments in 2/2.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v2 2/2] tools: moveconfig: Add a new --git-ref option
  2016-06-02  3:30   ` [U-Boot] [PATCH v2 2/2] tools: moveconfig: Add a new --git-ref option Joe Hershberger
@ 2016-06-08  2:56     ` Masahiro Yamada
  2016-06-10 19:53     ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Joe Hershberger
  1 sibling, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-08  2:56 UTC (permalink / raw)
  To: u-boot

Hi Joe,


2016-06-02 12:30 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
> This option allows the 'make *_defconfig' step to run against a former
> repo state, while the savedefconfig step runs against the current repo
> state. This is convenient for the case where something in the Kconfig
> has changed such that the defconfig is no longer complete with the new
> Kconfigs. This feature allows the .config to be built assuming those old
> Kconfigs, but then savedefconfig based on the new state of the Kconfigs.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>


Basically, your idea seems cool,
but please improve the implementation.

It looks like your patch includes various noises (more changes than needed),
and things are getting complex.





> @@ -412,6 +416,9 @@ class KconfigParser:
>          self.options = options
>          self.dotconfig = os.path.join(build_dir, '.config')
>          self.autoconf = os.path.join(build_dir, 'include', 'autoconf.mk')
> +        if options.git_ref:
> +            self.autoconf_orig = os.path.join(build_dir, 'include',
> +                                              'autoconf.orig.mk')


This change is not needed.  (see below)



>          self.config_autoconf = os.path.join(build_dir, 'include', 'config',
>                                              'auto.conf')
>          self.defconfig = os.path.join(build_dir, 'defconfig')
> @@ -464,14 +471,6 @@ class KconfigParser:
>          """
>          not_set = '# %s is not set' % config
>
> -        for line in dotconfig_lines:
> -            line = line.rstrip()
> -            if line.startswith(config + '=') or line == not_set:
> -                old_val = line
> -                break
> -        else:
> -            return (ACTION_NO_ENTRY, config)
> -
>          for line in autoconf_lines:
>              line = line.rstrip()
>              if line.startswith(config + '='):
> @@ -480,6 +479,14 @@ class KconfigParser:
>          else:
>              new_val = not_set
>
> +        for line in dotconfig_lines:
> +            line = line.rstrip()
> +            if line.startswith(config + '=') or line == not_set:
> +                old_val = line
> +                break
> +        else:
> +            return (ACTION_NO_ENTRY, config)
> +

Why did you move this hunk?




>          if old_val == new_val:
>              return (ACTION_NO_CHANGE, new_val)
>
> @@ -515,8 +522,14 @@ class KconfigParser:
>          with open(self.dotconfig) as f:
>              dotconfig_lines = f.readlines()
>
> -        with open(self.autoconf) as f:
> -            autoconf_lines = f.readlines()
> +
> +        if self.options.git_ref:
> +            # Pull the target value from the original autoconf.mk
> +            with open(self.autoconf_orig) as f:
> +                autoconf_lines = f.readlines()
> +        else:
> +            with open(self.autoconf) as f:
> +                autoconf_lines = f.readlines()

Unneeded.  (see below)


>          for config in self.configs:
>              result = self.parse_one_config(config, dotconfig_lines,
> @@ -585,7 +598,7 @@ class Slot:
>      for faster processing.
>      """
>
> -    def __init__(self, configs, options, progress, devnull, make_cmd):
> +    def __init__(self, configs, options, progress, devnull, make_cmd, defconfig_src_dir):
>          """Create a new process slot.
>
>          Arguments:

In this v2 approach, defconfig occurs twice;
first against the cwd tree, 2nd against the cloned-tree.
So, "defconfig_src_dir" does not best-describe the behavior, I think.
Could you rename "defconfig_src_dir" to something more suitable?
Maybe, "ref_src_dir" or something?


Also, when you add a new argument,
please add a comment to "Arguments:"



> @@ -600,8 +613,11 @@ class Slot:
>          self.build_dir = tempfile.mkdtemp()
>          self.devnull = devnull
>          self.make_cmd = (make_cmd, 'O=' + self.build_dir)
> +        self.defconfig_src_dir = defconfig_src_dir

Please consider renaming it.

>          self.parser = KconfigParser(configs, options, self.build_dir)
>          self.state = STATE_IDLE
> +        if self.options.git_ref:
> +            self.use_git_ref = True

This is not needed because it is always
initialized in the add() method.


>          self.failed_boards = []
>
>      def __del__(self):
> @@ -609,7 +625,7 @@ class Slot:
>
>          This function makes sure the temporary directory is cleaned away
>          even if Python suddenly dies due to error.  It should be done in here
> -        because it is guranteed the destructor is always invoked when the
> +        because it is guaranteed the destructor is always invoked when the
>          instance of the class gets unreferenced.
>
>          If the subprocess is still running, wait until it finishes.
> @@ -638,6 +654,7 @@ class Slot:
>          self.defconfig = defconfig
>          self.state = STATE_INIT
>          self.log = ''
> +        self.use_git_ref = True

Setting always use_git_ref to True seems odd.

Maybe, can you change it like this?

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



>          return True
>
>      def poll(self):
> @@ -662,6 +679,9 @@ class Slot:
>          if self.state == STATE_INIT:
>              cmd = list(self.make_cmd)
>              cmd.append(self.defconfig)
> +            if self.options.git_ref and self.use_git_ref:

With my suggestion above, checking self.use_git_ref should be enough.

               if self.use_git_ref:


> +                cmd.append('-C')
> +                cmd.append(self.defconfig_src_dir)
>              self.ps = subprocess.Popen(cmd, stdout=self.devnull,
>                                         stderr=subprocess.PIPE)
>              self.state = STATE_DEFCONFIG
> @@ -692,12 +712,22 @@ class Slot:
>                  cmd.append('CROSS_COMPILE=%s' % self.cross_compile)
>              cmd.append('KCONFIG_IGNORE_DUPLICATES=1')
>              cmd.append('include/config/auto.conf')
> +            if self.options.git_ref and self.use_git_ref:


Ditto.

                if self.use_git_ref:




> +                cmd.append('-C')
> +                cmd.append(self.defconfig_src_dir)
>              self.ps = subprocess.Popen(cmd, stdout=self.devnull,
>                                         stderr=subprocess.PIPE)
>              self.state = STATE_AUTOCONF
>              return False
>
>          if self.state == STATE_AUTOCONF:
> +            if self.options.git_ref and self.use_git_ref:
> +                shutil.move(os.path.join(self.build_dir, 'include/autoconf.mk'),
> +                            os.path.join(self.build_dir, 'include/autoconf.orig.mk'))
> +                self.state = STATE_INIT
> +                self.use_git_ref = False
> +                return False
> +
>              (updated, log) = self.parser.update_dotconfig()
>              self.log += log

As far as I understood, if -r options is specified,
the state moves like this.

(1) STATE_IDLE
(2) STATE_INIT
(3) STATE_DEFCONFIG
(4) STATE_AUTOCONF
(5) STATE_INIT (2nd)
(6) STATE_DEFCONFIG (2nd)
(7) STATE_AUTOCONF (2nd)
(8) STATE_SAVEDEFCONFIG


But, is the 2nd autoconf necessary?
We only want to use autoconf.mk from the first autoconf.
The second one is just harmful; it overwrites autoconf.mk we want to parse.

If the 2nd one is skipped, we do not need to copy
it to include/autoconf.orig.mk

I understand why you did so.
The state transition is getting very complicated.

After pondering on it for a while,
I decided splitting code into helper methods might get the
situation better.

Please see my this follow-up patch.
http://patchwork.ozlabs.org/patch/631921/


With the patch applied, the poll() method is like this,


        if self.ps.poll() != 0:
            self.handle_error()
        elif self.state == STATE_DEFCONFIG:
            self.do_autoconf()
        elif self.state == STATE_AUTOCONF:
            self.do_savedefconfig()
        elif self.state == STATE_SAVEDEFCONFIG:
            self.update_defconfig()
        else:
            sys.exit("Internal Error. This should not happen.")




-r option might be implemented like this:


        if self.ps.poll() != 0:
            self.handle_error()
        elif self.state == STATE_DEFCONFIG:
            if self.options.git_ref and not self.use_git_ref:
                self.do_savedefconfig()
            else:
                self.do_autoconf()
        elif self.state == STATE_AUTOCONF:
            if self.use_git_ref:
                self.use_git_ref = False
                self.do_defconfig
            else:
                self.do_savedefconfig()
        elif self.state == STATE_SAVEDEFCONFIG:
            self.update_defconfig()
        else:
            sys.exit("Internal Error. This should not happen.")






> @@ -724,7 +754,7 @@ class Slot:
>              updated = not filecmp.cmp(orig_defconfig, new_defconfig)
>
>              if updated:
> -                self.log += color_text(self.options.color, COLOR_LIGHT_GREEN,
> +                self.log += color_text(self.options.color, COLOR_LIGHT_BLUE,
>                                         "defconfig was updated.\n")

Unrelated change.

You should send a separate patch if you want to change it.




> @@ -853,11 +901,28 @@ def move_config(configs, options):
>          if options.force_sync:
>              print 'No CONFIG is specified. You are probably syncing defconfigs.',
>          else:
> -            print 'Neigher CONFIG nor --force-sync is specified. Nothing will happen.',
> +            print 'Neither CONFIG nor --force-sync is specified. Nothing will happen.',
>      else:
>          print 'Move ' + ', '.join(configs),
>      print '(jobs: %d)\n' % options.jobs

I will fix this typo in my patch.

Please drop this hunk when you send a new patch.



> +    defconfig_src_dir = ''
> +
> +    if options.git_ref:
> +        work_dir = WorkDir()
> +        defconfig_src_dir = work_dir.get()
> +        cwd = os.getcwd()
> +        print "Cloning git repo for 'make *_defconfig' step..."
> +       subprocess.check_output(['git', 'clone', cwd, '.'], \
> +                               cwd=defconfig_src_dir)
> +        print "Checkout '%s' to find original configs." % \
> +            subprocess.check_output(['git', 'rev-parse', '--short', \
> +                                   options.git_ref]).strip()
> +        os.chdir(defconfig_src_dir)
> +       subprocess.check_output(['git', 'checkout', options.git_ref],
> +                                stderr=subprocess.STDOUT)
> +        os.chdir(cwd)
> +

Please use cmd= option instead of os.chdir()

  subprocess.check_output(['git', 'checkout', options.git_ref],
                           stderr=subprocess.STDOUT,
                           cwd=defconfig_src_dir)






To sum up,

Apply my series without 11/21,
then apply  http://patchwork.ozlabs.org/patch/631921/,
then could you consider rebasing your 2/2 on top of that?


Thanks,

-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo
  2016-06-02  3:30   ` [U-Boot] [PATCH v2 2/2] tools: moveconfig: Add a new --git-ref option Joe Hershberger
  2016-06-08  2:56     ` Masahiro Yamada
@ 2016-06-10 19:53     ` Joe Hershberger
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig Joe Hershberger
                         ` (3 more replies)
  1 sibling, 4 replies; 19+ messages in thread
From: Joe Hershberger @ 2016-06-10 19:53 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

Changes in v3: None
Changes in v2: None

 tools/moveconfig.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index d9e88f2..2fdc331 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -608,7 +608,7 @@ class Slot:
 
         This function makes sure the temporary directory is cleaned away
         even if Python suddenly dies due to error.  It should be done in here
-        because it is guranteed the destructor is always invoked when the
+        because it is guaranteed the destructor is always invoked when the
         instance of the class gets unreferenced.
 
         If the subprocess is still running, wait until it finishes.
-- 
1.7.11.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig
  2016-06-10 19:53     ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Joe Hershberger
@ 2016-06-10 19:53       ` Joe Hershberger
  2016-06-11 15:06         ` Masahiro Yamada
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 3/4] tools: moveconfig: Fix bug in make Slot.poll() Joe Hershberger
                         ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Joe Hershberger @ 2016-06-10 19:53 UTC (permalink / raw)
  To: u-boot

The old color blends in with similar messages and makes them not stand
out.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

Changes in v3: None
Changes in v2: None

 tools/moveconfig.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 2fdc331..4832b86 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -742,7 +742,7 @@ class Slot:
         updated = not filecmp.cmp(orig_defconfig, new_defconfig)
 
         if updated:
-            self.log += color_text(self.options.color, COLOR_LIGHT_GREEN,
+            self.log += color_text(self.options.color, COLOR_LIGHT_BLUE,
                                    "defconfig was updated.\n")
 
         if not self.options.dry_run and updated:
-- 
1.7.11.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 3/4] tools: moveconfig: Fix bug in make Slot.poll()...
  2016-06-10 19:53     ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Joe Hershberger
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig Joe Hershberger
@ 2016-06-10 19:53       ` Joe Hershberger
  2016-06-11 15:14         ` Masahiro Yamada
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 4/4] tools: moveconfig: Add a new --git-ref option Joe Hershberger
  2016-06-11 15:05       ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Masahiro Yamada
  3 siblings, 1 reply; 19+ messages in thread
From: Joe Hershberger @ 2016-06-10 19:53 UTC (permalink / raw)
  To: u-boot

When this was moved out of add(), it should have started using 'self.'

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

Changes in v3: None
Changes in v2: None

 tools/moveconfig.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 4832b86..7d1e1ea 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -688,7 +688,7 @@ class Slot:
         """Run 'make <board>_defconfig' to create the .config file."""
 
         cmd = list(self.make_cmd)
-        cmd.append(defconfig)
+        cmd.append(self.defconfig)
         self.ps = subprocess.Popen(cmd, stdout=self.devnull,
                                    stderr=subprocess.PIPE)
         self.state = STATE_DEFCONFIG
-- 
1.7.11.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 4/4] tools: moveconfig: Add a new --git-ref option
  2016-06-10 19:53     ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Joe Hershberger
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig Joe Hershberger
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 3/4] tools: moveconfig: Fix bug in make Slot.poll() Joe Hershberger
@ 2016-06-10 19:53       ` Joe Hershberger
  2016-06-11 15:09         ` Masahiro Yamada
  2016-06-11 15:05       ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Masahiro Yamada
  3 siblings, 1 reply; 19+ messages in thread
From: Joe Hershberger @ 2016-06-10 19:53 UTC (permalink / raw)
  To: u-boot

This option allows the 'make autoconf.mk' step to run against a former
repo state, while the savedefconfig step runs against the current repo
state. This is convenient for the case where something in the Kconfig
has changed such that the defconfig is no longer complete with the new
Kconfigs. This feature allows the .config to be built assuming those old
Kconfigs, but then savedefconfig based on the new state of the Kconfigs.

If in doubt, always specify this switch. It will always do the right
thing even if not required, but if it was required and you don't use it,
the moved configs will be incorrect. When not using this switch, you
must very carefully evaluate that all moved configs are correct.

Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---

Changes in v3:
- Rebase on top of 'tools: moveconfig: make Slot.poll() more readable with helper methods'
- Simplify implementation

Changes in v2:
- Stop reusing functions from patman
- Rebase on top of the latest moveconfig series

 tools/moveconfig.py | 74 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 6 deletions(-)

diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index 7d1e1ea..fe37645 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -143,6 +143,14 @@ Available options
    Specify the number of threads to run simultaneously.  If not specified,
    the number of threads is the same as the number of CPU cores.
 
+ -r, --git-ref
+   Specify the git ref to clone for building the autoconf.mk. If unspecified
+   use the CWD. This is useful for when changes to the Kconfig affect the
+   default values and you want to capture the state of the defconfig from
+   before that change was in effect. If in doubt, specify a ref pre-Kconfig
+   changes (use HEAD if Kconfig changes are not committed). Worst case it will
+   take a bit longer to run, but will always do the right thing.
+
  -v, --verbose
    Show any build errors as boards are built
 
@@ -584,7 +592,7 @@ class Slot:
     for faster processing.
     """
 
-    def __init__(self, configs, options, progress, devnull, make_cmd):
+    def __init__(self, configs, options, progress, devnull, make_cmd, reference_src_dir):
         """Create a new process slot.
 
         Arguments:
@@ -593,12 +601,15 @@ class Slot:
           progress: A progress indicator.
           devnull: A file object of '/dev/null'.
           make_cmd: command name of GNU Make.
+          reference_src_dir: Determine the true starting config state from this
+                             source tree.
         """
         self.options = options
         self.progress = progress
         self.build_dir = tempfile.mkdtemp()
         self.devnull = devnull
         self.make_cmd = (make_cmd, 'O=' + self.build_dir)
+        self.reference_src_dir = reference_src_dir
         self.parser = KconfigParser(configs, options, self.build_dir)
         self.state = STATE_IDLE
         self.failed_boards = []
@@ -636,6 +647,7 @@ class Slot:
 
         self.defconfig = defconfig
         self.log = ''
+        self.use_git_ref = True if self.options.git_ref else False
         self.do_defconfig()
         return True
 
@@ -664,9 +676,16 @@ class Slot:
         if self.ps.poll() != 0:
             self.handle_error()
         elif self.state == STATE_DEFCONFIG:
-            self.do_autoconf()
+            if self.options.git_ref and not self.use_git_ref:
+                self.do_savedefconfig()
+            else:
+                self.do_autoconf()
         elif self.state == STATE_AUTOCONF:
-            self.do_savedefconfig()
+            if self.use_git_ref:
+                self.use_git_ref = False
+                self.do_defconfig()
+            else:
+                self.do_savedefconfig()
         elif self.state == STATE_SAVEDEFCONFIG:
             self.update_defconfig()
         else:
@@ -689,6 +708,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)
         self.state = STATE_DEFCONFIG
@@ -708,6 +730,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)
         self.state = STATE_AUTOCONF
@@ -784,13 +809,15 @@ class Slots:
 
     """Controller of the array of subprocess slots."""
 
-    def __init__(self, configs, options, progress):
+    def __init__(self, configs, options, progress, reference_src_dir):
         """Create a new slots controller.
 
         Arguments:
           configs: A list of CONFIGs to move.
           options: option flags.
           progress: A progress indicator.
+          reference_src_dir: Determine the true starting config state from this
+                             source tree.
         """
         self.options = options
         self.slots = []
@@ -798,7 +825,7 @@ class Slots:
         make_cmd = get_make_cmd()
         for i in range(options.jobs):
             self.slots.append(Slot(configs, options, progress, devnull,
-                                   make_cmd))
+                                   make_cmd, reference_src_dir))
 
     def add(self, defconfig):
         """Add a new subprocess if a vacant slot is found.
@@ -855,6 +882,24 @@ class Slots:
                 for board in failed_boards:
                     f.write(board + '\n')
 
+class WorkDir:
+    def __init__(self):
+        """Create a new working directory."""
+        self.work_dir = tempfile.mkdtemp()
+
+    def __del__(self):
+        """Delete the working directory
+
+        This function makes sure the temporary directory is cleaned away
+        even if Python suddenly dies due to error.  It should be done in here
+        because it is guaranteed the destructor is always invoked when the
+        instance of the class gets unreferenced.
+        """
+        shutil.rmtree(self.work_dir)
+
+    def get(self):
+        return self.work_dir
+
 def move_config(configs, options):
     """Move config options to defconfig files.
 
@@ -871,6 +916,21 @@ def move_config(configs, options):
         print 'Move ' + ', '.join(configs),
     print '(jobs: %d)\n' % options.jobs
 
+    reference_src_dir = ''
+
+    if options.git_ref:
+        work_dir = WorkDir()
+        reference_src_dir = work_dir.get()
+        print "Cloning git repo to a separate work directory..."
+        subprocess.check_output(['git', 'clone', os.getcwd(), '.'],
+                                cwd=reference_src_dir)
+        print "Checkout '%s' to build the original autoconf.mk." % \
+            subprocess.check_output(['git', 'rev-parse', '--short',
+                                    options.git_ref]).strip()
+        subprocess.check_output(['git', 'checkout', options.git_ref],
+                                stderr=subprocess.STDOUT,
+                                cwd=reference_src_dir)
+
     if options.defconfigs:
         defconfigs = [line.strip() for line in open(options.defconfigs)]
         for i, defconfig in enumerate(defconfigs):
@@ -888,7 +948,7 @@ def move_config(configs, options):
                 defconfigs.append(os.path.join(dirpath, filename))
 
     progress = Progress(len(defconfigs))
-    slots = Slots(configs, options, progress)
+    slots = Slots(configs, options, progress, reference_src_dir)
 
     # Main loop to process defconfig files:
     #  Add a new subprocess into a vacant slot.
@@ -930,6 +990,8 @@ def main():
                       help='only cleanup the headers')
     parser.add_option('-j', '--jobs', type='int', default=cpu_count,
                       help='the number of jobs to run simultaneously')
+    parser.add_option('-r', '--git-ref', type='string',
+                      help='the git ref to clone for building the autoconf.mk')
     parser.add_option('-v', '--verbose', action='store_true', default=False,
                       help='show any build errors as boards are built')
     parser.usage += ' CONFIG ...'
-- 
1.7.11.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo
  2016-06-10 19:53     ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Joe Hershberger
                         ` (2 preceding siblings ...)
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 4/4] tools: moveconfig: Add a new --git-ref option Joe Hershberger
@ 2016-06-11 15:05       ` Masahiro Yamada
  2016-06-12 22:31         ` Masahiro Yamada
  3 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-11 15:05 UTC (permalink / raw)
  To: u-boot

2016-06-11 4:53 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---


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



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig Joe Hershberger
@ 2016-06-11 15:06         ` Masahiro Yamada
  2016-06-12 22:31           ` Masahiro Yamada
  0 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-11 15:06 UTC (permalink / raw)
  To: u-boot

2016-06-11 4:53 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
> The old color blends in with similar messages and makes them not stand
> out.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>

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


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 4/4] tools: moveconfig: Add a new --git-ref option
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 4/4] tools: moveconfig: Add a new --git-ref option Joe Hershberger
@ 2016-06-11 15:09         ` Masahiro Yamada
  2016-06-12 22:31           ` Masahiro Yamada
  0 siblings, 1 reply; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-11 15:09 UTC (permalink / raw)
  To: u-boot

2016-06-11 4:53 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
> This option allows the 'make autoconf.mk' step to run against a former
> repo state, while the savedefconfig step runs against the current repo
> state. This is convenient for the case where something in the Kconfig
> has changed such that the defconfig is no longer complete with the new
> Kconfigs. This feature allows the .config to be built assuming those old
> Kconfigs, but then savedefconfig based on the new state of the Kconfigs.
>
> If in doubt, always specify this switch. It will always do the right
> thing even if not required, but if it was required and you don't use it,
> the moved configs will be incorrect. When not using this switch, you
> must very carefully evaluate that all moved configs are correct.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>


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


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 3/4] tools: moveconfig: Fix bug in make Slot.poll()...
  2016-06-10 19:53       ` [U-Boot] [PATCH v3 3/4] tools: moveconfig: Fix bug in make Slot.poll() Joe Hershberger
@ 2016-06-11 15:14         ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-11 15:14 UTC (permalink / raw)
  To: u-boot

2016-06-11 4:53 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
> When this was moved out of add(), it should have started using 'self.'
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---

I thought I fixed this bug before I sent the patch,
but somehow the patch before the fix was sent.  Probably due to my
mis-operation.

I will squash this into my patch.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo
  2016-06-11 15:05       ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Masahiro Yamada
@ 2016-06-12 22:31         ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-12 22:31 UTC (permalink / raw)
  To: u-boot

2016-06-12 0:05 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2016-06-11 4:53 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>> ---
>
>
> Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>


Applied, thanks!

-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig
  2016-06-11 15:06         ` Masahiro Yamada
@ 2016-06-12 22:31           ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-12 22:31 UTC (permalink / raw)
  To: u-boot

2016-06-12 0:06 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2016-06-11 4:53 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
>> The old color blends in with similar messages and makes them not stand
>> out.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
> Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>

Applied, thanks!


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [U-Boot] [PATCH v3 4/4] tools: moveconfig: Add a new --git-ref option
  2016-06-11 15:09         ` Masahiro Yamada
@ 2016-06-12 22:31           ` Masahiro Yamada
  0 siblings, 0 replies; 19+ messages in thread
From: Masahiro Yamada @ 2016-06-12 22:31 UTC (permalink / raw)
  To: u-boot

2016-06-12 0:09 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2016-06-11 4:53 GMT+09:00 Joe Hershberger <joe.hershberger@ni.com>:
>> This option allows the 'make autoconf.mk' step to run against a former
>> repo state, while the savedefconfig step runs against the current repo
>> state. This is convenient for the case where something in the Kconfig
>> has changed such that the defconfig is no longer complete with the new
>> Kconfigs. This feature allows the .config to be built assuming those old
>> Kconfigs, but then savedefconfig based on the new state of the Kconfigs.
>>
>> If in doubt, always specify this switch. It will always do the right
>> thing even if not required, but if it was required and you don't use it,
>> the moved configs will be incorrect. When not using this switch, you
>> must very carefully evaluate that all moved configs are correct.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>
>
> Reviewed-by: Masahiro Yamada <yamada.masahiro@socionext.com>


Applied, thanks!



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-06-12 22:31 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-29 21:23 [U-Boot] [PATCH] moveconfig: Add a new --git-ref option Joe Hershberger
2015-06-02 13:42 ` Joe Hershberger
2015-06-05  4:54 ` Masahiro Yamada
2015-06-10 14:16   ` Joe Hershberger
2016-06-02  3:30 ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Joe Hershberger
2016-06-02  3:30   ` [U-Boot] [PATCH v2 2/2] tools: moveconfig: Add a new --git-ref option Joe Hershberger
2016-06-08  2:56     ` Masahiro Yamada
2016-06-10 19:53     ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Joe Hershberger
2016-06-10 19:53       ` [U-Boot] [PATCH v3 2/4] tools: moveconfig: New color used for changed defconfig Joe Hershberger
2016-06-11 15:06         ` Masahiro Yamada
2016-06-12 22:31           ` Masahiro Yamada
2016-06-10 19:53       ` [U-Boot] [PATCH v3 3/4] tools: moveconfig: Fix bug in make Slot.poll() Joe Hershberger
2016-06-11 15:14         ` Masahiro Yamada
2016-06-10 19:53       ` [U-Boot] [PATCH v3 4/4] tools: moveconfig: Add a new --git-ref option Joe Hershberger
2016-06-11 15:09         ` Masahiro Yamada
2016-06-12 22:31           ` Masahiro Yamada
2016-06-11 15:05       ` [U-Boot] [PATCH v3 1/4] tools: moveconfig: Fix another typo Masahiro Yamada
2016-06-12 22:31         ` Masahiro Yamada
2016-06-08  2:47   ` [U-Boot] [PATCH v2 1/2] tools: moveconfig: Make the slot processing more linear Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.