* [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.