From: Owen Hilyard <ohilyard@iol.unh.edu> BREAKING CHANGE: --tags syntax changed Due to limitations of the argparse library, you can't have 2 varidic arguments. Since upcoming requirements for the CI require the ability to specify individual tests to be run (ex: weekly rte_flow testing), this script must have the ability to pass in those tests as well. This is a change made to support that. Old syntax: "--tags core documentation" New syntax: "--tag core --tag documentation" Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/create_new_execution_file_from_tags.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py index d1d4447..b88fa93 100755 --- a/tools/create_new_execution_file_from_tags.py +++ b/tools/create_new_execution_file_from_tags.py @@ -73,7 +73,8 @@ def __str__(self): parser.add_argument('output_path', help='The path to the output execution file') parser.add_argument('testing_type', type=TestingType, choices=list(TestingType), help='What type of testing to create an execution file for') - parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.') + parser.add_argument('--tag', type=str, action='append', + help='The tags to create an execution file for.') args = parser.parse_args() @@ -86,7 +87,9 @@ def __str__(self): test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in tag_to_test_map_parser[str(args.testing_type)].items()} - tests = map_tags_to_tests(args.tags, test_map) + tests = set() + if args.tag is not None: + tests = map_tags_to_tests(args.tag, test_map) try: output_file = open(args.output_path, 'x') @@ -98,7 +101,11 @@ def __str__(self): if execution_plan != 'DEFAULT': test_allowlist = parse_comma_delimited_list_from_string( template_execution_file_parser[execution_plan]['test_suites']) - tests_to_run = list(set(test_allowlist).intersection(tests)) + if len(tests) != 0: + tests_to_run = list(set(test_allowlist).intersection(tests)) + else: # no tags given + tests_to_run = [entry for entry in test_allowlist if entry != ''] + tests_to_run.sort() template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run) -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> Adds an argument to allow passing in individual test cases to be added to the resulting DTS execution file. This change was made to facilitate periodic testing of testcases not currently in CI, such as rte_flow. A test passed in using this argument will bypass the normal behavior of only allowing test cases specified in the template execution file. This behavior because the mapping of DPDK files to tags is not complete, and to avoid not testing patches, the decision was made to run all tests if the tests for a patch could not be determined. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/create_new_execution_file_from_tags.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py index b88fa93..61d801d 100755 --- a/tools/create_new_execution_file_from_tags.py +++ b/tools/create_new_execution_file_from_tags.py @@ -75,6 +75,8 @@ def __str__(self): help='What type of testing to create an execution file for') parser.add_argument('--tag', type=str, action='append', help='The tags to create an execution file for.') + parser.add_argument('--test', type=str, action='append', + help='The tests to run along with the tests required by the provided tags') args = parser.parse_args() @@ -106,6 +108,10 @@ def __str__(self): else: # no tags given tests_to_run = [entry for entry in test_allowlist if entry != ''] + if args.test is not None: + for test in args.test: + tests_to_run.append(test) + tests_to_run.sort() template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run) -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> The import statements in the file have been moved below the module doc comment per PEP 257. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/guess_git_tree.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tools/guess_git_tree.py b/tools/guess_git_tree.py index c9eef39..e186f5e 100755 --- a/tools/guess_git_tree.py +++ b/tools/guess_git_tree.py @@ -1,20 +1,8 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT) # Copyright 2019 Mellanox Technologies, Ltd -import os -import sys -import re -import argparse -import fnmatch - -from requests.exceptions import HTTPError - -from git_pw import config -from git_pw import api -from git_pw import utils - """ Description: This script uses the git-pw API to retrieve Patchwork's @@ -49,6 +37,17 @@ tree = maintainers.get_tree(files) """ +import os +import sys +import re +import argparse +import fnmatch + +from requests.exceptions import HTTPError + +from git_pw import config +from git_pw import api +from git_pw import utils MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH') if not MAINTAINERS_FILE_PATH: -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> A sanity check has been added to find_filenames. Occasionally, due to how the community lab internally handles getting patches from patchworks, a patch will result in no diff. This patch adds handling for this case. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/guess_git_tree.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/guess_git_tree.py b/tools/guess_git_tree.py index e186f5e..63892e1 100755 --- a/tools/guess_git_tree.py +++ b/tools/guess_git_tree.py @@ -92,6 +92,11 @@ def find_filenames(diff): - Moved _filename_re into the method. - Reduced newlines. """ + # sanity check diff + # for patches without any diff, it will try to run diff.replace + # while diff is None. just return an empty list + if diff is None: + return [] _filename_re = re.compile(r'^(---|\+\+\+) (\S+)') # normalise spaces diff = diff.replace('\r', '') -- 2.30.2
Hi Owen,
> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Wednesday, October 13, 2021 5:00 PM
> To: ci@dpdk.org
> Cc: aconole@redhat.com; Ali Alnubani <alialnu@nvidia.com>; Owen Hilyard
> <ohilyard@iol.unh.edu>
> Subject: [PATCH v2 1/4] create_new_execution_file_from_tags: change tag
> argument
>
Thank you for addressing my comment in v1. Can you please rebase this patchset on latest?
Thanks,
Ali
From: Owen Hilyard <ohilyard@iol.unh.edu> BREAKING CHANGE: --tags syntax changed Due to limitations of the argparse library, you can't have 2 varidic arguments. Since upcoming requirements for the CI require the ability to specify individual tests to be run (ex: weekly rte_flow testing), this script must have the ability to pass in those tests as well. This is a change made to support that. Old syntax: "--tags core documentation" New syntax: "--tag core --tag documentation" Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/create_new_execution_file_from_tags.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py index d1d4447..60ad663 100755 --- a/tools/create_new_execution_file_from_tags.py +++ b/tools/create_new_execution_file_from_tags.py @@ -73,7 +73,8 @@ if __name__ == '__main__': parser.add_argument('output_path', help='The path to the output execution file') parser.add_argument('testing_type', type=TestingType, choices=list(TestingType), help='What type of testing to create an execution file for') - parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.') + parser.add_argument('--tag', type=str, action='append', + help='The tags to create an execution file for.') args = parser.parse_args() @@ -86,7 +87,9 @@ if __name__ == '__main__': test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in tag_to_test_map_parser[str(args.testing_type)].items()} - tests = map_tags_to_tests(args.tags, test_map) + tests = set() + if args.tag is not None: + tests = map_tags_to_tests(args.tags, test_map) try: output_file = open(args.output_path, 'x') @@ -98,7 +101,11 @@ if __name__ == '__main__': if execution_plan != 'DEFAULT': test_allowlist = parse_comma_delimited_list_from_string( template_execution_file_parser[execution_plan]['test_suites']) - tests_to_run = list(set(test_allowlist).intersection(tests)) + if len(tests) != 0: + tests_to_run = list(set(test_allowlist).intersection(tests)) + else: + tests_to_run = [entry for entry in test_allowlist if entry != ''] + tests_to_run.sort() template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run) -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> Adds an argument to allow passing in individual test cases to be added to the resulting DTS execution file. This change was made to facilitate periodic testing of testcases not currently in CI, such as rte_flow. A test passed in using this argument will bypass the normal behavior of only allowing test cases specified in the template execution file. This behavior because the mapping of DPDK files to tags is not complete, and to avoid not testing patches, the decision was made to run all tests if the tests for a patch could not be determined. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/create_new_execution_file_from_tags.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py index 60ad663..58bcf1a 100755 --- a/tools/create_new_execution_file_from_tags.py +++ b/tools/create_new_execution_file_from_tags.py @@ -75,6 +75,8 @@ if __name__ == '__main__': help='What type of testing to create an execution file for') parser.add_argument('--tag', type=str, action='append', help='The tags to create an execution file for.') + parser.add_argument('--test', type=str, action='append', + help='The tests to run along with the tests required by the provided tags') args = parser.parse_args() @@ -105,7 +107,11 @@ if __name__ == '__main__': tests_to_run = list(set(test_allowlist).intersection(tests)) else: tests_to_run = [entry for entry in test_allowlist if entry != ''] - + + if args.test is not None: + for test in args.test: + tests_to_run.append(test) + tests_to_run.sort() template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run) -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> The import statements in the file have been moved below the module doc comment per PEP 257. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/pw_maintainers_cli.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py index fd69081..0705eee 100755 --- a/tools/pw_maintainers_cli.py +++ b/tools/pw_maintainers_cli.py @@ -3,19 +3,6 @@ # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT) # Copyright 2019 Mellanox Technologies, Ltd -import os -import sys -import re -import argparse -import fnmatch - -from requests.exceptions import HTTPError - -from git_pw import config -from git_pw import api -from git_pw import utils -from git_pw import patch as git_pw_patch - """ Description: This script uses the git-pw API to retrieve Patchwork's @@ -53,6 +40,18 @@ Or if you want to use inside other scripts: maintainers = maintainers.get_maintainers(tree_url) """ +import os +import sys +import re +import argparse +import fnmatch + +from requests.exceptions import HTTPError + +from git_pw import config +from git_pw import api +from git_pw import utils +from git_pw import patch as git_pw_patch MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH') if not MAINTAINERS_FILE_PATH: -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> A sanity check has been added to find_filenames. Occasionally, due to how the community lab internally handles getting patches from patchworks, a patch will result in no diff. This patch adds handling for this case. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/pw_maintainers_cli.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py index 0705eee..975b62b 100755 --- a/tools/pw_maintainers_cli.py +++ b/tools/pw_maintainers_cli.py @@ -124,6 +124,11 @@ class Diff(object): - Moved _filename_re into the method. - Reduced newlines. """ + # sanity check diff + # for patches without any diff, it will try to run diff.replace + # while diff is None. just return an empty list + if diff is None: + return [] _filename_re = re.compile(r'^(---|\+\+\+) (\S+)') # normalise spaces diff = diff.replace('\r', '') -- 2.30.2
> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Tuesday, February 1, 2022 11:36 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v3 4/4] guess_git_tree: fix crash caused by empty diff
Did you mean "pw_maintainers_cli: fix crash caused by empty diff"?
Tested-by: Ali Alnubani <alialnu@nvidia.com>
> -----Original Message----- > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu> > Sent: Tuesday, February 1, 2022 11:36 PM > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com> > Cc: Owen Hilyard <ohilyard@iol.unh.edu> > Subject: [PATCH v3 3/4] pw_maintainers_cli: module doc comment moved This subject line here isn't an instruction of what the commit does. See: https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line "pw_maintainers_cli: move module's doc comment" is a better wording IMO. Tested-by: Ali Alnubani <alialnu@nvidia.com>
> -----Original Message----- > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu> > Sent: Tuesday, February 1, 2022 11:36 PM > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com> > Cc: Owen Hilyard <ohilyard@iol.unh.edu> > Subject: [PATCH v3 1/4] create_new_execution_file_from_tags: change tag > argument > [..] > @@ -98,7 +101,11 @@ if __name__ == '__main__': > if execution_plan != 'DEFAULT': > test_allowlist = parse_comma_delimited_list_from_string( > template_execution_file_parser[execution_plan]['test_suites']) > - tests_to_run = list(set(test_allowlist).intersection(tests)) > + if len(tests) != 0: > + tests_to_run = list(set(test_allowlist).intersection(tests)) > + else: > + tests_to_run = [entry for entry in test_allowlist if entry != ''] > + There are empty spaces in this line.
From: Owen Hilyard <ohilyard@iol.unh.edu> BREAKING CHANGE: --tags syntax changed Due to limitations of the argparse library, you can't have 2 varidic arguments. Since upcoming requirements for the CI require the ability to specify individual tests to be run (ex: weekly rte_flow testing), this script must have the ability to pass in those tests as well. This is a change made to support that. Old syntax: "--tags core documentation" New syntax: "--tag core --tag documentation" Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/create_new_execution_file_from_tags.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py index d1d4447..60ad663 100755 --- a/tools/create_new_execution_file_from_tags.py +++ b/tools/create_new_execution_file_from_tags.py @@ -73,7 +73,8 @@ if __name__ == '__main__': parser.add_argument('output_path', help='The path to the output execution file') parser.add_argument('testing_type', type=TestingType, choices=list(TestingType), help='What type of testing to create an execution file for') - parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.') + parser.add_argument('--tag', type=str, action='append', + help='The tags to create an execution file for.') args = parser.parse_args() @@ -86,7 +87,9 @@ if __name__ == '__main__': test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in tag_to_test_map_parser[str(args.testing_type)].items()} - tests = map_tags_to_tests(args.tags, test_map) + tests = set() + if args.tag is not None: + tests = map_tags_to_tests(args.tags, test_map) try: output_file = open(args.output_path, 'x') @@ -98,7 +101,11 @@ if __name__ == '__main__': if execution_plan != 'DEFAULT': test_allowlist = parse_comma_delimited_list_from_string( template_execution_file_parser[execution_plan]['test_suites']) - tests_to_run = list(set(test_allowlist).intersection(tests)) + if len(tests) != 0: + tests_to_run = list(set(test_allowlist).intersection(tests)) + else: + tests_to_run = [entry for entry in test_allowlist if entry != ''] + tests_to_run.sort() template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run) -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> Adds an argument to allow passing in individual test cases to be added to the resulting DTS execution file. This change was made to facilitate periodic testing of testcases not currently in CI, such as rte_flow. A test passed in using this argument will bypass the normal behavior of only allowing test cases specified in the template execution file. This behavior because the mapping of DPDK files to tags is not complete, and to avoid not testing patches, the decision was made to run all tests if the tests for a patch could not be determined. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/create_new_execution_file_from_tags.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py index 60ad663..58bcf1a 100755 --- a/tools/create_new_execution_file_from_tags.py +++ b/tools/create_new_execution_file_from_tags.py @@ -75,6 +75,8 @@ if __name__ == '__main__': help='What type of testing to create an execution file for') parser.add_argument('--tag', type=str, action='append', help='The tags to create an execution file for.') + parser.add_argument('--test', type=str, action='append', + help='The tests to run along with the tests required by the provided tags') args = parser.parse_args() @@ -105,7 +107,11 @@ if __name__ == '__main__': tests_to_run = list(set(test_allowlist).intersection(tests)) else: tests_to_run = [entry for entry in test_allowlist if entry != ''] - + + if args.test is not None: + for test in args.test: + tests_to_run.append(test) + tests_to_run.sort() template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run) -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> The import statements in the file have been moved below the module doc comment per PEP 257. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/pw_maintainers_cli.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py index fd69081..0705eee 100755 --- a/tools/pw_maintainers_cli.py +++ b/tools/pw_maintainers_cli.py @@ -3,19 +3,6 @@ # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT) # Copyright 2019 Mellanox Technologies, Ltd -import os -import sys -import re -import argparse -import fnmatch - -from requests.exceptions import HTTPError - -from git_pw import config -from git_pw import api -from git_pw import utils -from git_pw import patch as git_pw_patch - """ Description: This script uses the git-pw API to retrieve Patchwork's @@ -53,6 +40,18 @@ Or if you want to use inside other scripts: maintainers = maintainers.get_maintainers(tree_url) """ +import os +import sys +import re +import argparse +import fnmatch + +from requests.exceptions import HTTPError + +from git_pw import config +from git_pw import api +from git_pw import utils +from git_pw import patch as git_pw_patch MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH') if not MAINTAINERS_FILE_PATH: -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> A sanity check has been added to find_filenames. Occasionally, due to how the community lab internally handles getting patches from patchworks, a patch will result in no diff. This patch adds handling for this case. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/pw_maintainers_cli.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py index 0705eee..975b62b 100755 --- a/tools/pw_maintainers_cli.py +++ b/tools/pw_maintainers_cli.py @@ -124,6 +124,11 @@ class Diff(object): - Moved _filename_re into the method. - Reduced newlines. """ + # sanity check diff + # for patches without any diff, it will try to run diff.replace + # while diff is None. just return an empty list + if diff is None: + return [] _filename_re = re.compile(r'^(---|\+\+\+) (\S+)') # normalise spaces diff = diff.replace('\r', '') -- 2.30.2
[-- Attachment #1: Type: text/plain, Size: 33 bytes --] V4 should address your feedback. [-- Attachment #2: Type: text/html, Size: 78 bytes --]
From: Owen Hilyard <ohilyard@iol.unh.edu> BREAKING CHANGE: --tags syntax changed Due to limitations of the argparse library, you can't have 2 varidic arguments. Since upcoming requirements for the CI require the ability to specify individual tests to be run (ex: weekly rte_flow testing), this script must have the ability to pass in those tests as well. This is a change made to support that. Old syntax: "--tags core documentation" New syntax: "--tag core --tag documentation" Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/create_new_execution_file_from_tags.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tools/create_new_execution_file_from_tags.py b/tools/create_new_execution_file_from_tags.py index d1d4447..f2081ee 100755 --- a/tools/create_new_execution_file_from_tags.py +++ b/tools/create_new_execution_file_from_tags.py @@ -73,7 +73,10 @@ if __name__ == '__main__': parser.add_argument('output_path', help='The path to the output execution file') parser.add_argument('testing_type', type=TestingType, choices=list(TestingType), help='What type of testing to create an execution file for') - parser.add_argument('tags', metavar='tag', type=str, nargs='*', help='The tags to create an execution file for.') + parser.add_argument('--tag', type=str, action='append', + help='The tags to create an execution file for.') + parser.add_argument('--test', type=str, action='append', + help='The tests to run along with the tests required by the provided tags') args = parser.parse_args() @@ -86,7 +89,9 @@ if __name__ == '__main__': test_map = {key: parse_comma_delimited_list_from_string(value.strip()) for key, value in tag_to_test_map_parser[str(args.testing_type)].items()} - tests = map_tags_to_tests(args.tags, test_map) + tests = set() + if args.tag is not None: + tests = map_tags_to_tests(args.tags, test_map) try: output_file = open(args.output_path, 'x') @@ -98,7 +103,14 @@ if __name__ == '__main__': if execution_plan != 'DEFAULT': test_allowlist = parse_comma_delimited_list_from_string( template_execution_file_parser[execution_plan]['test_suites']) - tests_to_run = list(set(test_allowlist).intersection(tests)) + if len(tests) != 0: + tests_to_run = list(set(test_allowlist).intersection(tests)) + else: + tests_to_run = [entry for entry in test_allowlist if entry != ''] + + if args.test is not None: + for test in args.test: + tests_to_run.append(test) tests_to_run.sort() template_execution_file_parser[execution_plan]['test_suites'] = ", ".join(tests_to_run) -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> The import statements in the file have been moved below the module doc comment per PEP 257. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/pw_maintainers_cli.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py index fd69081..0705eee 100755 --- a/tools/pw_maintainers_cli.py +++ b/tools/pw_maintainers_cli.py @@ -3,19 +3,6 @@ # SPDX-License-Identifier: (BSD-3-Clause AND GPL-2.0-or-later AND MIT) # Copyright 2019 Mellanox Technologies, Ltd -import os -import sys -import re -import argparse -import fnmatch - -from requests.exceptions import HTTPError - -from git_pw import config -from git_pw import api -from git_pw import utils -from git_pw import patch as git_pw_patch - """ Description: This script uses the git-pw API to retrieve Patchwork's @@ -53,6 +40,18 @@ Or if you want to use inside other scripts: maintainers = maintainers.get_maintainers(tree_url) """ +import os +import sys +import re +import argparse +import fnmatch + +from requests.exceptions import HTTPError + +from git_pw import config +from git_pw import api +from git_pw import utils +from git_pw import patch as git_pw_patch MAINTAINERS_FILE_PATH = os.environ.get('MAINTAINERS_FILE_PATH') if not MAINTAINERS_FILE_PATH: -- 2.30.2
From: Owen Hilyard <ohilyard@iol.unh.edu> A sanity check has been added to find_filenames. Occasionally, due to how the community lab internally handles getting patches from patchworks, a patch will result in no diff. This patch adds handling for this case. Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> --- tools/pw_maintainers_cli.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py index 0705eee..975b62b 100755 --- a/tools/pw_maintainers_cli.py +++ b/tools/pw_maintainers_cli.py @@ -124,6 +124,11 @@ class Diff(object): - Moved _filename_re into the method. - Reduced newlines. """ + # sanity check diff + # for patches without any diff, it will try to run diff.replace + # while diff is None. just return an empty list + if diff is None: + return [] _filename_re = re.compile(r'^(---|\+\+\+) (\S+)') # normalise spaces diff = diff.replace('\r', '') -- 2.30.2
> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Monday, February 7, 2022 5:08 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v5 2/3] pw_maintainers_cli: move module doc comment
>
> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> The import statements in the file have been moved below the module
> doc comment per PEP 257.
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
Thanks.
Tested-by: Ali Alnubani <alialnu@nvidia.com>
Acked-by: Ali Alnubani <alialnu@nvidia.com>
> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Monday, February 7, 2022 5:08 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
>
> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> A sanity check has been added to find_filenames. Occasionally, due to
> how the community lab internally handles getting patches from
> patchworks, a patch will result in no diff. This patch adds handling for
> this case.
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
Thanks Owen.
Tested-by: Ali Alnubani <alialnu@nvidia.com>
Acked-by: Ali Alnubani <alialnu@nvidia.com>
> -----Original Message-----
> From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> Sent: Monday, February 7, 2022 5:08 PM
> To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> Subject: [PATCH v5 1/3] create_new_execution_file_from_tags: change tag
> argument
>
> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> BREAKING CHANGE: --tags syntax changed
>
> Due to limitations of the argparse library, you can't have 2 varidic
> arguments. Since upcoming requirements for the CI require the ability to
> specify individual tests to be run (ex: weekly rte_flow testing), this
> script must have the ability to pass in those tests as well. This is a
> change made to support that.
>
> Old syntax:
> "--tags core documentation"
>
> New syntax:
> "--tag core --tag documentation"
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
Thanks.
Acked-by: Ali Alnubani <alialnu@nvidia.com>
> -----Original Message-----
> From: Ali Alnubani <alialnu@nvidia.com>
> Sent: Monday, February 7, 2022 5:56 PM
> To: ohilyard@iol.unh.edu; ci@dpdk.org
> Subject: RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
>
> > -----Original Message-----
> > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu>
> > Sent: Monday, February 7, 2022 5:08 PM
> > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com>
> > Cc: Owen Hilyard <ohilyard@iol.unh.edu>
> > Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff
> >
> > From: Owen Hilyard <ohilyard@iol.unh.edu>
> >
> > A sanity check has been added to find_filenames. Occasionally, due to
> > how the community lab internally handles getting patches from
> > patchworks, a patch will result in no diff. This patch adds handling for
> > this case.
> >
> > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> > ---
>
> Thanks Owen.
>
> Tested-by: Ali Alnubani <alialnu@nvidia.com>
> Acked-by: Ali Alnubani <alialnu@nvidia.com>
Hi Thomas and Aaron,
Can we merge this patch? Pull requests can still cause the script to crash because they have an empty diff.
Patrick, are [1/3] and [2/3] still needed as well?
Thanks,
Ali
[-- Attachment #1: Type: text/plain, Size: 1778 bytes --] > > Patrick, are [1/3] and [2/3] still needed as well? > Yes please on both. We are currently using 1/3 in our Jenkins process for including tag arguments in creating the execution file. Thanks for raising this. On Wed, Jun 21, 2023 at 11:19 AM Ali Alnubani <alialnu@nvidia.com> wrote: > > -----Original Message----- > > From: Ali Alnubani <alialnu@nvidia.com> > > Sent: Monday, February 7, 2022 5:56 PM > > To: ohilyard@iol.unh.edu; ci@dpdk.org > > Subject: RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by > empty diff > > > > > -----Original Message----- > > > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu> > > > Sent: Monday, February 7, 2022 5:08 PM > > > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com> > > > Cc: Owen Hilyard <ohilyard@iol.unh.edu> > > > Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty > diff > > > > > > From: Owen Hilyard <ohilyard@iol.unh.edu> > > > > > > A sanity check has been added to find_filenames. Occasionally, due to > > > how the community lab internally handles getting patches from > > > patchworks, a patch will result in no diff. This patch adds handling > for > > > this case. > > > > > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> > > > --- > > > > Thanks Owen. > > > > Tested-by: Ali Alnubani <alialnu@nvidia.com> > > Acked-by: Ali Alnubani <alialnu@nvidia.com> > > Hi Thomas and Aaron, > > Can we merge this patch? Pull requests can still cause the script to crash > because they have an empty diff. > > Patrick, are [1/3] and [2/3] still needed as well? > > Thanks, > Ali > -- Patrick Robb Technical Service Manager UNH InterOperability Laboratory 21 Madbury Rd, Suite 100, Durham, NH 03824 www.iol.unh.edu [-- Attachment #2: Type: text/html, Size: 5027 bytes --]
Patrick Robb <probb@iol.unh.edu> writes: > Patrick, are [1/3] and [2/3] still needed as well? > > Yes please on both. We are currently using 1/3 in our Jenkins process for including tag arguments in creating > the execution file. Thanks for raising this. Looks like this slipped my queue. I'll apply it now. Thanks. > > > On Wed, Jun 21, 2023 at 11:19 AM Ali Alnubani <alialnu@nvidia.com> wrote: > > > -----Original Message----- > > From: Ali Alnubani <alialnu@nvidia.com> > > Sent: Monday, February 7, 2022 5:56 PM > > To: ohilyard@iol.unh.edu; ci@dpdk.org > > Subject: RE: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff > > > > > -----Original Message----- > > > From: ohilyard@iol.unh.edu <ohilyard@iol.unh.edu> > > > Sent: Monday, February 7, 2022 5:08 PM > > > To: ci@dpdk.org; Ali Alnubani <alialnu@nvidia.com> > > > Cc: Owen Hilyard <ohilyard@iol.unh.edu> > > > Subject: [PATCH v5 3/3] pw_maintainers_cli: fix crash caused by empty diff > > > > > > From: Owen Hilyard <ohilyard@iol.unh.edu> > > > > > > A sanity check has been added to find_filenames. Occasionally, due to > > > how the community lab internally handles getting patches from > > > patchworks, a patch will result in no diff. This patch adds handling for > > > this case. > > > > > > Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu> > > > --- > > > > Thanks Owen. > > > > Tested-by: Ali Alnubani <alialnu@nvidia.com> > > Acked-by: Ali Alnubani <alialnu@nvidia.com> > > Hi Thomas and Aaron, > > Can we merge this patch? Pull requests can still cause the script to crash because they have an empty > diff. > > Patrick, are [1/3] and [2/3] still needed as well? > > Thanks, > Ali
ohilyard@iol.unh.edu writes:
> From: Owen Hilyard <ohilyard@iol.unh.edu>
>
> BREAKING CHANGE: --tags syntax changed
>
> Due to limitations of the argparse library, you can't have 2 varidic
> arguments. Since upcoming requirements for the CI require the ability to
> specify individual tests to be run (ex: weekly rte_flow testing), this
> script must have the ability to pass in those tests as well. This is a
> change made to support that.
>
> Old syntax:
> "--tags core documentation"
>
> New syntax:
> "--tag core --tag documentation"
>
> Signed-off-by: Owen Hilyard <ohilyard@iol.unh.edu>
> ---
1/3, 2/3, and 3/3 applied.
Thanks!