[U-Boot] [PATCH 0/7] tools/genboardscfg.py: various fixes and performance improvement

[U-Boot] [PATCH 0/7] tools/genboardscfg.py: various fixes and performance improvement

[Masahiro Yamada]

This series depends on the following prerequisites

http://patchwork.ozlabs.org/patch/380316/
http://patchwork.ozlabs.org/patch/376222/



Masahiro Yamada (7):
  tools/genboardscfg.py: ignore defconfigs starting with a dot
  tools/genboardscfg.py: be tolerant of missing MAINTAINERS
  tools/genboardscfg.py: be tolerant of insane Kconfig
  tools/genboardscfg.py: wait for unfinished subprocesses before
    error-out
  tools/genboardscfg.py: fix minor problems on termination
  tools/genboardscfg.py: check if the boards.cfg is up to date
  tools/genboardscfg.py: improve performance

 tools/genboardscfg.py | 278 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 204 insertions(+), 74 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH 1/7] tools/genboardscfg.py: ignore defconfigs starting with a dot

[Masahiro Yamada]

Kconfig in U-Boot creates a temporary file configs/.tmp_defconfig
during processing "make <board>_defconfig".  The temporary file
might be left over for some reasons.

Just in case, tools/genboardscfg.py should make sure to
not read such garbage files.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 tools/genboardscfg.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
index 2a05502..eb957ba 100755
--- a/tools/genboardscfg.py
+++ b/tools/genboardscfg.py
@@ -415,6 +415,8 @@ def __gen_boards_cfg(jobs):
     for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
         dirpath = dirpath[len(CONFIG_DIR) + 1:]
         for filename in fnmatch.filter(filenames, '*_defconfig'):
+            if fnmatch.fnmatch(filename, '.*'):
+                continue
             defconfigs.append(os.path.join(dirpath, filename))
 
     # Parse all the MAINTAINERS files
-- 
1.9.1

[U-Boot] [PATCH 1/7] tools/genboardscfg.py: ignore defconfigs starting with a dot

[Simon Glass]

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Kconfig in U-Boot creates a temporary file configs/.tmp_defconfig
> during processing "make <board>_defconfig".  The temporary file
> might be left over for some reasons.
>
> Just in case, tools/genboardscfg.py should make sure to
> not read such garbage files.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 2/7] tools/genboardscfg.py: be tolerant of missing MAINTAINERS

[Masahiro Yamada]

tools/genboardscfg.py expects all the boards have MAINTAINERS.
If someone adds a new board but misses to add its MAINTAINERS file,
tools/genboardscfg.py fails to generate the boards.cfg file.
It is annoying for the other developers.

This commit allows tools/genboardscfg.py to display warning messages
and continue processing even if some MAINTAINERS files are missing
or have broken formats.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 tools/genboardscfg.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
index eb957ba..03bd5b3 100755
--- a/tools/genboardscfg.py
+++ b/tools/genboardscfg.py
@@ -102,13 +102,19 @@ class MaintainersDatabase:
         Returns:
           Either 'Active' or 'Orphan'
         """
+        if not target in self.database:
+            print >> sys.stderr, "WARNING: no status info for '%s'" % target
+            return '-'
+
         tmp = self.database[target][0]
         if tmp.startswith('Maintained'):
             return 'Active'
         elif tmp.startswith('Orphan'):
             return 'Orphan'
         else:
-            print >> sys.stderr, 'Error: %s: unknown status' % tmp
+            print >> sys.stderr, ("WARNING: %s: unknown status for '%s'" %
+                                  (tmp, target))
+            return '-'
 
     def get_maintainers(self, target):
         """Return the maintainers of the given board.
@@ -116,6 +122,10 @@ class MaintainersDatabase:
         If the board has two or more maintainers, they are separated
         with colons.
         """
+        if not target in self.database:
+            print >> sys.stderr, "WARNING: no maintainers for '%s'" % target
+            return ''
+
         return ':'.join(self.database[target][1])
 
     def parse_file(self, file):
-- 
1.9.1

[U-Boot] [PATCH 2/7] tools/genboardscfg.py: be tolerant of missing MAINTAINERS

[Simon Glass]

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> tools/genboardscfg.py expects all the boards have MAINTAINERS.
> If someone adds a new board but misses to add its MAINTAINERS file,
> tools/genboardscfg.py fails to generate the boards.cfg file.
> It is annoying for the other developers.
>
> This commit allows tools/genboardscfg.py to display warning messages
> and continue processing even if some MAINTAINERS files are missing
> or have broken formats.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 3/7] tools/genboardscfg.py: be tolerant of insane Kconfig

[Masahiro Yamada]

The tools/genboardscfg.py expects all the Kconfig and defconfig are
written correctly.  Imagine someone accidentally has broken a board.
Error-out just for one broken board is annoying for the other
developers.  Let the tool skip insane boards and continue processing.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 tools/genboardscfg.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
index 03bd5b3..bdedb00 100755
--- a/tools/genboardscfg.py
+++ b/tools/genboardscfg.py
@@ -217,7 +217,10 @@ class DotConfigParser:
         # sanity check of '.config' file
         for field in self.must_fields:
             if not field in fields:
-                sys.exit('Error: %s is not defined in %s' % (field, defconfig))
+                print >> sys.stderr, (
+                    "WARNING: '%s' is not defined in '%s'. Skip." %
+                    (field, defconfig))
+                return
 
         # fix-up for aarch64 and tegra
         if fields['arch'] == 'arm' and 'cpu' in fields:
@@ -311,7 +314,11 @@ class Slot:
             return True
         if self.ps.poll() == None:
             return False
-        self.parser.parse(self.defconfig)
+        if self.ps.poll() == 0:
+            self.parser.parse(self.defconfig)
+        else:
+            print >> sys.stderr, ("WARNING: failed to process '%s'. skip." %
+                                  self.defconfig)
         self.occupied = False
         return True
 
-- 
1.9.1

[U-Boot] [PATCH 3/7] tools/genboardscfg.py: be tolerant of insane Kconfig

[Simon Glass]

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> The tools/genboardscfg.py expects all the Kconfig and defconfig are
> written correctly.  Imagine someone accidentally has broken a board.
> Error-out just for one broken board is annoying for the other
> developers.  Let the tool skip insane boards and continue processing.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 4/7] tools/genboardscfg.py: wait for unfinished subprocesses before error-out

[Masahiro Yamada]

When an error occurs or the program is terminated by the user
on the way, the destructer __del__ of class Slot is invoked and
the work directories are removed.

We have to make sure there are no subprocesses (in this case,
"make O=<work_dir> ...") using the work directories before
removing them.  Otherwise the subprocess spits a bunch of error
messages possibly causing more problems.  Perhaps some users
may get upset to see too many error messages.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 tools/genboardscfg.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
index bdedb00..9b3a9bf 100755
--- a/tools/genboardscfg.py
+++ b/tools/genboardscfg.py
@@ -280,6 +280,9 @@ class Slot:
 
     def __del__(self):
         """Delete the working directory"""
+        if not self.occupied:
+            while self.ps.poll() == None:
+                pass
         shutil.rmtree(self.build_dir)
 
     def add(self, defconfig):
-- 
1.9.1

[U-Boot] [PATCH 4/7] tools/genboardscfg.py: wait for unfinished subprocesses before error-out

[Simon Glass]

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> When an error occurs or the program is terminated by the user
> on the way, the destructer __del__ of class Slot is invoked and
> the work directories are removed.
>
> We have to make sure there are no subprocesses (in this case,
> "make O=<work_dir> ...") using the work directories before
> removing them.  Otherwise the subprocess spits a bunch of error
> messages possibly causing more problems.  Perhaps some users
> may get upset to see too many error messages.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 5/7] tools/genboardscfg.py: fix minor problems on termination

[Masahiro Yamada]

This tool deletes the incomplete boards.cfg
if it encounters an error or is is terminated by the user.

I notice some problems even though they rarely happen.

[1] The boards.cfg is removed if the program is terminated
during __gen_boards_cfg() function but before boards.cfg
is actually touched.  In this case, the previous boards.cfg
should be kept as it is.

[2] If an error occurs while deleting the incomplete boards.cfg,
the program throws another exception.  This hides the privious
exception and we will not be able to know the real cause.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 tools/genboardscfg.py | 156 +++++++++++++++++++++++++++++---------------------
 1 file changed, 90 insertions(+), 66 deletions(-)

diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
index 9b3a9bf..13bb424 100755
--- a/tools/genboardscfg.py
+++ b/tools/genboardscfg.py
@@ -417,63 +417,95 @@ class Indicator:
         sys.stdout.write('\r' + msg)
         sys.stdout.flush()
 
-def __gen_boards_cfg(jobs):
-    """Generate boards.cfg file.
+class BoardsFileGenerator:
 
-    Arguments:
-      jobs: The number of jobs to run simultaneously
+    """Generator of boards.cfg."""
 
-    Note:
-      The incomplete boards.cfg is left over when an error (including 
-      the termination by the keyboard interrupt) occurs on the halfway.
-    """
-    check_top_directory()
-    print 'Generating %s ...  (jobs: %d)' % (BOARD_FILE, jobs)
-
-    # All the defconfig files to be processed
-    defconfigs = []
-    for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
-        dirpath = dirpath[len(CONFIG_DIR) + 1:]
-        for filename in fnmatch.filter(filenames, '*_defconfig'):
-            if fnmatch.fnmatch(filename, '.*'):
-                continue
-            defconfigs.append(os.path.join(dirpath, filename))
-
-    # Parse all the MAINTAINERS files
-    maintainers_database = MaintainersDatabase()
-    for (dirpath, dirnames, filenames) in os.walk('.'):
-        if 'MAINTAINERS' in filenames:
-            maintainers_database.parse_file(os.path.join(dirpath,
-                                                         'MAINTAINERS'))
-
-    # Output lines should be piped into the reformat tool
-    reformat_process = subprocess.Popen(REFORMAT_CMD, stdin=subprocess.PIPE,
-                                        stdout=open(BOARD_FILE, 'w'))
-    pipe = reformat_process.stdin
-    pipe.write(COMMENT_BLOCK)
-
-    indicator = Indicator(len(defconfigs))
-    slots = Slots(jobs, pipe, maintainers_database)
-
-    # Main loop to process defconfig files:
-    #  Add a new subprocess into a vacant slot.
-    #  Sleep if there is no available slot.
-    for defconfig in defconfigs:
-        while not slots.add(defconfig):
-            while not slots.available():
-                # No available slot: sleep for a while
-                time.sleep(SLEEP_TIME)
-        indicator.inc()
-
-    # wait until all the subprocesses finish
-    while not slots.empty():
-        time.sleep(SLEEP_TIME)
-    print ''
-
-    # wait until the reformat tool finishes
-    reformat_process.communicate()
-    if reformat_process.returncode != 0:
-        sys.exit('"%s" failed' % REFORMAT_CMD[0])
+    def __init__(self):
+        """Prepare basic things for generating boards.cfg."""
+        # All the defconfig files to be processed
+        defconfigs = []
+        for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
+            dirpath = dirpath[len(CONFIG_DIR) + 1:]
+            for filename in fnmatch.filter(filenames, '*_defconfig'):
+                if fnmatch.fnmatch(filename, '.*'):
+                    continue
+                defconfigs.append(os.path.join(dirpath, filename))
+        self.defconfigs = defconfigs
+        self.indicator = Indicator(len(defconfigs))
+
+        # Parse all the MAINTAINERS files
+        maintainers_database = MaintainersDatabase()
+        for (dirpath, dirnames, filenames) in os.walk('.'):
+            if 'MAINTAINERS' in filenames:
+                maintainers_database.parse_file(os.path.join(dirpath,
+                                                             'MAINTAINERS'))
+        self.maintainers_database = maintainers_database
+
+    def __del__(self):
+        """Delete the incomplete boards.cfg
+
+        This destructor deletes boards.cfg if the private member 'in_progress'
+        is defined as True.  The 'in_progress' member is set to True at the
+        beginning of the generate() method and set to False at its end.
+        So, in_progress==True means generating boards.cfg was terminated
+        on the way.
+        """
+
+        if hasattr(self, 'in_progress') and self.in_progress:
+            try:
+                os.remove(BOARD_FILE)
+            except OSError, exception:
+                # Ignore 'No such file or directory' error
+                if exception.errno != errno.ENOENT:
+                    raise
+            print 'Removed incomplete %s' % BOARD_FILE
+
+    def generate(self, jobs):
+        """Generate boards.cfg
+
+        This method sets the 'in_progress' member to True at the beginning
+        and sets it to False on success.  The boards.cfg should not be
+        touched before/after this method because 'in_progress' is used
+        to detect the incomplete boards.cfg.
+
+        Arguments:
+          jobs: The number of jobs to run simultaneously
+        """
+
+        self.in_progress = True
+        print 'Generating %s ...  (jobs: %d)' % (BOARD_FILE, jobs)
+
+        # Output lines should be piped into the reformat tool
+        reformat_process = subprocess.Popen(REFORMAT_CMD,
+                                            stdin=subprocess.PIPE,
+                                            stdout=open(BOARD_FILE, 'w'))
+        pipe = reformat_process.stdin
+        pipe.write(COMMENT_BLOCK)
+
+        slots = Slots(jobs, pipe, self.maintainers_database)
+
+        # Main loop to process defconfig files:
+        #  Add a new subprocess into a vacant slot.
+        #  Sleep if there is no available slot.
+        for defconfig in self.defconfigs:
+            while not slots.add(defconfig):
+                while not slots.available():
+                    # No available slot: sleep for a while
+                    time.sleep(SLEEP_TIME)
+            self.indicator.inc()
+
+        # wait until all the subprocesses finish
+        while not slots.empty():
+            time.sleep(SLEEP_TIME)
+        print ''
+
+        # wait until the reformat tool finishes
+        reformat_process.communicate()
+        if reformat_process.returncode != 0:
+            sys.exit('"%s" failed' % REFORMAT_CMD[0])
+
+        self.in_progress = False
 
 def gen_boards_cfg(jobs):
     """Generate boards.cfg file.
@@ -484,17 +516,9 @@ def gen_boards_cfg(jobs):
     Arguments:
       jobs: The number of jobs to run simultaneously
     """
-    try:
-        __gen_boards_cfg(jobs)
-    except:
-        # We should remove incomplete boards.cfg
-        try:
-            os.remove(BOARD_FILE)
-        except OSError, exception:
-            # Ignore 'No such file or directory' error
-            if exception.errno != errno.ENOENT:
-                raise
-        raise
+    check_top_directory()
+    generator = BoardsFileGenerator()
+    generator.generate(jobs)
 
 def main():
     parser = optparse.OptionParser()
-- 
1.9.1

[U-Boot] [PATCH 5/7] tools/genboardscfg.py: fix minor problems on termination

[Simon Glass]

HI Masahiro,

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> This tool deletes the incomplete boards.cfg
> if it encounters an error or is is terminated by the user.
>
> I notice some problems even though they rarely happen.
>
> [1] The boards.cfg is removed if the program is terminated
> during __gen_boards_cfg() function but before boards.cfg
> is actually touched.  In this case, the previous boards.cfg
> should be kept as it is.
>
> [2] If an error occurs while deleting the incomplete boards.cfg,
> the program throws another exception.  This hides the privious
> exception and we will not be able to know the real cause.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Acked-by: Simon Glass <sjg@chromium.org>

A few suggestions below (please ignore as you wish, they are not important)

> ---
>
>  tools/genboardscfg.py | 156 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 90 insertions(+), 66 deletions(-)
>
> diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
> index 9b3a9bf..13bb424 100755
> --- a/tools/genboardscfg.py
> +++ b/tools/genboardscfg.py
> @@ -417,63 +417,95 @@ class Indicator:
>          sys.stdout.write('\r' + msg)
>          sys.stdout.flush()
>
> -def __gen_boards_cfg(jobs):
> -    """Generate boards.cfg file.
> +class BoardsFileGenerator:
>
> -    Arguments:
> -      jobs: The number of jobs to run simultaneously
> +    """Generator of boards.cfg."""
>
> -    Note:
> -      The incomplete boards.cfg is left over when an error (including
> -      the termination by the keyboard interrupt) occurs on the halfway.
> -    """
> -    check_top_directory()
> -    print 'Generating %s ...  (jobs: %d)' % (BOARD_FILE, jobs)
> -
> -    # All the defconfig files to be processed
> -    defconfigs = []
> -    for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
> -        dirpath = dirpath[len(CONFIG_DIR) + 1:]
> -        for filename in fnmatch.filter(filenames, '*_defconfig'):
> -            if fnmatch.fnmatch(filename, '.*'):
> -                continue
> -            defconfigs.append(os.path.join(dirpath, filename))
> -
> -    # Parse all the MAINTAINERS files
> -    maintainers_database = MaintainersDatabase()
> -    for (dirpath, dirnames, filenames) in os.walk('.'):
> -        if 'MAINTAINERS' in filenames:
> -            maintainers_database.parse_file(os.path.join(dirpath,
> -                                                         'MAINTAINERS'))
> -
> -    # Output lines should be piped into the reformat tool
> -    reformat_process = subprocess.Popen(REFORMAT_CMD, stdin=subprocess.PIPE,
> -                                        stdout=open(BOARD_FILE, 'w'))
> -    pipe = reformat_process.stdin
> -    pipe.write(COMMENT_BLOCK)
> -
> -    indicator = Indicator(len(defconfigs))
> -    slots = Slots(jobs, pipe, maintainers_database)
> -
> -    # Main loop to process defconfig files:
> -    #  Add a new subprocess into a vacant slot.
> -    #  Sleep if there is no available slot.
> -    for defconfig in defconfigs:
> -        while not slots.add(defconfig):
> -            while not slots.available():
> -                # No available slot: sleep for a while
> -                time.sleep(SLEEP_TIME)
> -        indicator.inc()
> -
> -    # wait until all the subprocesses finish
> -    while not slots.empty():
> -        time.sleep(SLEEP_TIME)
> -    print ''
> -
> -    # wait until the reformat tool finishes
> -    reformat_process.communicate()
> -    if reformat_process.returncode != 0:
> -        sys.exit('"%s" failed' % REFORMAT_CMD[0])
> +    def __init__(self):
> +        """Prepare basic things for generating boards.cfg."""
> +        # All the defconfig files to be processed
> +        defconfigs = []
> +        for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
> +            dirpath = dirpath[len(CONFIG_DIR) + 1:]
> +            for filename in fnmatch.filter(filenames, '*_defconfig'):
> +                if fnmatch.fnmatch(filename, '.*'):
> +                    continue
> +                defconfigs.append(os.path.join(dirpath, filename))
> +        self.defconfigs = defconfigs
> +        self.indicator = Indicator(len(defconfigs))

I try to put an _ before private members to indicate that they should
not be used outside the class. But It is not particularly important -
just thought I'd mention it.

> +
> +        # Parse all the MAINTAINERS files
> +        maintainers_database = MaintainersDatabase()
> +        for (dirpath, dirnames, filenames) in os.walk('.'):
> +            if 'MAINTAINERS' in filenames:
> +                maintainers_database.parse_file(os.path.join(dirpath,
> +                                                             'MAINTAINERS'))
> +        self.maintainers_database = maintainers_database
> +
> +    def __del__(self):
> +        """Delete the incomplete boards.cfg
> +
> +        This destructor deletes boards.cfg if the private member 'in_progress'
> +        is defined as True.  The 'in_progress' member is set to True at the
> +        beginning of the generate() method and set to False at its end.
> +        So, in_progress==True means generating boards.cfg was terminated
> +        on the way.
> +        """
> +
> +        if hasattr(self, 'in_progress') and self.in_progress:

Would it be better to initialise in_progress to None in the constructor?

> +            try:
> +                os.remove(BOARD_FILE)
> +            except OSError, exception:
> +                # Ignore 'No such file or directory' error
> +                if exception.errno != errno.ENOENT:
> +                    raise
> +            print 'Removed incomplete %s' % BOARD_FILE
> +
> +    def generate(self, jobs):
> +        """Generate boards.cfg
> +
> +        This method sets the 'in_progress' member to True at the beginning
> +        and sets it to False on success.  The boards.cfg should not be
> +        touched before/after this method because 'in_progress' is used
> +        to detect the incomplete boards.cfg.
> +
> +        Arguments:
> +          jobs: The number of jobs to run simultaneously
> +        """
> +
> +        self.in_progress = True
> +        print 'Generating %s ...  (jobs: %d)' % (BOARD_FILE, jobs)
> +
> +        # Output lines should be piped into the reformat tool
> +        reformat_process = subprocess.Popen(REFORMAT_CMD,
> +                                            stdin=subprocess.PIPE,
> +                                            stdout=open(BOARD_FILE, 'w'))
> +        pipe = reformat_process.stdin
> +        pipe.write(COMMENT_BLOCK)
> +
> +        slots = Slots(jobs, pipe, self.maintainers_database)
> +
> +        # Main loop to process defconfig files:
> +        #  Add a new subprocess into a vacant slot.
> +        #  Sleep if there is no available slot.
> +        for defconfig in self.defconfigs:
> +            while not slots.add(defconfig):
> +                while not slots.available():
> +                    # No available slot: sleep for a while
> +                    time.sleep(SLEEP_TIME)
> +            self.indicator.inc()
> +
> +        # wait until all the subprocesses finish
> +        while not slots.empty():
> +            time.sleep(SLEEP_TIME)
> +        print ''
> +
> +        # wait until the reformat tool finishes
> +        reformat_process.communicate()
> +        if reformat_process.returncode != 0:
> +            sys.exit('"%s" failed' % REFORMAT_CMD[0])
> +
> +        self.in_progress = False
>
>  def gen_boards_cfg(jobs):
>      """Generate boards.cfg file.
> @@ -484,17 +516,9 @@ def gen_boards_cfg(jobs):
>      Arguments:
>        jobs: The number of jobs to run simultaneously
>      """
> -    try:
> -        __gen_boards_cfg(jobs)
> -    except:
> -        # We should remove incomplete boards.cfg
> -        try:
> -            os.remove(BOARD_FILE)
> -        except OSError, exception:
> -            # Ignore 'No such file or directory' error
> -            if exception.errno != errno.ENOENT:
> -                raise
> -        raise
> +    check_top_directory()
> +    generator = BoardsFileGenerator()
> +    generator.generate(jobs)
>
>  def main():
>      parser = optparse.OptionParser()
> --
> 1.9.1
>

Regards,
Simon

[U-Boot] [PATCH 5/7] tools/genboardscfg.py: fix minor problems on termination

[Masahiro Yamada]

Hi Simon,



On Wed, 20 Aug 2014 13:10:52 -0600
Simon Glass <sjg@chromium.org> wrote:

> I try to put an _ before private members to indicate that they should
> not be used outside the class. But It is not particularly important -
> just thought I'd mention it.


I will try my best to keep this in mind
when I send the next version.
(and when I write other Python scripts.)

But I do not have an enough motivation to do so for now.



> > +
> > +        # Parse all the MAINTAINERS files
> > +        maintainers_database = MaintainersDatabase()
> > +        for (dirpath, dirnames, filenames) in os.walk('.'):
> > +            if 'MAINTAINERS' in filenames:
> > +                maintainers_database.parse_file(os.path.join(dirpath,
> > +                                                             'MAINTAINERS'))
> > +        self.maintainers_database = maintainers_database
> > +
> > +    def __del__(self):
> > +        """Delete the incomplete boards.cfg
> > +
> > +        This destructor deletes boards.cfg if the private member 'in_progress'
> > +        is defined as True.  The 'in_progress' member is set to True at the
> > +        beginning of the generate() method and set to False at its end.
> > +        So, in_progress==True means generating boards.cfg was terminated
> > +        on the way.
> > +        """
> > +
> > +        if hasattr(self, 'in_progress') and self.in_progress:
> 
> Would it be better to initialise in_progress to None in the constructor?
> 


At first I thought of that.


If the constructor fails before setting in_progress to None,
the destructor is invoked with undefined in_progress.

Of course, it rarely (never) happens.
But I am trying to be as safe as possible.


Best Regards
Masahiro Yamada

[U-Boot] [PATCH 5/7] tools/genboardscfg.py: fix minor problems on termination

[Simon Glass]

Hi Masahiro,

On 20 August 2014 22:00, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
> On Wed, 20 Aug 2014 13:10:52 -0600
> Simon Glass <sjg@chromium.org> wrote:
>
>> I try to put an _ before private members to indicate that they should
>> not be used outside the class. But It is not particularly important -
>> just thought I'd mention it.
>
>
> I will try my best to keep this in mind
> when I send the next version.
> (and when I write other Python scripts.)
>
> But I do not have an enough motivation to do so for now.
>
>
>
>> > +
>> > +        # Parse all the MAINTAINERS files
>> > +        maintainers_database = MaintainersDatabase()
>> > +        for (dirpath, dirnames, filenames) in os.walk('.'):
>> > +            if 'MAINTAINERS' in filenames:
>> > +                maintainers_database.parse_file(os.path.join(dirpath,
>> > +                                                             'MAINTAINERS'))
>> > +        self.maintainers_database = maintainers_database
>> > +
>> > +    def __del__(self):
>> > +        """Delete the incomplete boards.cfg
>> > +
>> > +        This destructor deletes boards.cfg if the private member 'in_progress'
>> > +        is defined as True.  The 'in_progress' member is set to True at the
>> > +        beginning of the generate() method and set to False at its end.
>> > +        So, in_progress==True means generating boards.cfg was terminated
>> > +        on the way.
>> > +        """
>> > +
>> > +        if hasattr(self, 'in_progress') and self.in_progress:
>>
>> Would it be better to initialise in_progress to None in the constructor?
>>
>
>
> At first I thought of that.
>
>
> If the constructor fails before setting in_progress to None,
> the destructor is invoked with undefined in_progress.
>
> Of course, it rarely (never) happens.
> But I am trying to be as safe as possible.

I think it's good practice to avoid doing any processing in a
constructor that can fail. You can have a separate method for that,
thus guaranteeing that the constructor finishes correctly. Anyway this
is not an important point.

Regards,
Simon

[U-Boot] [PATCH 5/7] tools/genboardscfg.py: fix minor problems on termination

[Masahiro Yamada]

Hi Simon,



On Wed, 20 Aug 2014 13:10:52 -0600
Simon Glass <sjg@chromium.org> wrote:


> I try to put an _ before private members to indicate that they should
> not be used outside the class. But It is not particularly important -
> just thought I'd mention it.


I tried this before posing v3.
On the way, I stopped and found it getting unreadable because
I had to put an _ to lots of variables.

At least, in my script, all the methods are public and all the variables
are private, so I did not do that after all.



Best Regards
Masahiro Yamada

[U-Boot] [PATCH 6/7] tools/genboardscfg.py: check if the boards.cfg is up to date

[Masahiro Yamada]

It looks silly to regenerate the boards.cfg even when it is
already up to date.

The tool should exit with doing nothing if the boards.cfg is newer
than any of defconfig, Kconfig and MAINTAINERS files.

Specify -f (--force) option to get the boards.cfg regenerated
regardless its time stamp.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

 tools/genboardscfg.py | 57 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 55 insertions(+), 2 deletions(-)

diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
index 13bb424..899db69 100755
--- a/tools/genboardscfg.py
+++ b/tools/genboardscfg.py
@@ -87,6 +87,51 @@ def get_make_cmd():
         sys.exit('GNU Make not found')
     return ret[0].rstrip()
 
+def output_is_new():
+    """Check if the boards.cfg file is up to date.
+
+    Returns:
+      True if the boards.cfg file exists and is newer than any of
+      *_defconfig, MAINTAINERS and Kconfig*.  False otherwise.
+    """
+    try:
+        ctime = os.path.getctime(BOARD_FILE)
+    except OSError, exception:
+        if exception.errno == errno.ENOENT:
+            # return False on 'No such file or directory' error
+            return False
+        else:
+            raise
+
+    for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
+        for filename in fnmatch.filter(filenames, '*_defconfig'):
+            if fnmatch.fnmatch(filename, '.*'):
+                continue
+            filepath = os.path.join(dirpath, filename)
+            if ctime < os.path.getctime(filepath):
+                return False
+
+    for (dirpath, dirnames, filenames) in os.walk('.'):
+        for filename in filenames:
+            if (fnmatch.fnmatch(filename, '*~') or
+                not fnmatch.fnmatch(filename, 'Kconfig*') and
+                not filename == 'MAINTAINERS'):
+                continue
+            filepath = os.path.join(dirpath, filename)
+            if ctime < os.path.getctime(filepath):
+                return False
+
+    # Detect a board that has been removed since the current boards.cfg
+    # was generated
+    for line in open(BOARD_FILE):
+        if line[0] == '#' or line == '\n':
+            continue
+        defconfig = line.split()[6] + '_defconfig'
+        if not os.path.exists(os.path.join(CONFIG_DIR, defconfig)):
+            return False
+
+    return True
+
 ### classes ###
 class MaintainersDatabase:
 
@@ -507,7 +552,7 @@ class BoardsFileGenerator:
 
         self.in_progress = False
 
-def gen_boards_cfg(jobs):
+def gen_boards_cfg(jobs=1, force=False):
     """Generate boards.cfg file.
 
     The incomplete boards.cfg is deleted if an error (including
@@ -517,6 +562,10 @@ def gen_boards_cfg(jobs):
       jobs: The number of jobs to run simultaneously
     """
     check_top_directory()
+    if not force and output_is_new():
+        print "%s is up to date. Nothing to do." % BOARD_FILE
+        sys.exit(0)
+
     generator = BoardsFileGenerator()
     generator.generate(jobs)
 
@@ -525,7 +574,10 @@ def main():
     # Add options here
     parser.add_option('-j', '--jobs',
                       help='the number of jobs to run simultaneously')
+    parser.add_option('-f', '--force', action="store_true", default=False,
+                      help='regenerate the output even if it is new')
     (options, args) = parser.parse_args()
+
     if options.jobs:
         try:
             jobs = int(options.jobs)
@@ -538,7 +590,8 @@ def main():
         except (OSError, ValueError):
             print 'info: failed to get the number of CPUs. Set jobs to 1'
             jobs = 1
-    gen_boards_cfg(jobs)
+
+    gen_boards_cfg(jobs, force=options.force)
 
 if __name__ == '__main__':
     main()
-- 
1.9.1

[U-Boot] [PATCH 6/7] tools/genboardscfg.py: check if the boards.cfg is up to date

[Simon Glass]

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> It looks silly to regenerate the boards.cfg even when it is
> already up to date.
>
> The tool should exit with doing nothing if the boards.cfg is newer
> than any of defconfig, Kconfig and MAINTAINERS files.
>
> Specify -f (--force) option to get the boards.cfg regenerated
> regardless its time stamp.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Acked-by: Simon Glass <sjg@chromium.org>

Great idea.

> ---
>
>  tools/genboardscfg.py | 57 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
> index 13bb424..899db69 100755
> --- a/tools/genboardscfg.py
> +++ b/tools/genboardscfg.py
> @@ -87,6 +87,51 @@ def get_make_cmd():
>          sys.exit('GNU Make not found')
>      return ret[0].rstrip()
>
> +def output_is_new():
> +    """Check if the boards.cfg file is up to date.
> +
> +    Returns:
> +      True if the boards.cfg file exists and is newer than any of
> +      *_defconfig, MAINTAINERS and Kconfig*.  False otherwise.
> +    """
> +    try:
> +        ctime = os.path.getctime(BOARD_FILE)
> +    except OSError, exception:
> +        if exception.errno == errno.ENOENT:
> +            # return False on 'No such file or directory' error
> +            return False
> +        else:
> +            raise

if not os.path.exists(BOARD_FILE)
   return False

would probably be enough.

> +
> +    for (dirpath, dirnames, filenames) in os.walk(CONFIG_DIR):
> +        for filename in fnmatch.filter(filenames, '*_defconfig'):
> +            if fnmatch.fnmatch(filename, '.*'):
> +                continue
> +            filepath = os.path.join(dirpath, filename)
> +            if ctime < os.path.getctime(filepath):
> +                return False
> +
> +    for (dirpath, dirnames, filenames) in os.walk('.'):
> +        for filename in filenames:
> +            if (fnmatch.fnmatch(filename, '*~') or
> +                not fnmatch.fnmatch(filename, 'Kconfig*') and
> +                not filename == 'MAINTAINERS'):
> +                continue
> +            filepath = os.path.join(dirpath, filename)
> +            if ctime < os.path.getctime(filepath):
> +                return False
> +
> +    # Detect a board that has been removed since the current boards.cfg
> +    # was generated
> +    for line in open(BOARD_FILE):
> +        if line[0] == '#' or line == '\n':
> +            continue
> +        defconfig = line.split()[6] + '_defconfig'
> +        if not os.path.exists(os.path.join(CONFIG_DIR, defconfig)):
> +            return False
> +
> +    return True
> +
>  ### classes ###
>  class MaintainersDatabase:
>
> @@ -507,7 +552,7 @@ class BoardsFileGenerator:
>
>          self.in_progress = False
>
> -def gen_boards_cfg(jobs):
> +def gen_boards_cfg(jobs=1, force=False):
>      """Generate boards.cfg file.
>
>      The incomplete boards.cfg is deleted if an error (including
> @@ -517,6 +562,10 @@ def gen_boards_cfg(jobs):
>        jobs: The number of jobs to run simultaneously
>      """
>      check_top_directory()
> +    if not force and output_is_new():
> +        print "%s is up to date. Nothing to do." % BOARD_FILE
> +        sys.exit(0)
> +
>      generator = BoardsFileGenerator()
>      generator.generate(jobs)
>
> @@ -525,7 +574,10 @@ def main():
>      # Add options here
>      parser.add_option('-j', '--jobs',
>                        help='the number of jobs to run simultaneously')
> +    parser.add_option('-f', '--force', action="store_true", default=False,
> +                      help='regenerate the output even if it is new')
>      (options, args) = parser.parse_args()
> +
>      if options.jobs:
>          try:
>              jobs = int(options.jobs)
> @@ -538,7 +590,8 @@ def main():
>          except (OSError, ValueError):
>              print 'info: failed to get the number of CPUs. Set jobs to 1'
>              jobs = 1
> -    gen_boards_cfg(jobs)
> +
> +    gen_boards_cfg(jobs, force=options.force)
>
>  if __name__ == '__main__':
>      main()
> --
> 1.9.1
>

Regards,
Simon

[U-Boot] [PATCH 6/7] tools/genboardscfg.py: check if the boards.cfg is up to date

[Masahiro Yamada]

Hi Simon,



On Wed, 20 Aug 2014 13:13:13 -0600
Simon Glass <sjg@chromium.org> wrote:
> > +def output_is_new():
> > +    """Check if the boards.cfg file is up to date.
> > +
> > +    Returns:
> > +      True if the boards.cfg file exists and is newer than any of
> > +      *_defconfig, MAINTAINERS and Kconfig*.  False otherwise.
> > +    """
> > +    try:
> > +        ctime = os.path.getctime(BOARD_FILE)
> > +    except OSError, exception:
> > +        if exception.errno == errno.ENOENT:
> > +            # return False on 'No such file or directory' error
> > +            return False
> > +        else:
> > +            raise
> 
> if not os.path.exists(BOARD_FILE)
>    return False
> 
> would probably be enough.
> 


Actually my first code was as follows:

------------>8------------------------
if not os.path.exists(BOARD_FILE)
    return False

ctime = os.path.getctime(BOARD_FILE)
-------------8<----------------------



But what if someone deletes BOARD_FILE
between os.path.exists(BOARD_FILE) and os.path.getctime(BOARD_FILE)?

I know it is ridiculous to consider such a rare case.


But I believe it is Python style to follow
"try something and then handle an exception" thing.

I am trying to be a bit strict to this rule
when invoking OS system calls where there is possibility of failure.



Best Regards
Masahiro Yamada

[U-Boot] [PATCH 6/7] tools/genboardscfg.py: check if the boards.cfg is up to date

[Simon Glass]

Hi Masahiro,

On 20 August 2014 22:02, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
> On Wed, 20 Aug 2014 13:13:13 -0600
> Simon Glass <sjg@chromium.org> wrote:
>> > +def output_is_new():
>> > +    """Check if the boards.cfg file is up to date.
>> > +
>> > +    Returns:
>> > +      True if the boards.cfg file exists and is newer than any of
>> > +      *_defconfig, MAINTAINERS and Kconfig*.  False otherwise.
>> > +    """
>> > +    try:
>> > +        ctime = os.path.getctime(BOARD_FILE)
>> > +    except OSError, exception:
>> > +        if exception.errno == errno.ENOENT:
>> > +            # return False on 'No such file or directory' error
>> > +            return False
>> > +        else:
>> > +            raise
>>
>> if not os.path.exists(BOARD_FILE)
>>    return False
>>
>> would probably be enough.
>>
>
>
> Actually my first code was as follows:
>
> ------------>8------------------------
> if not os.path.exists(BOARD_FILE)
>     return False
>
> ctime = os.path.getctime(BOARD_FILE)
> -------------8<----------------------
>
>
>
> But what if someone deletes BOARD_FILE
> between os.path.exists(BOARD_FILE) and os.path.getctime(BOARD_FILE)?
>
> I know it is ridiculous to consider such a rare case.
>
>
> But I believe it is Python style to follow
> "try something and then handle an exception" thing.
>
> I am trying to be a bit strict to this rule
> when invoking OS system calls where there is possibility of failure.

Failure in this case seems safe IMO :-) Anyway I will leave this up to
you, it is not important.

Regards,
Simon

[U-Boot] [PATCH 7/7] tools/genboardscfg.py: improve performance

[Masahiro Yamada]

I guess some developers are already getting sick of this tool
because it takes a few minites to generate the boards.cfg
on reasonable computers.

This commit makes it about 4 times faster.
You might not be satisfied at all, but better than now.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

On my computer (Core i7 2700K + 16GB memory),

60 sec --> 14 sec

This commit does not solve the root cause at all.
We still need to find a better way to replace this patch.


 tools/genboardscfg.py | 43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py
index 899db69..bfa338c 100755
--- a/tools/genboardscfg.py
+++ b/tools/genboardscfg.py
@@ -30,7 +30,7 @@ CONFIG_DIR = 'configs'
 REFORMAT_CMD = [os.path.join('tools', 'reformat.py'),
                 '-i', '-d', '-', '-s', '8']
 SHOW_GNU_MAKE = 'scripts/show-gnu-make'
-SLEEP_TIME=0.03
+SLEEP_TIME=0.003
 
 COMMENT_BLOCK = '''#
 # List of boards
@@ -315,13 +315,22 @@ class Slot:
         Arguments:
           output: File object which the result is written to
           maintainers_database: An instance of class MaintainersDatabase
+          devnull: file object of 'dev/null'
+          make_cmd: the command name of Make
         """
-        self.occupied = False
         self.build_dir = tempfile.mkdtemp()
         self.devnull = devnull
-        self.make_cmd = make_cmd
+        self.ps = subprocess.Popen([make_cmd, 'O=' + self.build_dir,
+                                    'allnoconfig'], stdout=devnull)
+        self.occupied = True
         self.parser = DotConfigParser(self.build_dir, output,
                                       maintainers_database)
+        self.tmp_defconfig = os.path.join(CONFIG_DIR, '.tmp_defconfig')
+        os.makedirs(os.path.join(self.build_dir, CONFIG_DIR))
+        self.env = os.environ.copy()
+        self.env['srctree'] = os.getcwd()
+        self.env['UBOOTVERSION'] = 'dummy'
+        self.env['KCONFIG_OBJDIR'] = ''
 
     def __del__(self):
         """Delete the working directory"""
@@ -344,13 +353,33 @@ class Slot:
         """
         if self.occupied:
             return False
-        o = 'O=' + self.build_dir
-        self.ps = subprocess.Popen([self.make_cmd, o, defconfig],
-                                   stdout=self.devnull)
+
+        f = open(os.path.join(self.build_dir, self.tmp_defconfig), 'w')
+        for line in open(os.path.join(CONFIG_DIR, defconfig)):
+            colon = line.find(':CONFIG_')
+            if colon == -1:
+                f.write(line)
+            else:
+                f.write(line[colon + 1:])
+        f.close()
+
+        self.ps = subprocess.Popen([os.path.join('scripts', 'kconfig', 'conf'),
+                                    '--defconfig=' + self.tmp_defconfig,
+                                    'Kconfig'],
+                                   stdout=self.devnull,
+                                   cwd=self.build_dir,
+                                   env=self.env)
+
         self.defconfig = defconfig
         self.occupied = True
         return True
 
+    def wait(self):
+        """Wait until the current subprocess finishes."""
+        while self.occupied and self.ps.poll() == None:
+            time.sleep(SLEEP_TIME)
+        self.occupied = False
+
     def poll(self):
         """Check if the subprocess is running and invoke the .config
         parser if the subprocess is terminated.
@@ -388,6 +417,8 @@ class Slots:
         for i in range(jobs):
             self.slots.append(Slot(output, maintainers_database,
                                    devnull, make_cmd))
+        for slot in self.slots:
+            slot.wait()
 
     def add(self, defconfig):
         """Add a new subprocess if a vacant slot is available.
-- 
1.9.1

[U-Boot] [PATCH 7/7] tools/genboardscfg.py: improve performance

[Simon Glass]

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> I guess some developers are already getting sick of this tool
> because it takes a few minites to generate the boards.cfg
> on reasonable computers.
>
> This commit makes it about 4 times faster.
> You might not be satisfied at all, but better than now.
>
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>

Acked-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 0/7] tools/genboardscfg.py: various fixes and performance improvement

[Simon Glass]

Hi Masahiro,

On 20 August 2014 05:47, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
>
> This series depends on the following prerequisites
>
> http://patchwork.ozlabs.org/patch/380316/
> http://patchwork.ozlabs.org/patch/376222/
>
>
>
> Masahiro Yamada (7):
>   tools/genboardscfg.py: ignore defconfigs starting with a dot
>   tools/genboardscfg.py: be tolerant of missing MAINTAINERS
>   tools/genboardscfg.py: be tolerant of insane Kconfig
>   tools/genboardscfg.py: wait for unfinished subprocesses before
>     error-out
>   tools/genboardscfg.py: fix minor problems on termination
>   tools/genboardscfg.py: check if the boards.cfg is up to date
>   tools/genboardscfg.py: improve performance
>
>  tools/genboardscfg.py | 278 ++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 204 insertions(+), 74 deletions(-)

Before:

 time ./tools/genboardscfg.py
Generating boards.cfg ...  (jobs: 32)
1177/1177 [=======================================================>]

real 0m27.018s
user 7m15.330s
sys 2m57.488s

After:

time ./tools/genboardscfg.py
boards.cfg is up to date. Nothing to do.

real 0m0.278s
user 0m0.199s
sys 0m0.079s

rm boards.cfg
time ./tools/genboardscfg.py
Generating boards.cfg ...  (jobs: 32)
1177/1177 [=======================================================>]

real 0m8.607s
user 3m9.580s
sys 0m23.997s


Wow, nice work!

Regards,
Simon