Skip to contentSkip to conversation

WIP: Changes to patman libraries

PATCH New Simon Glass 4 participants 17 messages Oct 31, 2012 – Dec 1, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 WIP: Changes to patman libraries

These changes are required to the patman libraries. This is not a proper patch yet, just sometime to try out.

---
 tools/patman/checkpatch.py      |    2 -
 tools/patman/command.py         |   86 ++++++--
 tools/patman/cros_subprocess.py |  402 +++++++++++++++++++++++++++++++++++++++
 tools/patman/gitutil.py         |  129 ++++++++++++-
 tools/patman/patchstream.py     |   38 +++-
 tools/patman/terminal.py        |   30 ++-
 6 files changed, 642 insertions(+), 45 deletions(-)
 create mode 100644 tools/patman/cros_subprocess.py
tools/patman/checkpatch.py +0 -2
index d831087..4b6748a 100644
@@ -70,8 +70,6 @@ def CheckPatch(fname, verbose=False):
                 '~/bin directory')
     item = {}
     stdout = command.Output(chk, '--no-tree', fname)
-    #pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE)
-    #stdout, stderr = pipe.communicate()
 
     # total: 0 errors, 0 warnings, 159 lines checked
     re_stats = re.compile('total: (\\d+) errors, (\d+) warnings, (\d+)')
tools/patman/command.py +64 -22

117 lines not on this page · raw message

tools/patman/cros_subprocess.py new file +402 -0

402 lines not on this page · raw message

tools/patman/gitutil.py +126 -3

160 lines not on this page · raw message

tools/patman/patchstream.py +30 -8
index ad280cc..db2cc6c 100644
@@ -237,7 +237,8 @@ class PatchStream:
         # Detect the start of a new commit
         elif commit_match:
             self.CloseCommit()
-            self.commit = commit.Commit(commit_match.group(1)[:7])
+            # TODO: We should store the whole hash, and just display a subset
+            self.commit = commit.Commit(commit_match.group(1)[:8])
 
         # Detect tags in the commit message
         elif tag_match:
@@ -334,26 +335,47 @@ class PatchStream:
         self.Finalize()
 
 
-def GetMetaData(start, count):
+def GetMetaDataForList(commit_range, git_dir=None, count=None,
+                       series = Series()):
     """Reads out patch series metadata from the commits
 
     This does a 'git log' on the relevant commits and pulls out the tags we
     are interested in.
 
     Args:
-        start: Commit to start from: 0=HEAD, 1=next one, etc.
-        count: Number of commits to list
+        commit_range: Range of commits to count (e.g. 'HEAD..base')
+        git_dir: Path to git repositiory (None to use default)
+        count: Number of commits to list, or None for no limit
+        series: Series object to add information into. By default a new series
+            is started.
+    Returns:
+        A Series object containing information about the commits.
     """
-    pipe = [['git', 'log', '--no-color', '--reverse', 'HEAD~%d' % start,
-	'-n%d' % count]]
-    stdout = command.RunPipe(pipe, capture=True)
-    series = Series()
+    params = ['git', 'log', '--no-color', '--reverse', commit_range]
+    if count is not None:
+        params[2:2] = ['-n%d' % count]
+    if git_dir:
+        params[1:1] = ['--git-dir', git_dir]
+    pipe = [params]
+    stdout = command.RunPipe(pipe, capture=True).stdout
     ps = PatchStream(series, is_log=True)
     for line in stdout.splitlines():
         ps.ProcessLine(line)
     ps.Finalize()
     return series
 
+def GetMetaData(start, count):
+    """Reads out patch series metadata from the commits
+
+    This does a 'git log' on the relevant commits and pulls out the tags we
+    are interested in.
+
+    Args:
+        start: Commit to start from: 0=HEAD, 1=next one, etc.
+        count: Number of commits to list
+    """
+    return GetMetaDataForList('HEAD~%d' % start, None, count)
+
 def FixPatch(backup_dir, fname, series, commit):
     """Fix up a patch file, by adding/removing as required.
 
tools/patman/terminal.py +20 -10
index 838c828..337a2a4 100644
@@ -24,24 +24,32 @@
 This module handles terminal interaction including ANSI color codes.
 """
 
