Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add build system support for un-ignore rules #12965

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions tools/resources/__init__.py
Expand Up @@ -434,8 +434,6 @@ def add_directory(
root_path = join(relpath(root, base_path))
if self._ignoreset.is_ignored(join(root_path,"")):
self.ignore_dir(join(into_path, root_path))
dirs[:] = []
continue

for d in copy(dirs):
dir_path = join(root, d)
Expand All @@ -451,14 +449,22 @@ def add_directory(
relpath(dir_path, base_path)
))
dirs.remove(d)
elif (d.startswith('.') or d in self._legacy_ignore_dirs or
self._ignoreset.is_ignored(join(root_path, d, ""))):

# hardcoded ignore rules -- don't want to visit any of these subdirs
elif d.startswith('.') or d in self._legacy_ignore_dirs:
self.ignore_dir(join(
into_path,
relpath(dir_path, base_path)
))
dirs.remove(d)

# manual ignore rules -- may want to visit subdirs if they have unignored files
elif self._ignoreset.is_ignored(join(root_path, d, "")):
self.ignore_dir(join(
into_path,
relpath(dir_path, base_path)
))

# Add root to include paths
root = root.rstrip("/")

Expand Down
102 changes: 91 additions & 11 deletions tools/resources/ignore.py
Expand Up @@ -31,11 +31,46 @@ class MbedIgnoreSet(object):

def __init__(self):
self._ignore_patterns = []
self._ignore_regex = re.compile("$^")
self._ignore_regexes = []
self._unignore_patterns = []
self._unignore_regexes = []

def is_ignored(self, file_path):
"""Check if file path is ignored by any .mbedignore thus far"""
return self._ignore_regex.match(normcase(file_path))

# find longest ignore and unignore pattern that matches the path
ignore_match_pattern = None
unignore_match_pattern = None

filepath_normcase = normcase(file_path)

for regex_index in range(0, len(self._ignore_regexes)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use zip here to tidy this loop up? Then we can say something like:

for ignore_regex, ignore_pattern in zip(self._ignore_regexes, self._ignore_patterns):
    this_regex_match = ignore_regex.match(filepath_normcase)
    ...

This way we have both of the elements there without having to introduce the extra regex_index variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh you can tell I'm mostly a C++ programmer... I had no idea this existed

this_regex_match = self._ignore_regexes[regex_index].match(filepath_normcase)
if this_regex_match:
if ignore_match_pattern is None:
# no previous match
ignore_match_pattern = self._ignore_patterns[regex_index]
elif len(self._ignore_patterns[regex_index]) > len(ignore_match_pattern):
# found a longer match
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to see a comment explaining why a longer match is important, rather than just repeating what the code says.

ignore_match_pattern = self._ignore_patterns[regex_index]

for regex_index in range(0, len(self._unignore_regexes)):
this_regex_match = self._unignore_regexes[regex_index].match(filepath_normcase)
if this_regex_match:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to factor out this duplicate logic into a helper function?

if unignore_match_pattern is None:
# no previous match
unignore_match_pattern = self._unignore_patterns[regex_index]
elif len(self._unignore_patterns[regex_index]) > len(unignore_match_pattern):
# found a longer match
unignore_match_pattern = self._unignore_patterns[regex_index]

if ignore_match_pattern is None:
return False

if unignore_match_pattern is not None and len(unignore_match_pattern) >= len(ignore_match_pattern):
return False

return True

def add_ignore_patterns(self, in_name, patterns):
"""Ignore all files and directories matching the paterns in
Expand All @@ -45,14 +80,42 @@ def add_ignore_patterns(self, in_name, patterns):
in_name - the filename prefix that this ignore will apply to
patterns - the list of patterns we will ignore in the future
"""

if len(patterns) == 0:
return

if in_name == ".":
self._ignore_patterns.extend(normcase(p) for p in patterns)
patterns_normpath = list(normcase(p) for p in patterns)
else:
self._ignore_patterns.extend(
normcase(join(in_name, pat)) for pat in patterns)
if self._ignore_patterns:
self._ignore_regex = re.compile("|".join(
fnmatch.translate(p) for p in self._ignore_patterns))
patterns_normpath = list(normcase(join(in_name, pat)) for pat in patterns)

self._ignore_patterns.extend(patterns_normpath)
self._ignore_regexes.extend(re.compile(fnmatch.translate(p)) for p in patterns_normpath)


def add_unignore_patterns(self, in_name, patterns):
"""Un-ignore all files and directories matching the patterns in
directories named by in_name.

Un-ignore rules take precedence based on depth. So ignoring
a/b/* then unignoring a/b/c.cpp would allow c.cpp to build.
But unignoring a/* then ignoring a/b/* would prevent c.cpp from building.

Positional arguments:
in_name - the filename prefix that this unignore will apply to
patterns - the list of patterns we will unignore in the future
"""

if len(patterns) == 0:
return

if in_name == ".":
patterns_normpath = list(normcase(p) for p in patterns)
else:
patterns_normpath = list(normcase(join(in_name, pat)) for pat in patterns)

self._unignore_patterns.extend(patterns_normpath)
self._unignore_regexes.extend(re.compile(fnmatch.translate(p)) for p in patterns_normpath)

def add_mbedignore(self, in_name, filepath):
"""Add a series of patterns to the ignored paths
Expand All @@ -62,8 +125,25 @@ def add_mbedignore(self, in_name, filepath):
patterns - the list of patterns we will ignore in the future
"""
with open (filepath) as f:
patterns = [l.strip() for l in f
if l.strip() != "" and not l.startswith("#")]
self.add_ignore_patterns(in_name, patterns)

ignore_patterns = []
unignore_patterns = []

for line in f:
line_text = line.strip()

if line_text == "" or line_text.startswith("#"):
# no content on this line
continue

if line_text.startswith("!"):
# unignore rule
unignore_patterns.append(line_text[1:])
else:
# ignore rule
ignore_patterns.append(line_text)

self.add_ignore_patterns(in_name, ignore_patterns)
self.add_unignore_patterns(in_name, unignore_patterns)