[libvirt] [PATCH 0/2] Improvements for apibuild.py

Hi. Patch #1 just simplifies the code a bit. This requires python 2.2.3. Is that OK? Patch #2 fixes an issue with parsing top level comments. Looking at the libvirt HTML output, you can see that the license text at the top of the file ends prematurely. This happens because the "<http:" is regarded by apibuild.py as an info key, the part afterwards as its value. The patch restricts the characters allowed for the key. Alternatively, it would also be possible using only specific words. Claudio Bley (2): docs: simplify code docs: restrict the set of characters for info keys docs/apibuild.py | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) -- 1.7.9.5

Signed-off-by: Claudio Bley <cbley@av-test.de> --- docs/apibuild.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 7b336b3..63b21e1 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -654,17 +654,11 @@ class CParser: lines = string.split(comment, "\n") item = None for line in lines: - while line != "" and (line[0] == ' ' or line[0] == '\t'): - line = line[1:] - while line != "" and line[0] == '*': - line = line[1:] - while line != "" and (line[0] == ' ' or line[0] == '\t'): - line = line[1:] + line = line.lstrip().lstrip('*').lstrip() try: (it, line) = string.split(line, ":", 1) item = it - while line != "" and (line[0] == ' ' or line[0] == '\t'): - line = line[1:] + line = line.lstrip() if res.has_key(item): res[item] = res[item] + " " + line else: -- 1.7.9.5

On 01/11/2013 04:20 AM, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- docs/apibuild.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
My python is very weak, so you may want a second opinion; but I at least tested that this patch succeeded on RHEL 5. (weak) ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, Jan 11, 2013 at 4:55 PM, Eric Blake <eblake@redhat.com> wrote:
On 01/11/2013 04:20 AM, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- docs/apibuild.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
My python is very weak, so you may want a second opinion; but I at least tested that this patch succeeded on RHEL 5.
(weak) ACK.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Only thing to note is the original code consumed 1 space, while this will consume all leading space. But if it worked for Eric then its likely fine. -- Doug Goldstein

At Fri, 11 Jan 2013 23:48:24 -0600, Doug Goldstein wrote:
On Fri, Jan 11, 2013 at 4:55 PM, Eric Blake <eblake@redhat.com> wrote:
On 01/11/2013 04:20 AM, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- docs/apibuild.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
My python is very weak, so you may want a second opinion; but I at least tested that this patch succeeded on RHEL 5.
(weak) ACK.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Only thing to note is the original code consumed 1 space, while this will consume all leading space.
Actually, it consumed 1 space, tab or * in a loop until the first character wasn't a space, tab or *, respectively. So, the new code is pretty much equivalent. Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

At Fri, 11 Jan 2013 15:55:36 -0700, Eric Blake wrote:
[1 <text/plain; UTF-8 (quoted-printable)>] On 01/11/2013 04:20 AM, Claudio Bley wrote:
Signed-off-by: Claudio Bley <cbley@av-test.de> --- docs/apibuild.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
My python is very weak, so you may want a second opinion; but I at least tested that this patch succeeded on RHEL 5.
(weak) ACK.
For me (even) your weak ACK suffices... Thanks, pushed now. Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern

When parsing the top level comment of a file, apibuild.py used to split on any ':' character of a line regarding the first part as a key for a setting, e.g. "Summary". The second part would then be assigned as the value for that key. This means you could not use a ':' character inside those comments without ill effects. Now, a key must consist solely of alphanumeric characters, '_' or '.'. Signed-off-by: Claudio Bley <cbley@av-test.de> --- docs/apibuild.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 63b21e1..e6a45ec 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -10,6 +10,7 @@ import os, sys import string import glob +import re quiet=True warnings=0 @@ -655,20 +656,17 @@ class CParser: item = None for line in lines: line = line.lstrip().lstrip('*').lstrip() - try: - (it, line) = string.split(line, ":", 1) - item = it - line = line.lstrip() + + m = re.match('([_.a-zA-Z0-9]+):(.*)', line) + if m: + item = m.group(1) + line = m.group(2).lstrip() + + if item: if res.has_key(item): res[item] = res[item] + " " + line else: res[item] = line - except: - if item != None: - if res.has_key(item): - res[item] = res[item] + " " + line - else: - res[item] = line self.index.info = res def strip_lead_star(self, line): -- 1.7.9.5

On 01/11/2013 04:20 AM, Claudio Bley wrote:
When parsing the top level comment of a file, apibuild.py used to split on any ':' character of a line regarding the first part as a key for a setting, e.g. "Summary". The second part would then be assigned as the value for that key.
This means you could not use a ':' character inside those comments without ill effects.
Now, a key must consist solely of alphanumeric characters, '_' or '.'.
Again, my python is weak; but this time, I compared http://libvirt.org/html/libvirt-libvirt.html with the result of this patch, and see the difference ("if not, see <http:///www.gnu.org/licenses/>." is no longer truncated). Therefore, ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/11/2013 04:20 AM, Claudio Bley wrote:
Hi.
Patch #1 just simplifies the code a bit. This requires python 2.2.3. Is that OK?
Our current requirement for how old we must support is that we must still build on the latest RHEL 5 (or CentOS 5) environment (RHEL 5.9 was released earlier this week). On that platform, python is at 2.4.3. So yes, I think you are safe on that front. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

At Fri, 11 Jan 2013 10:18:19 -0700, Eric Blake wrote:
[1 <text/plain; UTF-8 (quoted-printable)>] On 01/11/2013 04:20 AM, Claudio Bley wrote:
Hi.
Patch #1 just simplifies the code a bit. This requires python 2.2.3. Is that OK?
Our current requirement for how old we must support is that we must still build on the latest RHEL 5 (or CentOS 5) environment (RHEL 5.9 was released earlier this week). On that platform, python is at 2.4.3. So yes, I think you are safe on that front.
Good. Should we document that build requirement somewhere? Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
participants (3)
-
Claudio Bley
-
Doug Goldstein
-
Eric Blake