Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox