Ignore /COMMIT_MSG in files matchers even more

We have mostly managed to ignore the /COMMIT_MSG in files matchers
(because it is unintuitive that it would be considered), but the recent
change to allow negated regexes in irrelevant-files exposed the fact
that it can still have an effect in that case.  The new feature hasn't
been used yet, so we could silently correct this, but a very similar
construction would be possible with the deprecated style of regex with
a negative lookahead.  Just in case someone wrote that, this change
includes a release note letting them know they can drop the /COMMIT_MSG
from their regex.

Change-Id: Ide04ed01224b5c0c48ab8d3c15ea7aef324cc42d
This commit is contained in:
James E. Blair 2024-04-17 14:33:22 -07:00
parent 4df7c8def9
commit 47591f086d
3 changed files with 21 additions and 13 deletions

View File

@ -0,0 +1,10 @@
---
fixes:
- |
The pseudo-file "/COMMIT_MSG" is not considered at all when
determining whether a change matches files or irrelevant-files
matchers. It was previously not considered in most cases, but in
the case of a negated regular expression in an irrelevant-files
stanza, it could still have an effect. This has been corrected
and any regexes that explicitly mention "/COMMIT_MSG" may remove
it.

View File

@ -26,5 +26,5 @@
jobs:
- project-test-files:
files:
- regex: ^(dontrun|README|/COMMIT_MSG)$
- regex: ^(dontrun|README)$
negate: true

View File

@ -18,10 +18,10 @@ This module defines classes used in matching changes based on job
configuration.
"""
import re
from zuul.lib.re2util import ZuulRegex
COMMIT_MSG = '/COMMIT_MSG'
class AbstractChangeMatcher(object):
"""An abstract class that matches change attributes against regular
@ -135,8 +135,6 @@ class AbstractMatcherCollection(AbstractChangeMatcher):
class AbstractMatchFiles(AbstractMatcherCollection):
commit_regex = re.compile('^/COMMIT_MSG$')
@property
def regexes(self):
for matcher in self.matchers:
@ -148,18 +146,17 @@ class MatchAllFiles(AbstractMatchFiles):
def matches(self, change):
# NOTE(yoctozepto): make irrelevant files matcher match when
# there are no files to check - return False (NB: reversed)
if not (hasattr(change, 'files') and change.files):
if not hasattr(change, 'files'):
return False
if len(change.files) == 1 and self.commit_regex.match(change.files[0]):
files = [c for c in change.files if c != COMMIT_MSG]
if not files:
return False
for file_ in change.files:
for file_ in files:
matched_file = False
for regex in self.regexes:
if regex.match(file_):
matched_file = True
break
if self.commit_regex.match(file_):
matched_file = True
if not matched_file:
return False
return True
@ -170,11 +167,12 @@ class MatchAnyFiles(AbstractMatchFiles):
def matches(self, change):
# NOTE(yoctozepto): make files matcher match when
# there are no files to check - return True
if not (hasattr(change, 'files') and change.files):
if not hasattr(change, 'files'):
return True
if len(change.files) == 1 and self.commit_regex.match(change.files[0]):
files = [c for c in change.files if c != COMMIT_MSG]
if not files:
return True
for file_ in change.files:
for file_ in files:
for regex in self.regexes:
if regex.match(file_):
return True