+import os
+import sys
+
+# Selection of when we want our output to be colored
+COLOR_IF_TERMINAL, COLOR_ALWAYS, COLOR_NEVER = range(3)
+
 class Color(object):
   """Conditionally wraps text in ANSI color escape sequences."""
   BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE = range(8)
   BOLD = -1
-  COLOR_START = '\033[1;%dm'
+  BRIGHT_START = '\033[1;%dm'
+  NORMAL_START = '\033[22;%dm'
   BOLD_START = '\033[1m'
   RESET = '\033[0m'
 
-  def __init__(self, enabled=True):
+  def __init__(self, colored=COLOR_IF_TERMINAL):
     """Create a new Color object, optionally disabling color output.
 
     Args:
       enabled: True if color output should be enabled. If False then this
         class will not add color codes at all.
     """
-    self._enabled = enabled
+    self._enabled = (colored == COLOR_ALWAYS or
+        (colored == COLOR_IF_TERMINAL and os.isatty(sys.stdout.fileno())))
 
-  def Start(self, color):
+  def Start(self, color, bright=True):
     """Returns a start color code.
 
     Args:
@@ -52,7 +60,8 @@ class Color(object):
       otherwise returns empty string
     """
     if self._enabled:
-      return self.COLOR_START % (color + 30)
+        base = self.BRIGHT_START if bright else self.NORMAL_START
+        return base % (color + 30)
     return ''
 
   def Stop(self):
@@ -63,10 +72,10 @@ class Color(object):
       returns empty string
     """
     if self._enabled:
-      return self.RESET
+        return self.RESET
     return ''
 
-  def Color(self, color, text):
+  def Color(self, color, text, bright=True):
     """Returns text with conditionally added color escape sequences.
 
     Keyword arguments:
