* [Patch, v2] contrib/mklog.py: Improve PR handling (was: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog) [not found] ` <71b4a023-efb2-6c6a-9ced-93cce7c96540@gmail.com> @ 2021-06-21 7:54 ` Tobias Burnus 2021-06-21 8:09 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: Tobias Burnus @ 2021-06-21 7:54 UTC (permalink / raw) To: Martin Sebor, Jason Merrill, Martin Liška Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches [-- Attachment #1: Type: text/plain, Size: 2417 bytes --] On 17.06.21 02:17, Martin Sebor via Gcc wrote: > @@ -147,6 +152,12 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): > > # Extract PR entries from newly added tests > if 'testsuite' in file.path and file.is_added_file: > + name = os.path.basename(file.path) > + name = os.path.splitext(name)[0] > + if name.startswith("pr"): > + name = name[2:] > + name = "PR " + name > + prs.append(name) I think you need a regular expression to extract the PR – as it will both match too much and to little. We have file names such as: * libstdc++-pr91488.C (other prefix) * PR37039.f90 (capitalized PR) * pr98218-1.C (suffix with '-') * pr40724_1.f (suffix with '_') * pr101023a.C (suffix with a letter) But otherwise, I like that idea. * * * Changes in my patch compared to v1: - (From Martin's patch:) Extract the PR from new-files file name (using pattern matching), but only take the PR if the PR wasn't found in the file as PR comment. (The latter happens, e.g., with b376b1ef389.) - Avoid printing the same PR multiple times as summary line (duplicates occur due to 'PR 134' vs. 'PR comp/123' vs. 'PR othercomp/123') — This does not avoid all issues but at least some. If this becomes a real world issue, we can try harder. OK to commit this one? — Comments? * * * I did leave out other changes as they seem to be less clear cut, and which can be still be handled as follow up. Like: - Adding 'Resolves:' (as in some cases it only resolves part of the PR) - ... other changes/patches I missed. (This thread has too many emails.) In particular, if ^PR <comp>/<pr> - .... is accepted by gcc-commit/, then there is no need to list the PRs individually later on. But currently, it is still required. * * * Cross ref: * v1 of my patch was at https://gcc.gnu.org/pipermail/gcc/2021-June/236498.html * Discussion of the -b option is at https://gcc.gnu.org/pipermail/gcc/2021-June/236519.html * Martin S's patch (partially quoted above) is at https://gcc.gnu.org/pipermail/gcc/2021-June/236460.html Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf [-- Attachment #2: mklog-p-v2.diff --] [-- Type: text/x-patch, Size: 5558 bytes --] contrib/mklog.py: Improve PR handling Co-authored-by: Martin Sebor <msebor@redhat.com> contrib/ChangeLog: * mklog.py (bugzilla_url): Fetch also component. (pr_filename_regex): New. (get_pr_titles): Update PR string with correct format and component. (generate_changelog): Take additional PRs; extract PR from the filename. (__main__): Add -b/--pr-numbers argument. contrib/mklog.py | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/contrib/mklog.py b/contrib/mklog.py index 1f59055e723..bba6c1a0e1a 100755 --- a/contrib/mklog.py +++ b/contrib/mklog.py @@ -42,6 +42,7 @@ pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)') prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)') dr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<dr>DR [0-9]+)') dg_regex = re.compile(r'{\s+dg-(error|warning)') +pr_filename_regex = re.compile(r'(^|[\W_])[Pp][Rr](?P<pr>\d{4,})') identifier_regex = re.compile(r'^([a-zA-Z0-9_#].*)') comment_regex = re.compile(r'^\/\*') struct_regex = re.compile(r'^(class|struct|union|enum)\s+' @@ -52,7 +53,7 @@ fn_regex = re.compile(r'([a-zA-Z_][^()\s]*)\s*\([^*]') template_and_param_regex = re.compile(r'<[^<>]*>') md_def_regex = re.compile(r'\(define.*\s+"(.*)"') bugzilla_url = 'https://gcc.gnu.org/bugzilla/rest.cgi/bug?id=%s&' \ - 'include_fields=summary' + 'include_fields=summary,component' function_extensions = {'.c', '.cpp', '.C', '.cc', '.h', '.inc', '.def', '.md'} @@ -118,20 +119,23 @@ def sort_changelog_files(changed_file): def get_pr_titles(prs): - output = '' - for pr in prs: + output = [] + for idx, pr in enumerate(prs): pr_id = pr.split('/')[-1] r = requests.get(bugzilla_url % pr_id) bugs = r.json()['bugs'] if len(bugs) == 1: - output += '%s - %s\n' % (pr, bugs[0]['summary']) - print(output) + prs[idx] = 'PR %s/%s' % (bugs[0]['component'], pr_id) + out = '%s - %s\n' % (prs[idx], bugs[0]['summary']) + if out not in output: + output.append(out) if output: - output += '\n' - return output + output.append('') + return '\n'.join(output) -def generate_changelog(data, no_functions=False, fill_pr_titles=False): +def generate_changelog(data, no_functions=False, fill_pr_titles=False, + additional_prs=None): changelogs = {} changelog_list = [] prs = [] @@ -139,6 +143,8 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): diff = PatchSet(data) global firstpr + if additional_prs: + prs = [pr for pr in additional_prs if pr not in prs] for file in diff: # skip files that can't be parsed if file.path == '/dev/null': @@ -154,21 +160,33 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): # Only search first ten lines as later lines may # contains commented code which a note that it # has not been tested due to a certain PR or DR. + this_file_prs = [] for line in list(file)[0][0:10]: m = pr_regex.search(line.value) if m: pr = m.group('pr') if pr not in prs: prs.append(pr) + this_file_prs.append(pr.split('/')[-1]) else: m = dr_regex.search(line.value) if m: dr = m.group('dr') if dr not in prs: prs.append(dr) + this_file_prs.append(dr.split('/')[-1]) elif dg_regex.search(line.value): # Found dg-warning/dg-error line break + # PR number in the file name + fname = os.path.basename(file.path) + fname = os.path.splitext(fname)[0] + m = pr_filename_regex.search(fname) + if m: + pr = m.group('pr') + pr2 = "PR " + pr + if pr not in this_file_prs and pr2 not in prs: + prs.append(pr2) if prs: firstpr = prs[0] @@ -286,6 +304,8 @@ if __name__ == '__main__': parser = argparse.ArgumentParser(description=help_message) parser.add_argument('input', nargs='?', help='Patch file (or missing, read standard input)') + parser.add_argument('-b', '--pr-numbers', action='append', + help='Add the specified PRs (comma separated)') parser.add_argument('-s', '--no-functions', action='store_true', help='Do not generate function names in ChangeLogs') parser.add_argument('-p', '--fill-up-bug-titles', action='store_true', @@ -308,8 +328,11 @@ if __name__ == '__main__': if args.update_copyright: update_copyright(data) else: + pr_numbers = args.pr_numbers + if pr_numbers: + pr_numbers = [b for i in args.pr_numbers for b in i.split(',')] output = generate_changelog(data, args.no_functions, - args.fill_up_bug_titles) + args.fill_up_bug_titles, pr_numbers) if args.changelog: lines = open(args.changelog).read().split('\n') start = list(takewhile(lambda l: not l.startswith('#'), lines)) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch, v2] contrib/mklog.py: Improve PR handling (was: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog) 2021-06-21 7:54 ` [Patch, v2] contrib/mklog.py: Improve PR handling (was: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog) Tobias Burnus @ 2021-06-21 8:09 ` Martin Liška 2021-06-21 8:37 ` Tobias Burnus 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2021-06-21 8:09 UTC (permalink / raw) To: Tobias Burnus, Martin Sebor, Jason Merrill Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches Hi. I see the following warnings: $ pytest test_mklog.py FAILED test_mklog.py::TestMklog::test_sorting - AssertionError: assert '\n\tPR 50209...New test.\n\n' == 'gcc/ChangeLo...New test.\n\n' $ flake8 mklog.py mklog.py:187:23: Q000 Remove bad quotes and ... > contrib/mklog.py: Improve PR handling > > Co-authored-by: Martin Sebor <msebor@redhat.com> > > contrib/ChangeLog: > > * mklog.py (bugzilla_url): Fetch also component. > (pr_filename_regex): New. > (get_pr_titles): Update PR string with correct format and component. > (generate_changelog): Take additional PRs; extract PR from the > filename. > (__main__): Add -b/--pr-numbers argument. > > contrib/mklog.py | 41 ++++++++++++++++++++++++++++++++--------- > 1 file changed, 32 insertions(+), 9 deletions(-) > > diff --git a/contrib/mklog.py b/contrib/mklog.py > index 1f59055e723..bba6c1a0e1a 100755 > --- a/contrib/mklog.py > +++ b/contrib/mklog.py > @@ -42,6 +42,7 @@ pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)') > prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)') > dr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<dr>DR [0-9]+)') > dg_regex = re.compile(r'{\s+dg-(error|warning)') > +pr_filename_regex = re.compile(r'(^|[\W_])[Pp][Rr](?P<pr>\d{4,})') > identifier_regex = re.compile(r'^([a-zA-Z0-9_#].*)') > comment_regex = re.compile(r'^\/\*') > struct_regex = re.compile(r'^(class|struct|union|enum)\s+' > @@ -52,7 +53,7 @@ fn_regex = re.compile(r'([a-zA-Z_][^()\s]*)\s*\([^*]') > template_and_param_regex = re.compile(r'<[^<>]*>') > md_def_regex = re.compile(r'\(define.*\s+"(.*)"') > bugzilla_url = 'https://gcc.gnu.org/bugzilla/rest.cgi/bug?id=%s&' \ > - 'include_fields=summary' > + 'include_fields=summary,component' > > function_extensions = {'.c', '.cpp', '.C', '.cc', '.h', '.inc', '.def', '.md'} > > @@ -118,20 +119,23 @@ def sort_changelog_files(changed_file): > > > def get_pr_titles(prs): > - output = '' > - for pr in prs: > + output = [] > + for idx, pr in enumerate(prs): > pr_id = pr.split('/')[-1] > r = requests.get(bugzilla_url % pr_id) > bugs = r.json()['bugs'] > if len(bugs) == 1: > - output += '%s - %s\n' % (pr, bugs[0]['summary']) > - print(output) > + prs[idx] = 'PR %s/%s' % (bugs[0]['component'], pr_id) > + out = '%s - %s\n' % (prs[idx], bugs[0]['summary']) > + if out not in output: > + output.append(out) > if output: > - output += '\n' > - return output > + output.append('') > + return '\n'.join(output) > > > -def generate_changelog(data, no_functions=False, fill_pr_titles=False): > +def generate_changelog(data, no_functions=False, fill_pr_titles=False, > + additional_prs=None): > changelogs = {} > changelog_list = [] > prs = [] > @@ -139,6 +143,8 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): > diff = PatchSet(data) > global firstpr > > + if additional_prs: > + prs = [pr for pr in additional_prs if pr not in prs] > for file in diff: > # skip files that can't be parsed > if file.path == '/dev/null': > @@ -154,21 +160,33 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): > # Only search first ten lines as later lines may > # contains commented code which a note that it > # has not been tested due to a certain PR or DR. > + this_file_prs = [] > for line in list(file)[0][0:10]: > m = pr_regex.search(line.value) > if m: > pr = m.group('pr') > if pr not in prs: > prs.append(pr) > + this_file_prs.append(pr.split('/')[-1]) > else: > m = dr_regex.search(line.value) > if m: > dr = m.group('dr') > if dr not in prs: > prs.append(dr) > + this_file_prs.append(dr.split('/')[-1]) > elif dg_regex.search(line.value): > # Found dg-warning/dg-error line > break > + # PR number in the file name > + fname = os.path.basename(file.path) This is a dead code. > + fname = os.path.splitext(fname)[0] > + m = pr_filename_regex.search(fname) > + if m: > + pr = m.group('pr') > + pr2 = "PR " + pr Bad quotes here. > + if pr not in this_file_prs and pr2 not in prs: > + prs.append(pr2) > > if prs: > firstpr = prs[0] > @@ -286,6 +304,8 @@ if __name__ == '__main__': > parser = argparse.ArgumentParser(description=help_message) > parser.add_argument('input', nargs='?', > help='Patch file (or missing, read standard input)') > + parser.add_argument('-b', '--pr-numbers', action='append', > + help='Add the specified PRs (comma separated)') Do we really want to support '-b 1 -b 2' and also -b '1,2' formats? Seems to me quite complicated. Cheers, Martin > parser.add_argument('-s', '--no-functions', action='store_true', > help='Do not generate function names in ChangeLogs') > parser.add_argument('-p', '--fill-up-bug-titles', action='store_true', > @@ -308,8 +328,11 @@ if __name__ == '__main__': > if args.update_copyright: > update_copyright(data) > else: > + pr_numbers = args.pr_numbers > + if pr_numbers: > + pr_numbers = [b for i in args.pr_numbers for b in i.split(',')] > output = generate_changelog(data, args.no_functions, > - args.fill_up_bug_titles) > + args.fill_up_bug_titles, pr_numbers) > if args.changelog: > lines = open(args.changelog).read().split('\n') > start = list(takewhile(lambda l: not l.startswith('#'), lines)) On 6/21/21 9:54 AM, Tobias Burnus wrote: > On 17.06.21 02:17, Martin Sebor via Gcc wrote: >> @@ -147,6 +152,12 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): >> >> # Extract PR entries from newly added tests >> if 'testsuite' in file.path and file.is_added_file: >> + name = os.path.basename(file.path) >> + name = os.path.splitext(name)[0] >> + if name.startswith("pr"): >> + name = name[2:] >> + name = "PR " + name >> + prs.append(name) > > I think you need a regular expression to extract the PR – as it will both match > too much and to little. We have file names such as: > * libstdc++-pr91488.C (other prefix) > * PR37039.f90 (capitalized PR) > * pr98218-1.C (suffix with '-') > * pr40724_1.f (suffix with '_') > * pr101023a.C (suffix with a letter) > > But otherwise, I like that idea. > > * * * > > Changes in my patch compared to v1: > - (From Martin's patch:) Extract the PR from new-files file > name (using pattern matching), but only take the PR if the > PR wasn't found in the file as PR comment. > (The latter happens, e.g., with b376b1ef389.) > - Avoid printing the same PR multiple times as summary line > (duplicates occur due to 'PR 134' vs. 'PR comp/123' vs. > 'PR othercomp/123') — This does not avoid all issues but at least > some. If this becomes a real world issue, we can try harder. > > OK to commit this one? — Comments? > > * * * > > I did leave out other changes as they seem to be less clear cut, > and which can be still be handled as follow up. Like: > - Adding 'Resolves:' (as in some cases it only resolves part of > the PR) > - ... other changes/patches I missed. (This thread has too many > emails.) In particular, if > ^PR <comp>/<pr> - .... > is accepted by gcc-commit/, then there is no need to list the > PRs individually later on. But currently, it is still required. > > * * * > > Cross ref: > * v1 of my patch was at > https://gcc.gnu.org/pipermail/gcc/2021-June/236498.html > * Discussion of the -b option is at > https://gcc.gnu.org/pipermail/gcc/2021-June/236519.html > * Martin S's patch (partially quoted above) is at > https://gcc.gnu.org/pipermail/gcc/2021-June/236460.html > > Tobias > > ----------------- > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch, v2] contrib/mklog.py: Improve PR handling (was: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog) 2021-06-21 8:09 ` Martin Liška @ 2021-06-21 8:37 ` Tobias Burnus 2021-06-21 12:53 ` Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: Tobias Burnus @ 2021-06-21 8:37 UTC (permalink / raw) To: Martin Liška, Martin Sebor, Jason Merrill Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1548 bytes --] On 21.06.21 10:09, Martin Liška wrote: > $ pytest test_mklog.py > FAILED test_mklog.py::TestMklog::test_sorting - AssertionError: assert > '\n\tPR 50209...New test.\n\n' == 'gcc/ChangeLo...New test.\n\n' Aha, missed that there is indeed a testsuite - nice! > $ flake8 mklog.py > mklog.py:187:23: Q000 Remove bad quotes I have now filled: https://bugs.launchpad.net/ubuntu/+source/python-pytest-flake8/+bug/1933075 >> + # PR number in the file name >> + fname = os.path.basename(file.path) > > This is a dead code. > >> + fname = os.path.splitext(fname)[0] >> + m = pr_filename_regex.search(fname) It does not look like dead code to me. >> + parser.add_argument('-b', '--pr-numbers', action='append', >> + help='Add the specified PRs (comma separated)') > > Do we really want to support '-b 1 -b 2' and also -b '1,2' formats? > Seems to me quite > complicated. I don't have a strong opinion. I started with '-b 123,245', believing that the syntax is fine. But then I realized that without '-p' specifying multiple '-b' looks better by having multiple '-b' if 'PR <component>/' (needed for -p as the string is than taken as is). Thus, I ended up supporting either variant. But I also happily drop the ',' support. Change: One quote change, one test_mklog update. Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf [-- Attachment #2: mklog-p-v3.diff --] [-- Type: text/x-patch, Size: 5950 bytes --] contrib/mklog.py: Improve PR handling Co-authored-by: Martin Sebor <msebor@redhat.com> contrib/ChangeLog: * mklog.py (bugzilla_url): Fetch also component. (pr_filename_regex): New. (get_pr_titles): Update PR string with correct format and component. (generate_changelog): Take additional PRs; extract PR from the filename. (__main__): Add -b/--pr-numbers argument. * test_mklog.py (EXPECTED4): Update to expect a PR for the new file. contrib/mklog.py | 41 ++++++++++++++++++++++++++++++++--------- contrib/test_mklog.py | 3 +++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/contrib/mklog.py b/contrib/mklog.py index 1f59055e723..e49d14d0859 100755 --- a/contrib/mklog.py +++ b/contrib/mklog.py @@ -42,6 +42,7 @@ pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)') prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)') dr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<dr>DR [0-9]+)') dg_regex = re.compile(r'{\s+dg-(error|warning)') +pr_filename_regex = re.compile(r'(^|[\W_])[Pp][Rr](?P<pr>\d{4,})') identifier_regex = re.compile(r'^([a-zA-Z0-9_#].*)') comment_regex = re.compile(r'^\/\*') struct_regex = re.compile(r'^(class|struct|union|enum)\s+' @@ -52,7 +53,7 @@ fn_regex = re.compile(r'([a-zA-Z_][^()\s]*)\s*\([^*]') template_and_param_regex = re.compile(r'<[^<>]*>') md_def_regex = re.compile(r'\(define.*\s+"(.*)"') bugzilla_url = 'https://gcc.gnu.org/bugzilla/rest.cgi/bug?id=%s&' \ - 'include_fields=summary' + 'include_fields=summary,component' function_extensions = {'.c', '.cpp', '.C', '.cc', '.h', '.inc', '.def', '.md'} @@ -118,20 +119,23 @@ def sort_changelog_files(changed_file): def get_pr_titles(prs): - output = '' - for pr in prs: + output = [] + for idx, pr in enumerate(prs): pr_id = pr.split('/')[-1] r = requests.get(bugzilla_url % pr_id) bugs = r.json()['bugs'] if len(bugs) == 1: - output += '%s - %s\n' % (pr, bugs[0]['summary']) - print(output) + prs[idx] = 'PR %s/%s' % (bugs[0]['component'], pr_id) + out = '%s - %s\n' % (prs[idx], bugs[0]['summary']) + if out not in output: + output.append(out) if output: - output += '\n' - return output + output.append('') + return '\n'.join(output) -def generate_changelog(data, no_functions=False, fill_pr_titles=False): +def generate_changelog(data, no_functions=False, fill_pr_titles=False, + additional_prs=None): changelogs = {} changelog_list = [] prs = [] @@ -139,6 +143,8 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): diff = PatchSet(data) global firstpr + if additional_prs: + prs = [pr for pr in additional_prs if pr not in prs] for file in diff: # skip files that can't be parsed if file.path == '/dev/null': @@ -154,21 +160,33 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): # Only search first ten lines as later lines may # contains commented code which a note that it # has not been tested due to a certain PR or DR. + this_file_prs = [] for line in list(file)[0][0:10]: m = pr_regex.search(line.value) if m: pr = m.group('pr') if pr not in prs: prs.append(pr) + this_file_prs.append(pr.split('/')[-1]) else: m = dr_regex.search(line.value) if m: dr = m.group('dr') if dr not in prs: prs.append(dr) + this_file_prs.append(dr.split('/')[-1]) elif dg_regex.search(line.value): # Found dg-warning/dg-error line break + # PR number in the file name + fname = os.path.basename(file.path) + fname = os.path.splitext(fname)[0] + m = pr_filename_regex.search(fname) + if m: + pr = m.group('pr') + pr2 = 'PR ' + pr + if pr not in this_file_prs and pr2 not in prs: + prs.append(pr2) if prs: firstpr = prs[0] @@ -286,6 +304,8 @@ if __name__ == '__main__': parser = argparse.ArgumentParser(description=help_message) parser.add_argument('input', nargs='?', help='Patch file (or missing, read standard input)') + parser.add_argument('-b', '--pr-numbers', action='append', + help='Add the specified PRs (comma separated)') parser.add_argument('-s', '--no-functions', action='store_true', help='Do not generate function names in ChangeLogs') parser.add_argument('-p', '--fill-up-bug-titles', action='store_true', @@ -308,8 +328,11 @@ if __name__ == '__main__': if args.update_copyright: update_copyright(data) else: + pr_numbers = args.pr_numbers + if pr_numbers: + pr_numbers = [b for i in args.pr_numbers for b in i.split(',')] output = generate_changelog(data, args.no_functions, - args.fill_up_bug_titles) + args.fill_up_bug_titles, pr_numbers) if args.changelog: lines = open(args.changelog).read().split('\n') start = list(takewhile(lambda l: not l.startswith('#'), lines)) diff --git a/contrib/test_mklog.py b/contrib/test_mklog.py index a0670dac119..f5e9ecd577c 100755 --- a/contrib/test_mklog.py +++ b/contrib/test_mklog.py @@ -240,6 +240,9 @@ index 4ad78c1f77b..6687b368038 100644 ''' EXPECTED4 = '''\ + + PR 50209 + gcc/ChangeLog: * ipa-icf.c: ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch, v2] contrib/mklog.py: Improve PR handling (was: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog) 2021-06-21 8:37 ` Tobias Burnus @ 2021-06-21 12:53 ` Martin Liška 2021-06-21 13:26 ` Tobias Burnus 0 siblings, 1 reply; 10+ messages in thread From: Martin Liška @ 2021-06-21 12:53 UTC (permalink / raw) To: Tobias Burnus, Martin Sebor, Jason Merrill Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches On 6/21/21 10:37 AM, Tobias Burnus wrote: > On 21.06.21 10:09, Martin Liška wrote: > >> $ pytest test_mklog.py >> FAILED test_mklog.py::TestMklog::test_sorting - AssertionError: assert >> '\n\tPR 50209...New test.\n\n' == 'gcc/ChangeLo...New test.\n\n' > Aha, missed that there is indeed a testsuite - nice! >> $ flake8 mklog.py >> mklog.py:187:23: Q000 Remove bad quotes > I have now filled: > https://bugs.launchpad.net/ubuntu/+source/python-pytest-flake8/+bug/1933075 > >>> + # PR number in the file name >>> + fname = os.path.basename(file.path) >> >> This is a dead code. >> >>> + fname = os.path.splitext(fname)[0] >>> + m = pr_filename_regex.search(fname) > It does not look like dead code to me. Hello. The code is weird as os.path.basename returns: In [5]: os.path.basename('/tmp/a/b/c.txt') Out[5]: 'c.txt' why do you need os.path.splitext(fname) call? >>> + parser.add_argument('-b', '--pr-numbers', action='append', >>> + help='Add the specified PRs (comma separated)') >> >> Do we really want to support '-b 1 -b 2' and also -b '1,2' formats? >> Seems to me quite >> complicated. > > I don't have a strong opinion. I started with '-b 123,245', believing > that the syntax is fine. But then I realized that without '-p' > specifying multiple '-b' looks better by having multiple '-b' if 'PR > <component>/' (needed for -p as the string is than taken as is). Thus, > I ended up supporting either variant. I would start with -b 1,2,3,4 syntax. It will be likely easier for git alias integration. Martin > > But I also happily drop the ',' support. > > Change: One quote change, one test_mklog update. > > Tobias > > ----------------- > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch, v2] contrib/mklog.py: Improve PR handling (was: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog) 2021-06-21 12:53 ` Martin Liška @ 2021-06-21 13:26 ` Tobias Burnus 2021-06-22 7:30 ` [RFC][PATCH] contrib: add git-commit-mklog wrapper Martin Liška 0 siblings, 1 reply; 10+ messages in thread From: Tobias Burnus @ 2021-06-21 13:26 UTC (permalink / raw) To: Martin Liška, Martin Sebor, Jason Merrill Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1417 bytes --] Now committed as r12-1700-gedf0c3ffb59d75c11e05bc722432dc554e275c72 / as attached. (We had some follow-up discussion on IRC.) On 21.06.21 14:53, Martin Liška wrote: >>>> + # PR number in the file name >>>> + fname = os.path.basename(file.path) >>> >>> This is a dead code. >>> >>>> + fname = os.path.splitext(fname)[0] >>>> + m = pr_filename_regex.search(fname) (Meant was the 'splitext' line - it is dead code as the re.search globs all digits after 'pr' and then stops, ignoring the rest, including file extensions. – I first thought it referred to the basename line, which confused me.) >>>> + parser.add_argument('-b', '--pr-numbers', action='append', >>>> + help='Add the specified PRs (comma >>>> separated)') >>> >>> Do we really want to support '-b 1 -b 2' and also -b '1,2' formats? >>> Seems to me quite >>> complicated. > [...] > I would start with -b 1,2,3,4 syntax. It will be likely easier for git > alias integration. Done so. I note that argparse permits '-d dir1 -d dir2 -b bug1 -b bug2' which then only keeps the dir2 as directory and bug2 as bug without printing an error (or warning) for ignoring dir1 and bug1. Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf [-- Attachment #2: committed.diff --] [-- Type: text/x-patch, Size: 6041 bytes --] commit edf0c3ffb59d75c11e05bc722432dc554e275c72 Author: Tobias Burnus <tobias@codesourcery.com> Date: Mon Jun 21 15:17:22 2021 +0200 contrib/mklog.py: Improve PR handling Co-authored-by: Martin Sebor <msebor@redhat.com> contrib/ChangeLog: * mklog.py (bugzilla_url): Fetch also component. (pr_filename_regex): New. (get_pr_titles): Update PR string with correct format and component. (generate_changelog): Take additional PRs; extract PR from the filename. (__main__): Add -b/--pr-numbers argument. * test_mklog.py (EXPECTED4): Update to expect a PR for the new file. --- contrib/mklog.py | 38 +++++++++++++++++++++++++++++--------- contrib/test_mklog.py | 3 +++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/contrib/mklog.py b/contrib/mklog.py index 1f59055e723..0b434f67971 100755 --- a/contrib/mklog.py +++ b/contrib/mklog.py @@ -42,6 +42,7 @@ pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)') prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)') dr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<dr>DR [0-9]+)') dg_regex = re.compile(r'{\s+dg-(error|warning)') +pr_filename_regex = re.compile(r'(^|[\W_])[Pp][Rr](?P<pr>\d{4,})') identifier_regex = re.compile(r'^([a-zA-Z0-9_#].*)') comment_regex = re.compile(r'^\/\*') struct_regex = re.compile(r'^(class|struct|union|enum)\s+' @@ -52,7 +53,7 @@ fn_regex = re.compile(r'([a-zA-Z_][^()\s]*)\s*\([^*]') template_and_param_regex = re.compile(r'<[^<>]*>') md_def_regex = re.compile(r'\(define.*\s+"(.*)"') bugzilla_url = 'https://gcc.gnu.org/bugzilla/rest.cgi/bug?id=%s&' \ - 'include_fields=summary' + 'include_fields=summary,component' function_extensions = {'.c', '.cpp', '.C', '.cc', '.h', '.inc', '.def', '.md'} @@ -118,20 +119,23 @@ def sort_changelog_files(changed_file): def get_pr_titles(prs): - output = '' - for pr in prs: + output = [] + for idx, pr in enumerate(prs): pr_id = pr.split('/')[-1] r = requests.get(bugzilla_url % pr_id) bugs = r.json()['bugs'] if len(bugs) == 1: - output += '%s - %s\n' % (pr, bugs[0]['summary']) - print(output) + prs[idx] = 'PR %s/%s' % (bugs[0]['component'], pr_id) + out = '%s - %s\n' % (prs[idx], bugs[0]['summary']) + if out not in output: + output.append(out) if output: - output += '\n' - return output + output.append('') + return '\n'.join(output) -def generate_changelog(data, no_functions=False, fill_pr_titles=False): +def generate_changelog(data, no_functions=False, fill_pr_titles=False, + additional_prs=None): changelogs = {} changelog_list = [] prs = [] @@ -139,6 +143,8 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): diff = PatchSet(data) global firstpr + if additional_prs: + prs = [pr for pr in additional_prs if pr not in prs] for file in diff: # skip files that can't be parsed if file.path == '/dev/null': @@ -154,21 +160,32 @@ def generate_changelog(data, no_functions=False, fill_pr_titles=False): # Only search first ten lines as later lines may # contains commented code which a note that it # has not been tested due to a certain PR or DR. + this_file_prs = [] for line in list(file)[0][0:10]: m = pr_regex.search(line.value) if m: pr = m.group('pr') if pr not in prs: prs.append(pr) + this_file_prs.append(pr.split('/')[-1]) else: m = dr_regex.search(line.value) if m: dr = m.group('dr') if dr not in prs: prs.append(dr) + this_file_prs.append(dr.split('/')[-1]) elif dg_regex.search(line.value): # Found dg-warning/dg-error line break + # PR number in the file name + fname = os.path.basename(file.path) + m = pr_filename_regex.search(fname) + if m: + pr = m.group('pr') + pr2 = 'PR ' + pr + if pr not in this_file_prs and pr2 not in prs: + prs.append(pr2) if prs: firstpr = prs[0] @@ -286,6 +303,9 @@ if __name__ == '__main__': parser = argparse.ArgumentParser(description=help_message) parser.add_argument('input', nargs='?', help='Patch file (or missing, read standard input)') + parser.add_argument('-b', '--pr-numbers', action='store', + type=lambda arg: arg.split(','), nargs="?", + help='Add the specified PRs (comma separated)') parser.add_argument('-s', '--no-functions', action='store_true', help='Do not generate function names in ChangeLogs') parser.add_argument('-p', '--fill-up-bug-titles', action='store_true', @@ -309,7 +329,7 @@ if __name__ == '__main__': update_copyright(data) else: output = generate_changelog(data, args.no_functions, - args.fill_up_bug_titles) + args.fill_up_bug_titles, args.pr_numbers) if args.changelog: lines = open(args.changelog).read().split('\n') start = list(takewhile(lambda l: not l.startswith('#'), lines)) diff --git a/contrib/test_mklog.py b/contrib/test_mklog.py index a0670dac119..f5e9ecd577c 100755 --- a/contrib/test_mklog.py +++ b/contrib/test_mklog.py @@ -240,6 +240,9 @@ index 4ad78c1f77b..6687b368038 100644 ''' EXPECTED4 = '''\ + + PR 50209 + gcc/ChangeLog: * ipa-icf.c: ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC][PATCH] contrib: add git-commit-mklog wrapper 2021-06-21 13:26 ` Tobias Burnus @ 2021-06-22 7:30 ` Martin Liška 2021-06-22 8:23 ` Tobias Burnus 2021-06-22 18:40 ` Jason Merrill 0 siblings, 2 replies; 10+ messages in thread From: Martin Liška @ 2021-06-22 7:30 UTC (permalink / raw) To: Tobias Burnus, Martin Sebor, Jason Merrill Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches [-- Attachment #1: Type: text/plain, Size: 260 bytes --] Hello. There's a patch candidate that comes up with a wrapper for 'git commit-mklog' alias. Using my patch, one can do: $ git commit-mklog -a -b 12345,4444 Thoughts? Can one do that without the wrapper script and passing data through env. variable? Martin [-- Attachment #2: 0001-contrib-add-git-commit-mklog-wrapper.patch --] [-- Type: text/x-patch, Size: 3796 bytes --] From 6b63718e2836c1a5a63e476ea981ba65084ba867 Mon Sep 17 00:00:00 2001 From: Martin Liska <mliska@suse.cz> Date: Tue, 22 Jun 2021 09:19:45 +0200 Subject: [PATCH] contrib: add git-commit-mklog wrapper contrib/ChangeLog: * gcc-git-customization.sh: Use the new wrapper. * git-commit-mklog.py: New file. * prepare-commit-msg: Support GCC_MKLOG_ARGS. --- contrib/gcc-git-customization.sh | 2 +- contrib/git-commit-mklog.py | 44 ++++++++++++++++++++++++++++++++ contrib/prepare-commit-msg | 2 +- 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100755 contrib/git-commit-mklog.py diff --git a/contrib/gcc-git-customization.sh b/contrib/gcc-git-customization.sh index e7e66623786..6f8f23deebf 100755 --- a/contrib/gcc-git-customization.sh +++ b/contrib/gcc-git-customization.sh @@ -28,7 +28,7 @@ git config alias.gcc-undescr \!"f() { o=\$(git config --get gcc-config.upstream) git config alias.gcc-verify '!f() { "`git rev-parse --show-toplevel`/contrib/gcc-changelog/git_check_commit.py" $@; } ; f' git config alias.gcc-backport '!f() { "`git rev-parse --show-toplevel`/contrib/git-backport.py" $@; } ; f' git config alias.gcc-mklog '!f() { "`git rev-parse --show-toplevel`/contrib/mklog.py" $@; } ; f' -git config alias.gcc-commit-mklog '!f() { GCC_FORCE_MKLOG=1 git commit "$@"; }; f' +git config alias.gcc-commit-mklog '!f() { "`git rev-parse --show-toplevel`/contrib/git-commit-mklog.py" $@; }; f' # Make diff on MD files use "(define" as a function marker. # Use this in conjunction with a .gitattributes file containing diff --git a/contrib/git-commit-mklog.py b/contrib/git-commit-mklog.py new file mode 100755 index 00000000000..f1ef0dfa86b --- /dev/null +++ b/contrib/git-commit-mklog.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python3 + +# Copyright (C) 2020 Free Software Foundation, Inc. +# +# This file is part of GCC. +# +# GCC is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3, or (at your option) +# any later version. +# +# GCC is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING. If not, write to +# the Free Software Foundation, 51 Franklin Street, Fifth Floor, +# Boston, MA 02110-1301, USA. +# +# The script is wrapper for git commit-mklog alias where it parses +# -b/--pr-numbers argument and passes it via environment variable +# to mklog.py script. + +import argparse +import os +import subprocess + +if __name__ == '__main__': + children_args = [] + myenv = os.environ.copy() + + parser = argparse.ArgumentParser(description='git-commit-mklog wrapped') + parser.add_argument('-b', '--pr-numbers', action='store', + type=lambda arg: arg.split(','), nargs='?', + help='Add the specified PRs (comma separated)') + args, unknown_args = parser.parse_known_args() + + myenv['GCC_FORCE_MKLOG'] = '1' + if args.pr_numbers: + myenv['GCC_MKLOG_ARGS'] = f'-b {",".join(args.pr_numbers)}' + commit_args = ' '.join(unknown_args) + subprocess.run(f'git commit {commit_args}', shell=True, env=myenv) diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg index 969847df6f4..5da878458cd 100755 --- a/contrib/prepare-commit-msg +++ b/contrib/prepare-commit-msg @@ -78,4 +78,4 @@ else tee="cat" fi -git $cmd | $tee | git gcc-mklog -c "$COMMIT_MSG_FILE" +git $cmd | $tee | git gcc-mklog $GCC_MKLOG_ARGS -c "$COMMIT_MSG_FILE" -- 2.32.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] contrib: add git-commit-mklog wrapper 2021-06-22 7:30 ` [RFC][PATCH] contrib: add git-commit-mklog wrapper Martin Liška @ 2021-06-22 8:23 ` Tobias Burnus 2021-06-22 8:31 ` Martin Liška 2021-06-22 18:40 ` Jason Merrill 1 sibling, 1 reply; 10+ messages in thread From: Tobias Burnus @ 2021-06-22 8:23 UTC (permalink / raw) To: Martin Liška, Martin Sebor, Jason Merrill Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches Hello, On 22.06.21 09:30, Martin Liška wrote: > There's a patch candidate that comes up with a wrapper for 'git > commit-mklog' alias. > Using my patch, one can do: > $ git commit-mklog -a -b 12345, > Thoughts? What about '-p' – to fetch the data from GCC Bugzilla? I do note that 'git commit ' supports '-p, --patch' which may or may not be an issue. Tobias ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] contrib: add git-commit-mklog wrapper 2021-06-22 8:23 ` Tobias Burnus @ 2021-06-22 8:31 ` Martin Liška 0 siblings, 0 replies; 10+ messages in thread From: Martin Liška @ 2021-06-22 8:31 UTC (permalink / raw) To: Tobias Burnus, Martin Sebor, Jason Merrill Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches On 6/22/21 10:23 AM, Tobias Burnus wrote: > Hello, > > On 22.06.21 09:30, Martin Liška wrote: >> There's a patch candidate that comes up with a wrapper for 'git >> commit-mklog' alias. >> Using my patch, one can do: >> $ git commit-mklog -a -b 12345, >> Thoughts? > > What about '-p' – to fetch the data from GCC Bugzilla? Sure, that needs to be supported as well. > I do note that > 'git commit ' supports '-p, --patch' which may or may not be an issue. People likely do not use it with commit-mklog alias. Martin > > Tobias > > ----------------- > Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] contrib: add git-commit-mklog wrapper 2021-06-22 7:30 ` [RFC][PATCH] contrib: add git-commit-mklog wrapper Martin Liška 2021-06-22 8:23 ` Tobias Burnus @ 2021-06-22 18:40 ` Jason Merrill 2021-06-23 7:40 ` Martin Liška 1 sibling, 1 reply; 10+ messages in thread From: Jason Merrill @ 2021-06-22 18:40 UTC (permalink / raw) To: Martin Liška, Tobias Burnus, Martin Sebor Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches On 6/22/21 3:30 AM, Martin Liška wrote: > Hello. > > There's a patch candidate that comes up with a wrapper for 'git > commit-mklog' alias. > Using my patch, one can do: > > $ git commit-mklog -a -b 12345,4444 > > Thoughts? Looks good to me. > Can one do that without the wrapper script and passing data through env. > variable? The hook seems like the way to adjust the commit message, and we can't affect its command line arguments, so we're left with environment variables or a file somewhere for communicating to it. Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] contrib: add git-commit-mklog wrapper 2021-06-22 18:40 ` Jason Merrill @ 2021-06-23 7:40 ` Martin Liška 0 siblings, 0 replies; 10+ messages in thread From: Martin Liška @ 2021-06-23 7:40 UTC (permalink / raw) To: Jason Merrill, Tobias Burnus, Martin Sebor Cc: Jakub Jelinek, gcc Mailing List, Jonathan Wakely, gcc-patches On 6/22/21 8:40 PM, Jason Merrill wrote: > On 6/22/21 3:30 AM, Martin Liška wrote: >> Hello. >> >> There's a patch candidate that comes up with a wrapper for 'git commit-mklog' alias. >> Using my patch, one can do: >> >> $ git commit-mklog -a -b 12345,4444 >> >> Thoughts? > > Looks good to me. Cool, I've just pushed that (and started using it). Plus I've added support for -p argument as noted by Tobias. > >> Can one do that without the wrapper script and passing data through env. variable? > > The hook seems like the way to adjust the commit message, and we can't affect its command line arguments, so we're left with environment variables or a file somewhere for communicating to it. Yes, that's what I though. Martin > > Jason > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-06-23 7:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <YMHe/JbcYEm0HZ35@redhat.com> [not found] ` <CACb0b4=YG4p2ESJ-2QaObqwi8=JLEMGu9oRAi4JrWOyumo+8ZA@mail.gmail.com> [not found] ` <3228435b-aba0-6157-3266-c0f025822829@gmail.com> [not found] ` <5f89ddc0-aed4-2c20-0979-dfafb29046ee@gmail.com> [not found] ` <20210610173005.GI7746@tucnak> [not found] ` <e7663f13-e07c-01ed-d4c3-1500d0e52c6e@gmail.com> [not found] ` <20210610190941.GJ7746@tucnak> [not found] ` <cae4bbbc-3104-f01d-83c8-19ae740c3500@gmail.com> [not found] ` <CACb0b4kOavxRNMETn=UZx+kOabZ9wqaz7QpOLXpJ+LYdxvZWzw@mail.gmail.com> [not found] ` <58b63929-01f5-038c-931c-9ff8349d9f95@gmail.com> [not found] ` <CACb0b4nNAyhqJaap2N6XveABTUxKRew=knMDr6qZM1CZGOTwfw@mail.gmail.com> [not found] ` <fdee1b46-c44b-eb57-abd7-1ef3b4d2b756@gmail.com> [not found] ` <alpine.BSF.2.20.16.2106152045070.98326@arjuna.pair.com> [not found] ` <cf56da64-451b-d33f-278d-cbbfee28aaa7@gmail.com> [not found] ` <CADzB+2mO0hpCyeON0-6fM=ngTJSdxSceedyL+zbEDYmAyTUL9Q@mail.gmail.com> [not found] ` <c02221d2-8b74-bc19-4bed-68103efa31d0@redhat.com> [not found] ` <d1b914d8-ce31-5540-2d7f-76152b7b8444@gmail.com> [not found] ` <CADzB+2kCLfEcpYmco6QLyU69hTxw7zD=x5LZ09Ki+1qr6Oss_A@mail.gmail.com> [not found] ` <71b4a023-efb2-6c6a-9ced-93cce7c96540@gmail.com> 2021-06-21 7:54 ` [Patch, v2] contrib/mklog.py: Improve PR handling (was: Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog) Tobias Burnus 2021-06-21 8:09 ` Martin Liška 2021-06-21 8:37 ` Tobias Burnus 2021-06-21 12:53 ` Martin Liška 2021-06-21 13:26 ` Tobias Burnus 2021-06-22 7:30 ` [RFC][PATCH] contrib: add git-commit-mklog wrapper Martin Liška 2021-06-22 8:23 ` Tobias Burnus 2021-06-22 8:31 ` Martin Liška 2021-06-22 18:40 ` Jason Merrill 2021-06-23 7:40 ` Martin Liška
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).