* [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors
@ 2022-11-27 14:48 Ricardo Martincoski
2022-11-27 14:48 ` [Buildroot] [next, v2 2/2] utils/get-developers: print error for correct line Ricardo Martincoski
2023-02-07 8:24 ` [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors Thomas Petazzoni via buildroot
0 siblings, 2 replies; 3+ messages in thread
From: Ricardo Martincoski @ 2022-11-27 14:48 UTC (permalink / raw)
To: buildroot; +Cc: Thomas Petazzoni, Ricardo Martincoski
Currently 4 types of parsing errors/warnings can be found:
- entry for a file that is not in the tree anymore (warning)
- developer entry with no file entry (error)
- file entry with no developer (error)
- 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 all 3 error types into hard errors and bail out at the first error
found, because the rest of the state machine is not designed to handle
malformed input.
Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- do not change warning to error, an entry for a file that is not in
the tree anymore is not a syntax error (Thomas)
- add the lineno on the warning messages in the next patch instead of
this patch
check-*:
https://gitlab.com/RicardoMartincoski/buildroot/-/pipelines/706249815
tests.utils.test_get_developers.TestGetDevelopers:
https://gitlab.com/RicardoMartincoski/buildroot/-/pipelines/706249847
---
support/testing/tests/utils/test_get_developers.py | 9 ++++-----
utils/getdeveloperlib.py | 1 +
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/support/testing/tests/utils/test_get_developers.py b/support/testing/tests/utils/test_get_developers.py
index 12710fe8d3..0b313c7c59 100644
--- a/support/testing/tests/utils/test_get_developers.py
+++ b/support/testing/tests/utils/test_get_developers.py
@@ -70,11 +70,11 @@ class TestGetDevelopers(unittest.TestCase):
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
+ # -v generating error for developer entry with no file entries, stopping on first error
developers = b'# comment\n' \
b'# comment\n' \
b'\n' \
@@ -84,10 +84,9 @@ class TestGetDevelopers(unittest.TestCase):
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)
+ self.assertEqual(len(err), 1)
# -v not generating error for developer entry with empty list of file entries
developers = b'# comment\n' \
diff --git a/utils/getdeveloperlib.py b/utils/getdeveloperlib.py
index 2a8d5c213c..dbd21af443 100644
--- a/utils/getdeveloperlib.py
+++ b/utils/getdeveloperlib.py
@@ -243,6 +243,7 @@ def parse_developers(filename=None):
if name is not None or len(files) != 0:
print("Syntax error in DEVELOPERS file, line %d" % linen,
file=sys.stderr)
+ return None
name = line[2:].strip()
elif line.startswith("F:"):
fname = line[2:].strip()
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Buildroot] [next, v2 2/2] utils/get-developers: print error for correct line
2022-11-27 14:48 [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors Ricardo Martincoski
@ 2022-11-27 14:48 ` Ricardo Martincoski
2023-02-07 8:24 ` [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors Thomas Petazzoni via buildroot
1 sibling, 0 replies; 3+ messages in thread
From: Ricardo Martincoski @ 2022-11-27 14:48 UTC (permalink / raw)
To: buildroot; +Cc: Thomas Petazzoni, 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 in these cases print
the line number from the line before.
Also count empty and comment lines, so a developer fixing the file can
jump to the correct line (or the nearest one).
At same time standardize the messages, printing the line number
also in the case of a warning for a file that is not in the tree
anymore.
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- avoid changing the message to a weird one, just print the line
number for the line before (Thomas)
- add the lineno on the warning messages in the this patch instead of
the previous one
check-*:
https://gitlab.com/RicardoMartincoski/buildroot/-/pipelines/706255042
tests.utils.test_get_developers.TestGetDevelopers:
https://gitlab.com/RicardoMartincoski/buildroot/-/pipelines/706255012
---
.../testing/tests/utils/test_get_developers.py | 16 ++++++++--------
utils/getdeveloperlib.py | 6 +++---
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 0b313c7c59..ffc01f6ce7 100644
--- a/support/testing/tests/utils/test_get_developers.py
+++ b/support/testing/tests/utils/test_get_developers.py
@@ -48,7 +48,7 @@ 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.assertIn("Syntax error in DEVELOPERS file, line 1: 'text1'", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 1)
@@ -56,7 +56,7 @@ class TestGetDevelopers(unittest.TestCase):
# -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)
@@ -69,7 +69,7 @@ class TestGetDevelopers(unittest.TestCase):
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, line 4", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 1)
@@ -83,7 +83,7 @@ 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.assertIn("Syntax error in DEVELOPERS file, line 1", err)
+ self.assertIn("Syntax error in DEVELOPERS file, line 4", err)
self.assertEqual(rc, 1)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 1)
@@ -108,8 +108,8 @@ class TestGetDevelopers(unittest.TestCase):
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.assertIn("WARNING: 'path/that/does/not/exists/1' doesn't match any file, line 2", err)
+ self.assertIn("WARNING: 'path/that/does/not/exists/2' doesn't match any file, line 3", err)
self.assertEqual(rc, 0)
self.assertEqual(len(out), 0)
self.assertEqual(len(err), 2)
@@ -119,8 +119,8 @@ class TestGetDevelopers(unittest.TestCase):
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.assertIn("WARNING: 'path/that/does/not/exists/1' doesn't match any file, line 2", err)
+ self.assertIn("WARNING: 'path/that/does/not/exists/2' doesn't match any file, line 3", err)
self.assertEqual(rc, 0)
self.assertGreater(len(out), 1000)
self.assertEqual(len(err), 2)
diff --git a/utils/getdeveloperlib.py b/utils/getdeveloperlib.py
index dbd21af443..e7d0d23e49 100644
--- a/utils/getdeveloperlib.py
+++ b/utils/getdeveloperlib.py
@@ -236,12 +236,13 @@ def parse_developers(filename=None):
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:
- print("Syntax error in DEVELOPERS file, line %d" % linen,
+ print("Syntax error in DEVELOPERS file, line %d" % (linen - 1),
file=sys.stderr)
return None
name = line[2:].strip()
@@ -249,7 +250,7 @@ def parse_developers(filename=None):
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,
+ print("WARNING: '%s' doesn't match any file, line %d" % (fname, linen),
file=sys.stderr)
for f in dev_files:
dev_file = os.path.relpath(f, brpath)
@@ -267,7 +268,6 @@ 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))
--
2.34.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors
2022-11-27 14:48 [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors Ricardo Martincoski
2022-11-27 14:48 ` [Buildroot] [next, v2 2/2] utils/get-developers: print error for correct line Ricardo Martincoski
@ 2023-02-07 8:24 ` Thomas Petazzoni via buildroot
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-07 8:24 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: buildroot
On Sun, 27 Nov 2022 11:48:18 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> Currently 4 types of parsing errors/warnings can be found:
> - entry for a file that is not in the tree anymore (warning)
> - developer entry with no file entry (error)
> - file entry with no developer (error)
> - 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 all 3 error types into hard errors and bail out at the first error
> found, because the rest of the state machine is not designed to handle
> malformed input.
> Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
> Changes v1 -> v2:
> - do not change warning to error, an entry for a file that is not in
> the tree anymore is not a syntax error (Thomas)
> - add the lineno on the warning messages in the next patch instead of
> this patch
Both 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] 3+ messages in thread
end of thread, other threads:[~2023-02-07 8:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-27 14:48 [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors Ricardo Martincoski
2022-11-27 14:48 ` [Buildroot] [next, v2 2/2] utils/get-developers: print error for correct line Ricardo Martincoski
2023-02-07 8:24 ` [Buildroot] [next, v2 1/2] utils/get-developers: bail out on parsing errors 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.