gcc-changelog: Catch forbidden files.
Commit Message
Hi.
The patch is about FORBIDDEN_FILES that we don't want to be commited.
So far we have $root/Makefile.am and $root/build.log.
Do you have any other candidates?
Cheers,
Martin
contrib/ChangeLog:
* gcc-changelog/git_commit.py: Add FORBIDDEN_FILES.
* gcc-changelog/test_email.py: Test it.
* gcc-changelog/test_patches.txt: Add test for it.
---
contrib/gcc-changelog/git_commit.py | 10 ++++++++++
contrib/gcc-changelog/test_email.py | 4 ++++
contrib/gcc-changelog/test_patches.txt | 21 +++++++++++++++++++++
3 files changed, 35 insertions(+)
Comments
On Mon, Jan 24, 2022 at 12:28:53PM +0100, Martin Liška wrote:
> The patch is about FORBIDDEN_FILES that we don't want to be commited.
> So far we have $root/Makefile.am and $root/build.log.
>
> Do you have any other candidates?
I think it shouldn't be about blacklisting all the mistakes from the past.
Instead, I think we should reject any new file additions to the toplevel
unless they are clearly mentioned in the entry for the toplevel ChangeLog
file (and as toplevel Changes are rare, we could be more strict for that
and require that one actually uses a syntax that makes it clear it is for
toplevel. So that when one does:
date name email
* foo.xyz: New file.
and means to add file in say gcc/ dir and adds it also or just in the
toplevel instead, we catch it.
ChangeLog:
is certainly one explicit syntax for toplevel dir, dunno if we have others.
Jakub
On 1/24/22 12:40, Jakub Jelinek wrote:
> On Mon, Jan 24, 2022 at 12:28:53PM +0100, Martin Liška wrote:
>> The patch is about FORBIDDEN_FILES that we don't want to be commited.
>> So far we have $root/Makefile.am and $root/build.log.
>>
>> Do you have any other candidates?
>
> I think it shouldn't be about blacklisting all the mistakes from the past.
> Instead, I think we should reject any new file additions to the toplevel
> unless they are clearly mentioned in the entry for the toplevel ChangeLog
> file (and as toplevel Changes are rare, we could be more strict for that
> and require that one actually uses a syntax that makes it clear it is for
> toplevel. So that when one does:
> date name email
>
> * foo.xyz: New file.
> and means to add file in say gcc/ dir and adds it also or just in the
> toplevel instead, we catch it.
> ChangeLog:
> is certainly one explicit syntax for toplevel dir, dunno if we have others.
>
> Jakub
>
Makes sense, implemented in the attached patch.
Ready to be installed?
Thanks,
Martin
On Mon, Jan 24, 2022 at 01:06:25PM +0100, Martin Liška wrote:
> Makes sense, implemented in the attached patch.
>
> Ready to be installed?
LGTM.
> From 55e9774ba91e0acf12102d7c595482c29d5397cb Mon Sep 17 00:00:00 2001
> From: Martin Liska <mliska@suse.cz>
> Date: Mon, 24 Jan 2022 13:03:01 +0100
> Subject: [PATCH] gcc-changelog: Be stricter for top-level dir.
>
> contrib/ChangeLog:
>
> * gcc-changelog/git_commit.py: New files in toplev must
> be explicitly marked as "New file".
> * gcc-changelog/test_email.py: Test.
> * gcc-changelog/test_patches.txt: Add test.
> ---
> contrib/gcc-changelog/git_commit.py | 12 +++-
> contrib/gcc-changelog/test_email.py | 5 ++
> contrib/gcc-changelog/test_patches.txt | 82 ++++++++++++++++++++++++++
> 3 files changed, 96 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/gcc-changelog/git_commit.py b/contrib/gcc-changelog/git_commit.py
> index 27a1d59b211..95dc49e5d48 100755
> --- a/contrib/gcc-changelog/git_commit.py
> +++ b/contrib/gcc-changelog/git_commit.py
> @@ -718,9 +718,15 @@ class GitCommit:
> self.changelog_entries.append(entry)
> # strip prefix of the file
> assert file.startswith(entry.folder)
> - file = file[len(entry.folder):].lstrip('/')
> - entry.lines.append('\t* %s: New file.' % file)
> - entry.files.append(file)
> + # do not allow auto-addition of New files
> + # for the top-level folder
> + if entry.folder:
> + file = file[len(entry.folder):].lstrip('/')
> + entry.lines.append('\t* %s: New file.' % file)
> + entry.files.append(file)
> + else:
> + msg = 'new file in the top-level folder not mentioned in a ChangeLog'
> + self.errors.append(Error(msg, file))
> else:
> used_pattern = [p for p in mentioned_patterns
> if file.startswith(p)]
> diff --git a/contrib/gcc-changelog/test_email.py b/contrib/gcc-changelog/test_email.py
> index a4796dbbe94..c56f6da513a 100755
> --- a/contrib/gcc-changelog/test_email.py
> +++ b/contrib/gcc-changelog/test_email.py
> @@ -446,3 +446,8 @@ class TestGccChangelog(unittest.TestCase):
> email = self.from_patch_glob('non-ascii-email.patch')
> assert (email.errors[0].message ==
> 'non-ASCII characters in git commit email address (jbglaw@ług-owl.de)')
> +
> + def test_new_file_in_root_folder(self):
> + email = self.from_patch_glob('toplev-new-file.patch')
> + assert (email.errors[0].message ==
> + 'new file in the top-level folder not mentioned in a ChangeLog')
> diff --git a/contrib/gcc-changelog/test_patches.txt b/contrib/gcc-changelog/test_patches.txt
> index 98a0d3f1ee0..95ad961f2d3 100644
> --- a/contrib/gcc-changelog/test_patches.txt
> +++ b/contrib/gcc-changelog/test_patches.txt
> @@ -3489,3 +3489,85 @@ index 2a9917cde62..0033b0004b3 100644
> @@ -0,0 +1,1 @@
> +
> --
> +
> +=== toplev-new-file.patch ===
> +From 05e37b6e65027188f08e6374c7d356d75b54738e Mon Sep 17 00:00:00 2001
> +From: Martin Liska <mliska@suse.cz>
> +Date: Mon, 24 Jan 2022 12:46:27 +0100
> +Subject: [PATCH] New file.
> +
> +ChangeLog:
> +
> + * Makefile.in: Update.
> +
> +gcc/ChangeLog:
> +
> + * ipa-icf.cc: Update.
> +---
> + Makefile.am | 1 +
> + Makefile.in | 1 +
> + gcc/ipa-icf.cc | 1 +
> + gcc/ipa-icf2.cc | 1 +
> + 4 files changed, 4 insertions(+)
> + create mode 100644 Makefile.am
> + create mode 100644 gcc/ipa-icf2.cc
> +
> +diff --git a/Makefile.am b/Makefile.am
> +new file mode 100644
> +index 00000000000..f0129caae3d
> +--- /dev/null
> ++++ b/Makefile.am
> +@@ -0,0 +1 @@
> ++new file.
> +diff --git a/Makefile.in b/Makefile.in
> +index 79c77fccf0f..7a090030119 100644
> +--- a/Makefile.in
> ++++ b/Makefile.in
> +@@ -1,4 +1,5 @@
> +
> ++
> + # Makefile.in is generated from Makefile.tpl by 'autogen Makefile.def'.
> + #
> + # Makefile for directory with subdirs to build.
> +diff --git a/gcc/ipa-icf.cc b/gcc/ipa-icf.cc
> +index 765ae746745..15735b6684f 100644
> +--- a/gcc/ipa-icf.cc
> ++++ b/gcc/ipa-icf.cc
> +@@ -1,3 +1,4 @@
> ++
> + /* Interprocedural Identical Code Folding pass
> + Copyright (C) 2014-2022 Free Software Foundation, Inc.
> +
> +diff --git a/gcc/ipa-icf2.cc b/gcc/ipa-icf2.cc
> +new file mode 100644
> +index 00000000000..c49c556e0e4
> +--- /dev/null
> ++++ b/gcc/ipa-icf2.cc
> +@@ -0,0 +1 @@
> ++tt
> +--
> +2.34.1
> +
> +From 80c9d63af350b280bfccb82adb3867c25a25e6d0 Mon Sep 17 00:00:00 2001
> +From: Martin Liska <mliska@suse.cz>
> +Date: Mon, 24 Jan 2022 12:17:09 +0100
> +Subject: [PATCH] Add Makefile.am file.
> +
> +Foo bar.
> +
> +ChangeLog:
> +
> +---
> + Makefile.am | 0
> + 1 file changed, 0 insertions(+), 0 deletions(-)
> + create mode 100644 Makefile.am
> +
> +diff --git a/Makefile.am b/Makefile.am
> +new file mode 100644
> +index 00000000000..d6459e00543
> +--- /dev/null
> ++++ b/Makefile.am
> +@@ -0,0 +1 @@
> ++xxx
> +--
> +2.34.1
> --
> 2.34.1
>
Jakub
@@ -153,6 +153,11 @@ misc_files = {
'gcc/DEV-PHASE'
}
+FORBIDDEN_FILES = {
+ 'Makefile.am',
+ 'build.log'
+}
+
author_line_regex = \
re.compile(r'^(?P<datetime>\d{4}-\d{2}-\d{2})\ {2}(?P<name>.* <.*>)')
additional_author_regex = re.compile(r'^\t(?P<spaces>\ *)?(?P<name>.* <.*>)')
@@ -339,6 +344,11 @@ class GitCommit:
self.errors.append(Error('invalid PR component in subject', info.lines[0]))
self.subject_prs.add(m.group('pr'))
+ for f in self.info.modified_files:
+ if f[1] != 'D':
+ if f[0] in FORBIDDEN_FILES:
+ self.errors.append(Error('forbidden file name', f[0]))
+
# Allow complete deletion of ChangeLog files in a commit
project_files = [f for f in self.info.modified_files
if (self.is_changelog_filename(f[0], allow_suffix=True) and f[1] != 'D')
@@ -446,3 +446,7 @@ class TestGccChangelog(unittest.TestCase):
email = self.from_patch_glob('non-ascii-email.patch')
assert (email.errors[0].message ==
'non-ASCII characters in git commit email address (jbglaw@ług-owl.de)')
+
+ def test_forbidden_file(self):
+ email = self.from_patch_glob('Makefile.am.patch')
+ assert (email.errors[0].message == 'forbidden file name')
@@ -3489,3 +3489,24 @@ index 2a9917cde62..0033b0004b3 100644
@@ -0,0 +1,1 @@
+
--
+
+=== Makefile.am.patch ===
+From 80c9d63af350b280bfccb82adb3867c25a25e6d0 Mon Sep 17 00:00:00 2001
+From: Martin Liska <mliska@suse.cz>
+Date: Mon, 24 Jan 2022 12:17:09 +0100
+Subject: [PATCH] Add Makefile.am file.
+
+---
+ Makefile.am | 0
+ 1 file changed, 0 insertions(+), 0 deletions(-)
+ create mode 100644 Makefile.am
+
+diff --git a/Makefile.am b/Makefile.am
+new file mode 100644
+index 00000000000..d6459e00543
+--- /dev/null
++++ b/Makefile.am
+@@ -0,0 +1 @@
++xxx
+--
+2.34.1