@@ -78,9 +87,10 @@ class Color(object):
       returns text with color escape sequences based on the value of color.
     """
     if not self._enabled:
-      return text
+        return text
     if color == self.BOLD:
-      start = self.BOLD_START
+        start = self.BOLD_START
     else:
-      start = self.COLOR_START % (color + 30)
+        base = self.BRIGHT_START if bright else self.NORMAL_START
+        start = base % (color + 30)
     return start + text + self.RESET
Signature · 1 line
1.7.7.3
#1 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

This tool handles building U-Boot to check that you have not broken it with your patch series. It can build each individual commit and report which boards fail on which commits, and which errors come up. It aims to make full use of multi-processor machines.

Buildman is not yet ready for prime time. I am posting it now to obtain feedback as to its operation and bugs, and to hopefully attract patches for these. It does some incorrect things, crashes, hangs, and use lots of disk space.

Documentation and caveats are in tools/buildman/README.

---
 tools/buildman/.gitignore   |    1 +
 tools/buildman/README       |  454 ++++++++++++++++++++++
 tools/buildman/board.py     |  159 ++++++++
 tools/buildman/bsettings.py |   59 +++
 tools/buildman/builder.py   |  893 +++++++++++++++++++++++++++++++++++++++++++
 tools/buildman/buildman     |    1 +
 tools/buildman/buildman.py  |  116 ++++++
 tools/buildman/control.py   |  164 ++++++++
 tools/buildman/test.py      |  194 ++++++++++
 tools/buildman/toolchain.py |  156 ++++++++
 10 files changed, 2197 insertions(+), 0 deletions(-)
 create mode 100644 tools/buildman/.gitignore
 create mode 100644 tools/buildman/README
 create mode 100644 tools/buildman/board.py
 create mode 100644 tools/buildman/bsettings.py
 create mode 100644 tools/buildman/builder.py
 create mode 120000 tools/buildman/buildman
 create mode 100755 tools/buildman/buildman.py
 create mode 100644 tools/buildman/control.py
 create mode 100644 tools/buildman/test.py
 create mode 100644 tools/buildman/toolchain.py
tools/buildman/.gitignore new file +1 -0
new file mode 100644
index 0000000..0d20b64
@@ -0,0 +1 @@
+*.pyc
tools/buildman/README new file +454 -0

454 lines not on this page · raw message

tools/buildman/board.py new file +159 -0

159 lines not on this page · raw message

tools/buildman/bsettings.py new file +59 -0

59 lines not on this page · raw message

tools/buildman/builder.py new file +893 -0

893 lines not on this page · raw message

tools/buildman/buildman new file +1 -0
new file mode 120000
index 0000000..e4fba2d
@@ -0,0 +1 @@
+buildman.py
\ No newline at end of file
tools/buildman/buildman.py new file +116 -0

116 lines not on this page · raw message

tools/buildman/control.py new file +164 -0

164 lines not on this page · raw message

tools/buildman/test.py new file +194 -0

194 lines not on this page · raw message

tools/buildman/toolchain.py new file +156 -0

156 lines not on this page · raw message

Signature · 1 line
1.7.7.3
#2 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Dear Simon Glass,

Quoted 9 lines · In message <[email protected]> you wrote:

This tool handles building U-Boot to check that you have not broken it with your patch series. It can build each individual commit and report which boards fail on which commits, and which errors come up. It aims to make full use of multi-processor machines.

Buildman is not yet ready for prime time. I am posting it now to obtain feedback as to its operation and bugs, and to hopefully attract patches for these. It does some incorrect things, crashes, hangs, and use lots of disk space.

Can you please explain a bit if or how this is related to MAKEALL? At first (_very_ sshort) glance it appears to be a completely different tool - but then, do we need two separate tools for appearently pretty similar purposes?

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I used to be indecisive, now I'm not sure.
#3 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Quoted 13 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 9 lines · In message <[email protected]> you wrote:

This tool handles building U-Boot to check that you have not broken it with your patch series. It can build each individual commit and report which boards fail on which commits, and which errors come up. It aims to make full use of multi-processor machines.

Buildman is not yet ready for prime time. I am posting it now to obtain feedback as to its operation and bugs, and to hopefully attract patches for these. It does some incorrect things, crashes, hangs, and use lots of disk space.

Quoted 4 lines

Can you please explain a bit if or how this is related to MAKEALL? At first (_very_ sshort) glance it appears to be a completely different tool - but then, do we need two separate tools for appearently pretty similar purposes?

I'll let Simon explain what Buildman handles better/worse than MAKEALL but my 2 cents is that MAKEALL is like patchwork in that it's a tool we all use and many of us wish was better about X/Y/Z. Unlike patchwork, it's bothered various folks enough to get changes made (Joe and his bugfixes last night, I've pastebin'd my wrapper a number of times, Simon has this tool going).

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121031/b70a67c8/attachment.pgp>
#4 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Hi Wolfgang,

Quoted 19 lines · Tom Rini
Quoted 13 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 9 lines · In message <[email protected]> you wrote:

This tool handles building U-Boot to check that you have not broken it with your patch series. It can build each individual commit and report which boards fail on which commits, and which errors come up. It aims to make full use of multi-processor machines.

Buildman is not yet ready for prime time. I am posting it now to obtain feedback as to its operation and bugs, and to hopefully attract patches for these. It does some incorrect things, crashes, hangs, and use lots of disk space.

Quoted 4 lines

Can you please explain a bit if or how this is related to MAKEALL? At first (_very_ sshort) glance it appears to be a completely different tool - but then, do we need two separate tools for appearently pretty similar purposes?

Quoted 6 lines

I'll let Simon explain what Buildman handles better/worse than MAKEALL but my 2 cents is that MAKEALL is like patchwork in that it's a tool we all use and many of us wish was better about X/Y/Z. Unlike patchwork, it's bothered various folks enough to get changes made (Joe and his bugfixes last night, I've pastebin'd my wrapper a number of times, Simon has this tool going).

Yes...it's mostly for building a list of commits (e.g. an entire branch) and automatically tracking and showing what boards break between commits. It is optimised for this - e.g. it can build 22 commits for 1000 boards (22,000 builds) in about an hour on a fast machine, which is a few times faster than I have managed with MAKEALL. If run a second time it doesn't rebuild commits it has already done, which can save time. Also it handles the toolchains mostly automatically.

It is a completely different tool, yes, but I have found myself using it instead of what I previously used: MAKEALL plus Mike's 'buildall' wrapper. The main thing I like about it is that it quickly shows me which builds are broken by which commits, and what the errors were.

I have posted it since it's not a lot of use having it privately - it might be useful to others.

Regards,
Simon

Quoted 3 lines

--
Tom

#5 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Dear Simon,

Quoted 6 lines · In message <CAPnjgZ1qV=Odn93NMWEeAOzOOeBHyWwXB21s+n+cXXh0nan=eg@mail.gmail.com>…

Yes...it's mostly for building a list of commits (e.g. an entire branch) and automatically tracking and showing what boards break between commits. It is optimised for this - e.g. it can build 22 commits for 1000 boards (22,000 builds) in about an hour on a fast machine, which is a few times faster than I have managed with MAKEALL.

Do you have an explanation why this is so? There must be some fundamental difference, and I would really like to understand that.

Quoted 7 lines

It is a completely different tool, yes, but I have found myself using it instead of what I previously used: MAKEALL plus Mike's 'buildall' wrapper. The main thing I like about it is that it quickly shows me which builds are broken by which commits, and what the errors were.

I have posted it since it's not a lot of use having it privately - it might be useful to others.

Pleasae don;t misunderstand me. I really appreciate your posting. And if this tool is really that fast, I want to have it, and the sooner the better. But then, we should dump MAKEALL, especially if it's so slow.

In the end, there should be just one such tool...

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"If you can, help others. If you can't, at least don't hurt  others."
- the Dalai Lama
#6 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Hi Wolfgang,

Quoted 4 lines · Wolfgang Denk

Dear Simon,

Quoted 1 line · In message <CAPnjgZ1qV=Odn93NMWEeAOzOOeBHyWwXB21s+n+cXXh0nan=eg@mail.gmail.com>…
Quoted 8 lines
Quoted 5 lines

Yes...it's mostly for building a list of commits (e.g. an entire branch) and automatically tracking and showing what boards break between commits. It is optimised for this - e.g. it can build 22 commits for 1000 boards (22,000 builds) in about an hour on a fast machine, which is a few times faster than I have managed with MAKEALL.

Do you have an explanation why this is so? There must be some fundamental difference, and I would really like to understand that.

I think it is partly that it checks out each commit and builds it incrementally (without throwing away existing files). So checking out the next commit may not affect the board being built at all, in which case the build is very fast.

Also rather than running with 'make -j40' or whatever, it uses 'make -j1' but with 40 threads. This increases CPU utilisation quite substantially (almost 50%) - I think this was discussed some time ago, and was something that Mike's buildall script did. But I found that it had conflicts and was hard to use. For example, it seems to be unhappy when different board were configured to different architectures (something to do with include path confusion which I hope is now fixed
- partly the subject of my efb2172 commit). Also, it prints all the output in a bit of a mess, so it's hard to see what board it relates to, and you have to go back to the log files to see.

Quoted 9 lines
Quoted 8 lines

It is a completely different tool, yes, but I have found myself using it instead of what I previously used: MAKEALL plus Mike's 'buildall' wrapper. The main thing I like about it is that it quickly shows me which builds are broken by which commits, and what the errors were.

I have posted it since it's not a lot of use having it privately - it might be useful to others.

Quoted 6 lines

Pleasae don;t misunderstand me. I really appreciate your posting. And if this tool is really that fast, I want to have it, and the sooner the better. But then, we should dump MAKEALL, especially if it's so slow.

In the end, there should be just one such tool...

Well I hope we can have a period of trying it out :-) Let's see what people think, and there is still work to do. It has become my tool of choice, but only in the last few months.

Regards,
Simon

Quoted 11 lines

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "If you can, help others. If you can't, at least don't hurt others."
- the Dalai Lama

#7 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

On Wed, Oct 31, 2012 at 05:11:51PM -0700, Simon Glass wrote:

[snip]

Quoted 3 lines

Also rather than running with 'make -j40' or whatever, it uses 'make -j1' but with 40 threads. This increases CPU utilisation quite substantially (almost 50%) - I think this was discussed some time ago,

MAKEALL supports this, but doesn't default to it, today. I'm wondering if we ought to make it the default for all non-single board builds. The winning point is right around boards-to-build == grep -c processor /proc/cpuinfo and I think is right around setting NBUILDS to that value, both on consumer multicore and heavy-duty 32/64 core boxes.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121031/9134fd62/attachment.pgp>
#8 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Quoted 7 lines · Tom Rini

On Wed, Oct 31, 2012 at 05:11:51PM -0700, Simon Glass wrote:

[snip]

Quoted 3 lines

Also rather than running with 'make -j40' or whatever, it uses 'make -j1' but with 40 threads. This increases CPU utilisation quite substantially (almost 50%) - I think this was discussed some time ago,

Quoted 5 lines

MAKEALL supports this, but doesn't default to it, today. I'm wondering if we ought to make it the default for all non-single board builds. The winning point is right around boards-to-build == grep -c processor /proc/cpuinfo and I think is right around setting NBUILDS to that value, both on consumer multicore and heavy-duty 32/64 core boxes.

Yeah, I usually set BUILD_NBUILDS to 24-50 on my 24-thread system, and then set BUILD_NCPUS to 1-4. It's nice to offer a little bit of parallelism on the individual builds, sometimes. I did a bunch of tests, but not in a scientific fashion. Enough to agree with you that maxing out builds seems to win for u-boot.

Andy

#9 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Hi Andy / Tom,

Quoted 14 lines · Andy Fleming
Quoted 7 lines · Tom Rini

On Wed, Oct 31, 2012 at 05:11:51PM -0700, Simon Glass wrote:

[snip]

Quoted 3 lines

Also rather than running with 'make -j40' or whatever, it uses 'make -j1' but with 40 threads. This increases CPU utilisation quite substantially (almost 50%) - I think this was discussed some time ago,

Quoted 5 lines

MAKEALL supports this, but doesn't default to it, today. I'm wondering if we ought to make it the default for all non-single board builds. The winning point is right around boards-to-build == grep -c processor /proc/cpuinfo and I think is right around setting NBUILDS to that value, both on consumer multicore and heavy-duty 32/64 core boxes.

Quoted 5 lines

Yeah, I usually set BUILD_NBUILDS to 24-50 on my 24-thread system, and then set BUILD_NCPUS to 1-4. It's nice to offer a little bit of parallelism on the individual builds, sometimes. I did a bunch of tests, but not in a scientific fashion. Enough to agree with you that maxing out builds seems to win for u-boot.

As a bit of an unscientific test, what sort of time does it take to build all 1000-or-so boards on your systems?

Quoted 2 lines

Andy

Regards,
Simon

#10 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 20 lines · Simon Glass

Hi Andy / Tom,

Quoted 16 lines · Andy Fleming
Quoted 14 lines · Tom Rini

On Wed, Oct 31, 2012 at 05:11:51PM -0700, Simon Glass wrote:

[snip]

Quoted 4 lines

Also rather than running with 'make -j40' or whatever, it uses 'make -j1' but with 40 threads. This increases CPU utilisation quite substantially (almost 50%) - I think this was discussed some time ago,

MAKEALL supports this, but doesn't default to it, today. I'm wondering if we ought to make it the default for all non-single board builds. The winning point is right around boards-to-build == grep -c processor /proc/cpuinfo and I think is right around setting NBUILDS to that value, both on consumer multicore and heavy-duty 32/64 core boxes.

Quoted 8 lines
Quoted 5 lines

Yeah, I usually set BUILD_NBUILDS to 24-50 on my 24-thread system, and then set BUILD_NCPUS to 1-4. It's nice to offer a little bit of parallelism on the individual builds, sometimes. I did a bunch of tests, but not in a scientific fashion. Enough to agree with you that maxing out builds seems to win for u-boot.

As a bit of an unscientific test, what sort of time does it take to build all 1000-or-so boards on your systems?

My setup using MAKEALL does 921 boards (arm/powerpc/mips) with ELDK 5.2 in 54min wall-clock with 397% CPU util (on a 6 core machine).

- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQkwXKAAoJENk4IS6UOR1Ws+oP/A2Estr8LHk2zt3cclDrshwI 9iNpiMvwJ2a/Qj3DzK6q3tuLKIJq/Chfsgczrz3wlv6RGqN5SB8U/Cbl9/OAg9oC jmSEjpGxT3pJyRKscyf/78cjMb06BadHVybPjkMtmfdlay3+H4CLLFkVahM1BKut +7MxTFtQIkPkHtXUwcTSYXuIhwwz61d6R65colHoHWVIbtRWpksaWa64107mEg3K 0A4Ite+FYoMK0BeswA1mPGknDCCLfgE32oXI4k4tQBAXrLcYWKlxLcirGydiZ46l Mrqh+62EA4U2FEVj3VbRjB2JtqRiqtbYYNSk9fwkdU0Fa/eYZacYYxStZVnVTfaQ RgZd0Gq7knJUed8bS0RvptDKLnMCadc0CgVEQ6Svj2JgfeZ9l/HOJkUX/MI7bCmv eKrtQQium4cJfXx2fGGdR2O21g+oQML/V0UTHkq78qMr3Z9Bqj3wRRyVGc5ch7kb 3P8aYjlJG8qU6OtDiwvRNIMvAQzovetJu18L+F/X3lgxqsvrEjk1Ugdach0smmkl TcM9U21la7oOAxhFg4J5W6aatdDx6kKU6s4K68kpcvpQ0MJjQ6bhjKASzf/VFuTl 6vLXUiN+qHc/2yBmuxgMluRfAzFEsLWZ3Y6r3bQC4f3mfmUZxt7CJ8D+rYqSQKUW TDHABRVdUDJxbypUaA8q
=VhSN
-----END PGP SIGNATURE-----

#11 lore raw

PATCH 2/2 WIP: Test version of buildman - U-Boot builder

Hi,

Quoted 30 lines · Tom Rini

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 26 lines · Simon Glass

Hi Andy / Tom,

Quoted 21 lines · Andy Fleming
Quoted 8 lines · Tom Rini

On Wed, Oct 31, 2012 at 05:11:51PM -0700, Simon Glass wrote:

[snip]

Quoted 4 lines

Also rather than running with 'make -j40' or whatever, it uses 'make -j1' but with 40 threads. This increases CPU utilisation quite substantially (almost 50%) - I think this was discussed some time ago,

Quoted 6 lines

MAKEALL supports this, but doesn't default to it, today. I'm wondering if we ought to make it the default for all non-single board builds. The winning point is right around boards-to-build == grep -c processor /proc/cpuinfo and I think is right around setting NBUILDS to that value, both on consumer multicore and heavy-duty 32/64 core boxes.

Yeah, I usually set BUILD_NBUILDS to 24-50 on my 24-thread system, and then set BUILD_NCPUS to 1-4. It's nice to offer a little bit of parallelism on the individual builds, sometimes. I did a bunch of tests, but not in a scientific fashion. Enough to agree with you that maxing out builds seems to win for u-boot.

Quoted 5 lines
Quoted 2 lines

As a bit of an unscientific test, what sort of time does it take to build all 1000-or-so boards on your systems?

My setup using MAKEALL does 921 boards (arm/powerpc/mips) with ELDK 5.2 in 54min wall-clock with 397% CPU util (on a 6 core machine).

buildman is best at building a series of commits at once. I suspect with just one commit its performance would be similar to MAKEALL. I use it to verify a series so that I know it will bisect correctly.

Has anyone given it a try? I have found one bug where it gets confused when a builder thread moved from one architecture to another. Apart from that I am thinking of cleaning up the series and submitting some proper patches. Any comments?

Regards,
Simon

Quoted 21 lines

- --
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQkwXKAAoJENk4IS6UOR1Ws+oP/A2Estr8LHk2zt3cclDrshwI 9iNpiMvwJ2a/Qj3DzK6q3tuLKIJq/Chfsgczrz3wlv6RGqN5SB8U/Cbl9/OAg9oC jmSEjpGxT3pJyRKscyf/78cjMb06BadHVybPjkMtmfdlay3+H4CLLFkVahM1BKut +7MxTFtQIkPkHtXUwcTSYXuIhwwz61d6R65colHoHWVIbtRWpksaWa64107mEg3K 0A4Ite+FYoMK0BeswA1mPGknDCCLfgE32oXI4k4tQBAXrLcYWKlxLcirGydiZ46l Mrqh+62EA4U2FEVj3VbRjB2JtqRiqtbYYNSk9fwkdU0Fa/eYZacYYxStZVnVTfaQ RgZd0Gq7knJUed8bS0RvptDKLnMCadc0CgVEQ6Svj2JgfeZ9l/HOJkUX/MI7bCmv eKrtQQium4cJfXx2fGGdR2O21g+oQML/V0UTHkq78qMr3Z9Bqj3wRRyVGc5ch7kb 3P8aYjlJG8qU6OtDiwvRNIMvAQzovetJu18L+F/X3lgxqsvrEjk1Ugdach0smmkl TcM9U21la7oOAxhFg4J5W6aatdDx6kKU6s4K68kpcvpQ0MJjQ6bhjKASzf/VFuTl 6vLXUiN+qHc/2yBmuxgMluRfAzFEsLWZ3Y6r3bQC4f3mfmUZxt7CJ8D+rYqSQKUW TDHABRVdUDJxbypUaA8q
=VhSN
-----END PGP SIGNATURE-----

#12 lore raw

PATCH 1/2 WIP: Changes to patman libraries

In reply to Simon Glass

Dear Simon Glass,

Quoted 2 lines · In message <[email protected]> you wrote:

These changes are required to the patman libraries. This is not a proper patch yet, just sometime to try out.

...are required. So. And why exactly? Or what is the purpose of these changes?

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Nobody goes to that restaurant anymore. It's too crowded.
#13 lore raw

PATCH 1/2 WIP: Changes to patman libraries

Hi Wolfgang,

Quoted 8 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 2 lines · In message <[email protected]> you wrote:

These changes are required to the patman libraries. This is not a proper patch yet, just sometime to try out.

...are required. So. And why exactly? Or what is the purpose of these changes?

Just so that people can try the builder if they want to. The patches enhance functions in patman, mostly on the git side, so that the builder can do its job. For example it needs to clone a repo, checkout code into a different directory and work with branches a bit more.

This is not a useful patch for any other purpose (e.g. review) - it is just a lump of code. If there is interest in this it will need to be turned into proper patches.

Regards,
Simon

Quoted 10 lines

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Nobody goes to that restaurant anymore. It's too crowded.

#14 lore raw

PATCH 1/2 WIP: Changes to patman libraries

Dear Simon,

Quoted 7 lines · In message <CAPnjgZ3us4yoeqOHxozQ6VPHXhaKJdLWfb0HPWHWvnnUaZ0Ghg@mail.gmail.com>…
Quoted 6 lines
Quoted 3 lines

These changes are required to the patman libraries. This is not a proper patch yet, just sometime to try out.

...are required. So. And why exactly? Or what is the purpose of these changes?

Quoted 8 lines

Just so that people can try the builder if they want to. The patches enhance functions in patman, mostly on the git side, so that the builder can do its job. For example it needs to clone a repo, checkout code into a different directory and work with branches a bit more.

This is not a useful patch for any other purpose (e.g. review) - it is just a lump of code. If there is interest in this it will need to be turned into proper patches.

Hm... I apologize, but I'm just an old man, and a bit slow of wits these days. I cannot review any such code without knowing what it is suppose to acchieve. Maybe you should add such explanations to the commit message, even if it's only a WIP patch? I would definitely appreciate this (as it would help me to understand what this is all about).

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Half of the people in the world are below average.

#15 lore raw

PATCH 1/2 WIP: Changes to patman libraries

Hi Wolfgang,

Quoted 19 lines · Wolfgang Denk

Dear Simon,

Quoted 7 lines · In message <CAPnjgZ3us4yoeqOHxozQ6VPHXhaKJdLWfb0HPWHWvnnUaZ0Ghg@mail.gmail.com>…
Quoted 6 lines
Quoted 3 lines

These changes are required to the patman libraries. This is not a proper patch yet, just sometime to try out.

...are required. So. And why exactly? Or what is the purpose of these changes?

Quoted 8 lines

Just so that people can try the builder if they want to. The patches enhance functions in patman, mostly on the git side, so that the builder can do its job. For example it needs to clone a repo, checkout code into a different directory and work with branches a bit more.

This is not a useful patch for any other purpose (e.g. review) - it is just a lump of code. If there is interest in this it will need to be turned into proper patches.

Quoted 6 lines

Hm... I apologize, but I'm just an old man, and a bit slow of wits these days. I cannot review any such code without knowing what it is suppose to acchieve. Maybe you should add such explanations to the commit message, even if it's only a WIP patch? I would definitely appreciate this (as it would help me to understand what this is all about).

:-) OK I see. I will add a proper commit message so it is properly described, but please do understand it is WIP so far.

Regards,
Simon

Quoted 10 lines

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Half of the people in the world are below average.

#16 lore raw

PATCH 1/2 WIP: Changes to patman libraries

Hi Wolfgang,

Quoted 28 lines · Simon Glass

Hi Wolfgang,

Quoted 25 lines · Wolfgang Denk

Dear Simon,

Quoted 7 lines · In message <CAPnjgZ3us4yoeqOHxozQ6VPHXhaKJdLWfb0HPWHWvnnUaZ0Ghg@mail.gmail.com>…
Quoted 6 lines
Quoted 3 lines

These changes are required to the patman libraries. This is not a proper patch yet, just sometime to try out.

...are required. So. And why exactly? Or what is the purpose of these changes?

Quoted 8 lines

Just so that people can try the builder if they want to. The patches enhance functions in patman, mostly on the git side, so that the builder can do its job. For example it needs to clone a repo, checkout code into a different directory and work with branches a bit more.

This is not a useful patch for any other purpose (e.g. review) - it is just a lump of code. If there is interest in this it will need to be turned into proper patches.

Hm... I apologize, but I'm just an old man, and a bit slow of wits these days. I cannot review any such code without knowing what it is suppose to acchieve. Maybe you should add such explanations to the commit message, even if it's only a WIP patch? I would definitely appreciate this (as it would help me to understand what this is all about).

Here are the notes for the changes. When I split this into commits I will put these comments with each commit.

checkpatch.py:
- remove two commented-out lines which are no-longer needed

command.py:

Change these functions to return a CommandResult class instead of just a return code.
This allows us to capture output, errors and return codes. Should we need to expand
the functionality in the future, it will be fairly easy - just add new members to the class.

Also provide an option to raise an exception on error (non-zero return code), rather than
always just returning the return code. This can make it easier to handle unforseen
errors.

Provide a back-door way of killing all tasks (although this needs further work).

cros_subprocess.py:

This is a slight enhancement of the build-in python subprocess module. It permits
access to command output while it is in progress. This is important if we want to
filter it for errors, etc., while still displaying it on the terminal. Since some tasks can
take a minute to complete, it is not acceptable to show no output during this time.

gitutils.py:

Several functions are enhanced so that you can specify a --git-dir with the command,
and also a --work-tree. This allows us to work with git repositories outside the current directory. This is needed for buildman, since it has multiple threads working
in their own place with their own checked-out commit.

New methods are added to clone a repo, fetch updates from a repo, checkout a commit, and obtain an expression for the list of commits in a branch.

patchstream.py

GetMetaDataForList() now supports --git-dir, and also allows an existing Series to be
added too. This allows us to call it twice with different commit ranges and get a single,
unified series. The old GetMetaData() function now calls this.

terminal.py:

This now supports both bright and normal ANSI colours.

Colours are automatically supressed if the stdout is not a terminal. The avoids getting
ANSI characters in piped output.

Regards,
Simon