| Message ID | 20221127144819.2022348-1-ricardo.martincoski@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [next,v2,1/2] utils/get-developers: bail out on parsing errors | expand |
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
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()
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(-)