utils/get-developers: misc cleanups and improvements
cover utils/get-developers: misc cleanups and improvements
From: "Yann E. MORIN" <[email protected]>
Hello All!
This small series brings in a few trivial cleanups and improvements to get-developers.
Changes v1 -> v2:
- fix test-suite (Julien)
- enhance help text for new option (Julien)
- fix typoes
Regards,
Yann E. MORIN.
The following changes since commit 621ebccf3f1ca70443160103ae12ab3ea89f63bd
package/python-autocommand: add patch to fix build issue (2024-12-22 17:55:22 +0100)
are available as patches in this mail series,
for you to apply patches up to af1aba057da1bd3bb056f7e6886641149bf22592
utils/get-developers: add option to report Cc: lines (2024-12-23 08:24:24 +0100)
----------------------------------------------------------------
Yann E. MORIN (5):
utils/get-developers: don't offload parse_args() utils/get-developers: use parser.error() rather than canned print+return utils/get-developers: read patch from stdin when it's not a tty utils/get-developers: sort reported developers utils/get-developers: add option to report Cc: lines
support/testing/tests/utils/test_get_developers.py | 12 +++++------ utils/get-developers | 23 +++++++++++----------- 2 files changed, 17 insertions(+), 18 deletions(-)
--
____________ .-----------------.--------------------: _ :------------------. | Yann E. MORIN | Real-Time Embedded | __/ ) | /"\ ASCII RIBBON | | | Software Designer | _/ - /' | \ / CAMPAIGN | | +33 638.411.245 '--------------------: (_ `--, | X AGAINST | | yann.morin (at) orange.com |_=" ,--' | / \ HTML MAIL |
'--------------------------------------:______/_____:------------------'
____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you.
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 1/5 utils/get-developers: don't offload parse_args()
From: "Yann E. MORIN" <[email protected]>
Offloading parser.parse_args() to a helper function does not bring much, if at all; it even is restrictive: indeed, we can't use parser.error() to report errors and thus have to resort to a canned print+return sequence...
--- utils/get-developers | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
utils/get-developers +2 -6
@@ -5,7 +5,7 @@ import getdeveloperlib import sys -def parse_args(): +def __main__(): parser = argparse.ArgumentParser() parser.add_argument('patches', metavar='P', type=argparse.FileType('r'), nargs='*', help='list of patches (use - to read patches from stdin)')
@@ -23,11 +23,7 @@ def parse_args(): const=True, help='validate syntax of DEVELOPERS file') parser.add_argument('-d', dest='filename', action='store', default=None, help='override the default DEVELOPERS file (for debug)') - return parser.parse_args() - - -def __main__(): - args = parse_args() + args = parser.parse_args() # Check that only one action is given action = 0
Signature · 18 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 1/5] utils/get-developers: don't offload parse_args()
Quoted 8 lines · On 23/12/2024 08:27, [email protected] wrote:
From: "Yann E. MORIN" <[email protected]>
Offloading
parser.parse_args()to a helper function does not bring much, if at all; it even is restrictive: indeed, we can't useparser.error()to report errors and thus have to resort to a canned print+return sequence...Signed-off-by Yann E. MORIN <[email protected]>
Series applied to master, thanks.
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 1/5] utils/get-developers: don't offload parse_args()
Quoted 11 lines · Julien Olivain
Quoted 8 lines · On 23/12/2024 08:27, [email protected] wrote:
From: "Yann E. MORIN" <[email protected]>
Offloading
parser.parse_args()to a helper function does not bring much, if at all; it even is restrictive: indeed, we can't useparser.error()to report errors and thus have to resort to a canned print+return sequence...Signed-off-by Yann E. MORIN <[email protected]>Series applied to master, thanks.
And 2024.02.x / 2024.11.x as well, thanks.
Signature · 5 lines
Bye, Peter Korsgaard _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 2/5 utils/get-developers: use parser.error() rather than canned print+return
In reply to [email protected]From: "Yann E. MORIN" <[email protected]>
parser.error() reports a nice error message, that also displays a short reminder of the available options.
Adapt the test-suite accordingly: previously, the error string was an exact string in the stdout list, while it now is a substring in one of the strings in stderr. The exit code changes, too.
---
Changes v1 -> v2:
- fix test-suite (Julien)
--- support/testing/tests/utils/test_get_developers.py | 12 ++++++------ utils/get-developers | 6 ++---- 2 files changed, 8 insertions(+), 10 deletions(-)
support/testing/tests/utils/test_get_developers.py +6 -6
@@ -41,9 +41,9 @@ class TestGetDevelopers(unittest.TestCase): # no args, with syntax error in the file developers = b'text3\n' out, err, rc = call_get_developers("./utils/get-developers", [], self.WITH_EMPTY_PATH, topdir, developers) - self.assertIn("No action specified", out) - self.assertEqual(rc, 0) - self.assertEqual(len(err), 0) + self.assertIn("No action specified", "\n".join(err)) + self.assertEqual(rc, 2) + self.assertEqual(len(out), 0) # -v generating error, called from the main dir developers = b'text1\n'
@@ -150,9 +150,9 @@ class TestGetDevelopers(unittest.TestCase): # no args, with syntax error in the file developers = b'text3\n' out, err, rc = call_get_developers("./utils/get-developers", [], self.WITH_EMPTY_PATH, topdir, developers) - self.assertIn("No action specified", out) - self.assertEqual(rc, 0) - self.assertEqual(len(err), 0) + self.assertIn("No action specified", "\n".join(err)) + self.assertEqual(rc, 2) + self.assertEqual(len(out), 0) # patchfile from topdir and from elsewhere abs_path = infra.filepath("tests/utils/test_get_developers/")
utils/get-developers +2 -4
@@ -40,11 +40,9 @@ def __main__(): if len(args.patches) != 0: action += 1 if action > 1: - print("Cannot do more than one action") - return + parser.error("Cannot do more than one action") if action == 0: - print("No action specified") - return + parser.error("No action specified") devs = getdeveloperlib.parse_developers(args.filename) if devs is None:
Signature · 18 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 3/5 utils/get-developers: read patch from stdin when it's not a tty
In reply to [email protected]From: "Yann E. MORIN" <[email protected]>
It is very common that get-developers be used with its stdin a pipe from git-show:
git show |./utils-get-developers -
In this case, the '-' is superfluous: we can very easily deduce that the user wants to read stdin as the patch.
So, if no other action was requested, and stdin is not a tty, use it as the source of the patch, and thus '-' is then no longer required.
---
Changes v2 -> v2:
- fix typoes in commit log
--- utils/get-developers | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
utils/get-developers +3 -1
@@ -42,7 +42,9 @@ def __main__(): if action > 1: parser.error("Cannot do more than one action") if action == 0: - parser.error("No action specified") + if sys.stdin.isatty(): + parser.error("No action specified") + args.patches.append(sys.stdin) devs = getdeveloperlib.parse_developers(args.filename) if devs is None:
Signature · 18 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH 3/5] utils/get-developers: read patch from stdin when it's not a tty
Hi Yann,
Quoted 26 lines · On 23/12/2024 08:27, [email protected] wrote:
From: "Yann E. MORIN" <[email protected]>
It is very common that get-developers be used with its stdin a pipe from git-show:
git show |./utils-get-developers -In this case, the '-' is superfluous: we can very easily deduce that the user wants to read stdin as the patch.
So, if no other action was requested, and stdin is not a tty, use it as the source of the patch, and thus '-' is then no longer required.
Signed-off-by Yann E. MORIN <[email protected]>---
Changes v2 -> v2:
- fix typoes in commit log--- utils/get-developers | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)utils/get-developers +0
-0
utils/get-developers def __main__(): · line 47
Quoted 7 lines
utils/get-developers +3
-1@@ -42,7 +42,9 @@ def __main__(): if action > 1: parser.error("Cannot do more than one action") if action == 0: - parser.error("No action specified") + if sys.stdin.isatty(): + parser.error("No action specified") + args.patches.append(sys.stdin)
This patch introduces runtime test failures. See for example:
https://gitlab.com/buildroot.org/buildroot/-/jobs/8830671800
When running the test on a tty, the test succeed. For example:
support/testing/run-tests \
-d dl -o output_folder \
tests.utils.test_get_developers
...
OKBut when running the test _without_ a tty, the test fails. This is the case in the Gitlab CI.
For example:
support/testing/run-tests \
-d dl -o output_folder \
tests.utils.test_get_developers < /dev/null
...
FAILED (failures=1)Maybe the runtime test needs to adjust depending stdin is a tty or not.
Could you have a look please?
Quoted 5 lines
utils/get-developers +0
-1devs = getdeveloperlib.parse_developers(args.filename) if devs is None: --2.34.1
Best regards,
Julien.
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 4/5 utils/get-developers: sort reported developers
In reply to [email protected]From: "Yann E. MORIN" <[email protected]>
The list of reported developers is not ordered: that may leave the impression (when receiving a patch) that a Cc is more important than another, by virtue of being earlier in the list.
Also, the ordering changes on every call.
Report the developers in an alphabetically order, so that there is no confusion anymore, and so the ordering is reproducible across calls.
--- utils/get-developers | 1 + 1 file changed, 1 insertion(+)
utils/get-developers +1 -0
@@ -98,6 +98,7 @@ def __main__(): if i in dev.infras: matching_devs.add(dev.name) + matching_devs = sorted(matching_devs) if args.email: for dev in matching_devs: print(dev)
Signature · 18 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 5/5 utils/get-developers: add option to report Cc: lines
In reply to [email protected]From: "Yann E. MORIN" <[email protected]>
It is very common to use the output of get-developers to add cc: lines in the commit log.
Add an option so that get-developers reports Cc: lines ready to be pasted in a commit log. That new option behaves similarly to the existing -e option: it only affects the output when parsing a patch.
---
Changes v1 -> v2:
- model the help text after the one of the -e option, below (Julien)
- extend commit log to explain that.
--- utils/get-developers | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
utils/get-developers +4 -2
@@ -17,6 +17,8 @@ def __main__(): help='find developers in charge of these files') parser.add_argument('-c', dest='check', action='store_const', const=True, help='list files not handled by any developer') + parser.add_argument('--cc', action="store_true", + help='list affected developers as CC: lines ready to be pasted in a commit log') parser.add_argument('-e', dest='email', action='store_const', const=True, help='only list affected developer email addresses') parser.add_argument('-v', dest='validate', action='store_const',
@@ -99,9 +101,9 @@ def __main__(): matching_devs.add(dev.name) matching_devs = sorted(matching_devs) - if args.email: + if args.email or args.cc: for dev in matching_devs: - print(dev) + print(f"{'Cc: ' if args.cc else ''}{dev}") else: result = "--to [email protected]" for dev in matching_devs:
Signature · 18 lines
2.34.1 ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot