* [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test
@ 2022-07-24 23:52 Ricardo Martincoski
2022-07-24 23:53 ` [Buildroot] [PATCH 2/3] utils/get-developers: bail out on parsing errors Ricardo Martincoski
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ricardo Martincoski @ 2022-07-24 23:52 UTC (permalink / raw
To: buildroot; +Cc: Ricardo Martincoski
Change the test into a characterization test for all warnings and errors
get-developers can return when parsing the DEVELOPERS files.
It will be helpful when changing the behavior of get-developers to bail
out on all syntax checking warnings.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
.../tests/utils/test_get_developers.py | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/support/testing/tests/utils/test_get_developers.py b/support/testing/tests/utils/test_get_developers.py
index e4420bc6dc..12710fe8d3 100644
--- a/support/testing/tests/utils/test_get_developers.py
+++ b/support/testing/tests/utils/test_get_developers.py
@@ -47,31 +47,98 @@ class TestGetDevelopers(unittest.TestCase):
# -v generating error, called from the main dir
developers = b'text1\n'
out, err, rc = call_get_developers("./utils/get-developers", ["-v"], self.WITH_EMPTY_PATH, topdir, developers)
self.assertIn("Syntax error in DEVELOPERS file, line 0: 'text1'", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
+ self.assertEqual(len(err), 1)
# -v generating error, called from path
developers = b'text2\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
self.assertIn("Syntax error in DEVELOPERS file, line 0: 'text2'", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
+ self.assertEqual(len(err), 1)
+
+ # -v generating error for file entry with no developer entry
+ developers = b'# comment\n' \
+ b'\n' \
+ b'F:\tutils/get-developers\n' \
+ b'\n' \
+ b'N:\tAuthor2 <email>\n' \
+ b'F:\tutils/get-developers\n'
+ out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
+ self.assertIn("Syntax error in DEVELOPERS file, line 1", err)
+ self.assertEqual(rc, 0)
+ self.assertEqual(len(out), 0)
+ self.assertEqual(len(err), 1)
+
+ # -v generating error for developer entry with no file entries
+ developers = b'# comment\n' \
+ b'# comment\n' \
+ b'\n' \
+ b'N:\tAuthor1 <email>\n' \
+ b'N:\tAuthor2 <email>\n' \
+ b'N:\tAuthor3 <email>\n' \
+ b'F:\tutils/get-developers\n'
+ out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
+ self.assertIn("Syntax error in DEVELOPERS file, line 1", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 2", err)
+ self.assertEqual(rc, 0)
+ self.assertEqual(len(out), 0)
+ self.assertEqual(len(err), 2)
+
+ # -v not generating error for developer entry with empty list of file entries
+ developers = b'# comment\n' \
+ b'# comment\n' \
+ b'\n' \
+ b'N:\tAuthor1 <email>\n' \
+ b'\n' \
+ b'N:\tAuthor2 <email>\n' \
+ b'\n' \
+ b'N:\tAuthor3 <email>\n' \
+ b'F:\tutils/get-developers\n'
+ out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
+ self.assertEqual(rc, 0)
+ self.assertEqual(len(out), 0)
+ self.assertEqual(len(err), 0)
+
+ # -v generating warning for old file entry
+ developers = b'N:\tAuthor <email>\n' \
+ b'F:\tpath/that/does/not/exists/1\n' \
+ b'F:\tpath/that/does/not/exists/2\n'
+ out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
+ self.assertIn("WARNING: 'path/that/does/not/exists/1' doesn't match any file", err)
+ self.assertIn("WARNING: 'path/that/does/not/exists/2' doesn't match any file", err)
+ self.assertEqual(rc, 0)
+ self.assertEqual(len(out), 0)
+ self.assertEqual(len(err), 2)
# -c generating warning and printing lots of files with no developer
developers = b'N:\tAuthor <email>\n' \
b'F:\tpath/that/does/not/exists/1\n' \
b'F:\tpath/that/does/not/exists/2\n'
out, err, rc = call_get_developers("./utils/get-developers", ["-c"], self.WITH_EMPTY_PATH, topdir, developers)
self.assertIn("WARNING: 'path/that/does/not/exists/1' doesn't match any file", err)
self.assertIn("WARNING: 'path/that/does/not/exists/2' doesn't match any file", err)
self.assertEqual(rc, 0)
self.assertGreater(len(out), 1000)
+ self.assertEqual(len(err), 2)
+
+ # -c printing lots of files with no developer
+ developers = b'# comment\n' \
+ b'\n' \
+ b'N:\tAuthor <email>\n' \
+ b'F:\tutils/get-developers\n'
+ out, err, rc = call_get_developers("./utils/get-developers", ["-c"], self.WITH_EMPTY_PATH, topdir, developers)
+ self.assertEqual(rc, 0)
+ self.assertGreater(len(out), 1000)
+ self.assertEqual(len(err), 0)
# -p lists more than one developer
developers = b'N:\tdev1\n' \
b'F:\ttoolchain/\n' \
b'\n' \
b'N:\tdev2\n' \
b'F:\ttoolchain/\n'
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 2/3] utils/get-developers: bail out on parsing errors
2022-07-24 23:52 [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test Ricardo Martincoski
@ 2022-07-24 23:53 ` Ricardo Martincoski
2022-08-01 21:03 ` Thomas Petazzoni via buildroot
2022-07-24 23:53 ` [Buildroot] [PATCH 3/3] utils/get-developers: print error for correct line Ricardo Martincoski
2022-08-01 20:59 ` [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test Thomas Petazzoni via buildroot
2 siblings, 1 reply; 6+ messages in thread
From: Ricardo Martincoski @ 2022-07-24 23:53 UTC (permalink / raw
To: buildroot; +Cc: Thomas Petazzoni, Ricardo Martincoski
Currently 4 types of parsing errors can be found:
- developer entry with no file entry (error)
- file entry with no developer (error)
- entry for file that are not in the tree anymore (warning)
- entry that is not a developer, a file or a comment (hard error)
Currently only the last one ends the script with -v with error code.
Make the warning an error.
Suggested-by: Arnout Vandecappelle <arnout@mind.be>
Make all errors into hard errors.
Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
At same time standardize the error message, printing the line number
that contains the error.
Do not bail out in the first error found but instead print all errors
found, in order to help the developer fixing the DEVELOPERS file.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
NOTICE that when DEVELOPERS file has errors, pkg-stats will fail
(with a DEVELOPERS file *modified* to have an error):
Build package list ...
Getting developers ...
Syntax error in DEVELOPERS file, line 2: 'old_file' doesn't match any file
Build defconfig list ...
Traceback (most recent call last):
File "support/scripts/pkg-stats", line 1289, in <module>
__main__()
File "support/scripts/pkg-stats", line 1249, in __main__
d.set_developers(developers)
File "support/scripts/pkg-stats", line 60, in set_developers
self.developers = [
TypeError: 'NoneType' object is not iterable
---
.../tests/utils/test_get_developers.py | 22 +++++++++----------
utils/getdeveloperlib.py | 7 +++++-
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/support/testing/tests/utils/test_get_developers.py b/support/testing/tests/utils/test_get_developers.py
index 12710fe8d3..d0c8e4d028 100644
--- a/support/testing/tests/utils/test_get_developers.py
+++ b/support/testing/tests/utils/test_get_developers.py
@@ -66,30 +66,30 @@ class TestGetDevelopers(unittest.TestCase):
b'\n' \
b'F:\tutils/get-developers\n' \
b'\n' \
b'N:\tAuthor2 <email>\n' \
b'F:\tutils/get-developers\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
self.assertIn("Syntax error in DEVELOPERS file, line 1", err)
- self.assertEqual(rc, 0)
+ self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 1)
# -v generating error for developer entry with no file entries
developers = b'# comment\n' \
b'# comment\n' \
b'\n' \
b'N:\tAuthor1 <email>\n' \
b'N:\tAuthor2 <email>\n' \
b'N:\tAuthor3 <email>\n' \
b'F:\tutils/get-developers\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
self.assertIn("Syntax error in DEVELOPERS file, line 1", err)
self.assertIn("Syntax error in DEVELOPERS file, line 2", err)
- self.assertEqual(rc, 0)
+ self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 2)
# -v not generating error for developer entry with empty list of file entries
developers = b'# comment\n' \
b'# comment\n' \
b'\n' \
@@ -100,34 +100,34 @@ class TestGetDevelopers(unittest.TestCase):
b'N:\tAuthor3 <email>\n' \
b'F:\tutils/get-developers\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
self.assertEqual(rc, 0)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 0)
- # -v generating warning for old file entry
+ # -v generating error for old file entry
developers = b'N:\tAuthor <email>\n' \
b'F:\tpath/that/does/not/exists/1\n' \
b'F:\tpath/that/does/not/exists/2\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
- self.assertIn("WARNING: 'path/that/does/not/exists/1' doesn't match any file", err)
- self.assertIn("WARNING: 'path/that/does/not/exists/2' doesn't match any file", err)
- self.assertEqual(rc, 0)
+ self.assertIn("Syntax error in DEVELOPERS file, line 1: 'path/that/does/not/exists/1' doesn't match any file", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 2: 'path/that/does/not/exists/2' doesn't match any file", err)
+ self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 2)
- # -c generating warning and printing lots of files with no developer
+ # -c generating error for old file entry
developers = b'N:\tAuthor <email>\n' \
b'F:\tpath/that/does/not/exists/1\n' \
b'F:\tpath/that/does/not/exists/2\n'
out, err, rc = call_get_developers("./utils/get-developers", ["-c"], self.WITH_EMPTY_PATH, topdir, developers)
- self.assertIn("WARNING: 'path/that/does/not/exists/1' doesn't match any file", err)
- self.assertIn("WARNING: 'path/that/does/not/exists/2' doesn't match any file", err)
- self.assertEqual(rc, 0)
- self.assertGreater(len(out), 1000)
+ self.assertIn("Syntax error in DEVELOPERS file, line 1: 'path/that/does/not/exists/1' doesn't match any file", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 2: 'path/that/does/not/exists/2' doesn't match any file", err)
+ self.assertEqual(rc, 1)
+ self.assertEqual(len(out), 0)
self.assertEqual(len(err), 2)
# -c printing lots of files with no developer
developers = b'# comment\n' \
b'\n' \
b'N:\tAuthor <email>\n' \
b'F:\tutils/get-developers\n'
diff --git a/utils/getdeveloperlib.py b/utils/getdeveloperlib.py
index 2a8d5c213c..f9f84ee8e1 100644
--- a/utils/getdeveloperlib.py
+++ b/utils/getdeveloperlib.py
@@ -225,34 +225,37 @@ def parse_developer_runtime_tests(fnames):
return runtimes
def parse_developers(filename=None):
"""Parse the DEVELOPERS file and return a list of Developer objects."""
developers = []
linen = 0
+ errors = 0
global unittests
unittests = list_unittests()
developers_fname = filename or os.path.join(brpath, 'DEVELOPERS')
with open(developers_fname, mode='r', encoding='utf_8') as f:
files = []
name = None
for line in f:
line = line.strip()
if line.startswith("#"):
continue
elif line.startswith("N:"):
if name is not None or len(files) != 0:
+ errors += 1
print("Syntax error in DEVELOPERS file, line %d" % linen,
file=sys.stderr)
name = line[2:].strip()
elif line.startswith("F:"):
fname = line[2:].strip()
dev_files = glob.glob(os.path.join(brpath, fname))
if len(dev_files) == 0:
- print("WARNING: '%s' doesn't match any file" % fname,
+ errors += 1
+ print("Syntax error in DEVELOPERS file, line %d: '%s' doesn't match any file" % (linen, fname),
file=sys.stderr)
for f in dev_files:
dev_file = os.path.relpath(f, brpath)
dev_file = dev_file.replace(os.sep, '/') # force unix sep
if f[-1] == '/': # relpath removes the trailing /
dev_file = dev_file + '/'
files.append(dev_file)
@@ -266,14 +269,16 @@ def parse_developers(filename=None):
print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, line),
file=sys.stderr)
return None
linen += 1
# handle last developer
if name is not None:
developers.append(Developer(name, files))
+ if errors > 0:
+ return None
return developers
def check_developers(developers, basepath=None):
"""Look at the list of files versioned in Buildroot, and returns the
list of files that are not handled by any developer"""
if basepath is None:
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 3/3] utils/get-developers: print error for correct line
2022-07-24 23:52 [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test Ricardo Martincoski
2022-07-24 23:53 ` [Buildroot] [PATCH 2/3] utils/get-developers: bail out on parsing errors Ricardo Martincoski
@ 2022-07-24 23:53 ` Ricardo Martincoski
2022-08-01 21:04 ` Thomas Petazzoni via buildroot
2022-08-01 20:59 ` [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test Thomas Petazzoni via buildroot
2 siblings, 1 reply; 6+ messages in thread
From: Ricardo Martincoski @ 2022-07-24 23:53 UTC (permalink / raw
To: buildroot; +Cc: Ricardo Martincoski
Start counting the line numbers in 1 instead of 0, in case an error
must be printed.
Both the error about a developer entry with no file entry and the error
about a file entry with no developer entry actually belong to the
non-empty line previous the one being analysed, so add a hint to the
error message.
Also count empty and comment lines, so a developer fixing the file can
jump to the correct line (or the nearest one).
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
.../testing/tests/utils/test_get_developers.py | 18 +++++++++---------
utils/getdeveloperlib.py | 4 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/support/testing/tests/utils/test_get_developers.py b/support/testing/tests/utils/test_get_developers.py
index d0c8e4d028..df216d3b02 100644
--- a/support/testing/tests/utils/test_get_developers.py
+++ b/support/testing/tests/utils/test_get_developers.py
@@ -44,51 +44,51 @@ class TestGetDevelopers(unittest.TestCase):
self.assertIn("No action specified", out)
self.assertEqual(rc, 0)
self.assertEqual(len(err), 0)
# -v generating error, called from the main dir
developers = b'text1\n'
out, err, rc = call_get_developers("./utils/get-developers", ["-v"], self.WITH_EMPTY_PATH, topdir, developers)
- self.assertIn("Syntax error in DEVELOPERS file, line 0: 'text1'", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 1: 'text1'", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 1)
# -v generating error, called from path
developers = b'text2\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
- self.assertIn("Syntax error in DEVELOPERS file, line 0: 'text2'", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 1: 'text2'", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 1)
# -v generating error for file entry with no developer entry
developers = b'# comment\n' \
b'\n' \
b'F:\tutils/get-developers\n' \
b'\n' \
b'N:\tAuthor2 <email>\n' \
b'F:\tutils/get-developers\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
- self.assertIn("Syntax error in DEVELOPERS file, line 1", err)
+ self.assertIn("Syntax error in DEVELOPERS file, just before line 5", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 1)
# -v generating error for developer entry with no file entries
developers = b'# comment\n' \
b'# comment\n' \
b'\n' \
b'N:\tAuthor1 <email>\n' \
b'N:\tAuthor2 <email>\n' \
b'N:\tAuthor3 <email>\n' \
b'F:\tutils/get-developers\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
- self.assertIn("Syntax error in DEVELOPERS file, line 1", err)
- self.assertIn("Syntax error in DEVELOPERS file, line 2", err)
+ self.assertIn("Syntax error in DEVELOPERS file, just before line 5", err)
+ self.assertIn("Syntax error in DEVELOPERS file, just before line 6", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 2)
# -v not generating error for developer entry with empty list of file entries
developers = b'# comment\n' \
b'# comment\n' \
@@ -105,27 +105,27 @@ class TestGetDevelopers(unittest.TestCase):
self.assertEqual(len(err), 0)
# -v generating error for old file entry
developers = b'N:\tAuthor <email>\n' \
b'F:\tpath/that/does/not/exists/1\n' \
b'F:\tpath/that/does/not/exists/2\n'
out, err, rc = call_get_developers("get-developers", ["-v"], self.WITH_UTILS_IN_PATH, topdir, developers)
- self.assertIn("Syntax error in DEVELOPERS file, line 1: 'path/that/does/not/exists/1' doesn't match any file", err)
- self.assertIn("Syntax error in DEVELOPERS file, line 2: 'path/that/does/not/exists/2' doesn't match any file", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 2: 'path/that/does/not/exists/1' doesn't match any file", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 3: 'path/that/does/not/exists/2' doesn't match any file", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 2)
# -c generating error for old file entry
developers = b'N:\tAuthor <email>\n' \
b'F:\tpath/that/does/not/exists/1\n' \
b'F:\tpath/that/does/not/exists/2\n'
out, err, rc = call_get_developers("./utils/get-developers", ["-c"], self.WITH_EMPTY_PATH, topdir, developers)
- self.assertIn("Syntax error in DEVELOPERS file, line 1: 'path/that/does/not/exists/1' doesn't match any file", err)
- self.assertIn("Syntax error in DEVELOPERS file, line 2: 'path/that/does/not/exists/2' doesn't match any file", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 2: 'path/that/does/not/exists/1' doesn't match any file", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 3: 'path/that/does/not/exists/2' doesn't match any file", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 2)
# -c printing lots of files with no developer
developers = b'# comment\n' \
b'\n' \
diff --git a/utils/getdeveloperlib.py b/utils/getdeveloperlib.py
index f9f84ee8e1..3383df3c9e 100644
--- a/utils/getdeveloperlib.py
+++ b/utils/getdeveloperlib.py
@@ -233,21 +233,22 @@ def parse_developers(filename=None):
global unittests
unittests = list_unittests()
developers_fname = filename or os.path.join(brpath, 'DEVELOPERS')
with open(developers_fname, mode='r', encoding='utf_8') as f:
files = []
name = None
for line in f:
+ linen += 1
line = line.strip()
if line.startswith("#"):
continue
elif line.startswith("N:"):
if name is not None or len(files) != 0:
errors += 1
- print("Syntax error in DEVELOPERS file, line %d" % linen,
+ print("Syntax error in DEVELOPERS file, just before line %d" % linen,
file=sys.stderr)
name = line[2:].strip()
elif line.startswith("F:"):
fname = line[2:].strip()
dev_files = glob.glob(os.path.join(brpath, fname))
if len(dev_files) == 0:
errors += 1
@@ -265,15 +266,14 @@ def parse_developers(filename=None):
developers.append(Developer(name, files))
files = []
name = None
else:
print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, line),
file=sys.stderr)
return None
- linen += 1
# handle last developer
if name is not None:
developers.append(Developer(name, files))
if errors > 0:
return None
return developers
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test
2022-07-24 23:52 [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test Ricardo Martincoski
2022-07-24 23:53 ` [Buildroot] [PATCH 2/3] utils/get-developers: bail out on parsing errors Ricardo Martincoski
2022-07-24 23:53 ` [Buildroot] [PATCH 3/3] utils/get-developers: print error for correct line Ricardo Martincoski
@ 2022-08-01 20:59 ` Thomas Petazzoni via buildroot
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-08-01 20:59 UTC (permalink / raw
To: Ricardo Martincoski; +Cc: buildroot
On Sun, 24 Jul 2022 20:52:59 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> Change the test into a characterization test for all warnings and errors
> get-developers can return when parsing the DEVELOPERS files.
>
> It will be helpful when changing the behavior of get-developers to bail
> out on all syntax checking warnings.
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
> .../tests/utils/test_get_developers.py | 67 +++++++++++++++++++
> 1 file changed, 67 insertions(+)
Applied to master, thanks.
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 2/3] utils/get-developers: bail out on parsing errors
2022-07-24 23:53 ` [Buildroot] [PATCH 2/3] utils/get-developers: bail out on parsing errors Ricardo Martincoski
@ 2022-08-01 21:03 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-08-01 21:03 UTC (permalink / raw
To: Ricardo Martincoski; +Cc: buildroot
Hello,
On Sun, 24 Jul 2022 20:53:00 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> diff --git a/utils/getdeveloperlib.py b/utils/getdeveloperlib.py
> index 2a8d5c213c..f9f84ee8e1 100644
> --- a/utils/getdeveloperlib.py
> +++ b/utils/getdeveloperlib.py
> @@ -225,34 +225,37 @@ def parse_developer_runtime_tests(fnames):
> return runtimes
>
>
> def parse_developers(filename=None):
> """Parse the DEVELOPERS file and return a list of Developer objects."""
> developers = []
> linen = 0
> + errors = 0
> global unittests
> unittests = list_unittests()
> developers_fname = filename or os.path.join(brpath, 'DEVELOPERS')
> with open(developers_fname, mode='r', encoding='utf_8') as f:
> files = []
> name = None
> for line in f:
> line = line.strip()
> if line.startswith("#"):
> continue
> elif line.startswith("N:"):
> if name is not None or len(files) != 0:
> + errors += 1
> print("Syntax error in DEVELOPERS file, line %d" % linen,
> file=sys.stderr)
So in this error condition, we continue the parsing...
> name = line[2:].strip()
> elif line.startswith("F:"):
> fname = line[2:].strip()
> dev_files = glob.glob(os.path.join(brpath, fname))
> if len(dev_files) == 0:
> - print("WARNING: '%s' doesn't match any file" % fname,
> + errors += 1
> + print("Syntax error in DEVELOPERS file, line %d: '%s' doesn't match any file" % (linen, fname),
> file=sys.stderr)
> for f in dev_files:
> dev_file = os.path.relpath(f, brpath)
> dev_file = dev_file.replace(os.sep, '/') # force unix sep
> if f[-1] == '/': # relpath removes the trailing /
> dev_file = dev_file + '/'
> files.append(dev_file)
> @@ -266,14 +269,16 @@ def parse_developers(filename=None):
> print("Syntax error in DEVELOPERS file, line %d: '%s'" % (linen, line),
> file=sys.stderr)
> return None
but in this one we abort immediately. It is quite confusing that those
two (similar?) error conditions are handled in a different way. Of
course, it is already the case in the current code, but your errors +=
1 made me think about this. I guess on syntax errors, we should stop
parsing immediately, because the rest of the state machine is not
designed to handle malformed input.
So on the two syntax errors conditions: return None immediately, don't
bother counting errors.
On the file not matching case, perhaps a dedicated counter:
'unmatched_files' ?
Regarding the "%s doesn't match any file", I am not happy with having
the message start with "Syntax error in DEVELOPERS file", because it's
not a syntax error. The syntax is perfectly correct. So I appreciate
your wish to make message more consistent, but it doesn't work well
here I'm afraid.
Thanks a lot!
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 3/3] utils/get-developers: print error for correct line
2022-07-24 23:53 ` [Buildroot] [PATCH 3/3] utils/get-developers: print error for correct line Ricardo Martincoski
@ 2022-08-01 21:04 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni via buildroot @ 2022-08-01 21:04 UTC (permalink / raw
To: Ricardo Martincoski; +Cc: buildroot
Hello Ricardo,
On Sun, 24 Jul 2022 20:53:01 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> diff --git a/utils/getdeveloperlib.py b/utils/getdeveloperlib.py
> index f9f84ee8e1..3383df3c9e 100644
> --- a/utils/getdeveloperlib.py
> +++ b/utils/getdeveloperlib.py
> @@ -233,21 +233,22 @@ def parse_developers(filename=None):
> global unittests
> unittests = list_unittests()
> developers_fname = filename or os.path.join(brpath, 'DEVELOPERS')
> with open(developers_fname, mode='r', encoding='utf_8') as f:
> files = []
> name = None
> for line in f:
> + linen += 1
> line = line.strip()
> if line.startswith("#"):
> continue
> elif line.startswith("N:"):
> if name is not None or len(files) != 0:
> errors += 1
> - print("Syntax error in DEVELOPERS file, line %d" % linen,
> + print("Syntax error in DEVELOPERS file, just before line %d" % linen,
What about:
+ print("Syntax error in DEVELOPERS file, line %d" % linen-1,
because well "just before line %d" seems a bit weird.
Thanks!
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-01 21:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-24 23:52 [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test Ricardo Martincoski
2022-07-24 23:53 ` [Buildroot] [PATCH 2/3] utils/get-developers: bail out on parsing errors Ricardo Martincoski
2022-08-01 21:03 ` Thomas Petazzoni via buildroot
2022-07-24 23:53 ` [Buildroot] [PATCH 3/3] utils/get-developers: print error for correct line Ricardo Martincoski
2022-08-01 21:04 ` Thomas Petazzoni via buildroot
2022-08-01 20:59 ` [Buildroot] [PATCH 1/3] support/testing/tests: improve get-developers test Thomas Petazzoni via buildroot
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.