Skip to content

Commit

Permalink
manifest: change project-filter semantics
Browse files Browse the repository at this point in the history
Make this option consistent with all others by only having the highest
precedence configuration option take effect. We revisited this
decision following review.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
  • Loading branch information
mbolivar-nordic authored and carlescufi committed Jun 2, 2023
1 parent 50c316a commit 31ac603
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 45 deletions.
51 changes: 15 additions & 36 deletions src/west/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,24 +129,19 @@ class ProjectFilterElt(NamedTuple):
ProjectFilterType = List[ProjectFilterElt]

def _update_project_filter(project_filter: ProjectFilterType,
option_value: Optional[str],
configfile: ConfigFile,
name: str) -> None:
option_value: Optional[str]) -> None:
# Validate a 'manifest.project-filter' configuration option's
# value. The 'option_value' argument is the raw configuration
# option. If 'option_value' is invalid, error out. Otherwise,
# destructively modify 'project_filter' to reflect the option's
# value.
#
# The 'configfile' and 'name' arguments are just for error
# reporting.

if option_value is None:
return

def _err(message):
raise MalformedConfig(
f'invalid {name} "manifest.project-filter" option value '
f'invalid "manifest.project-filter" option value '
f'"{option_value}": {message}')

for elt in option_value.split(','):
Expand Down Expand Up @@ -1662,24 +1657,25 @@ def is_active(self, project: Project,
extra_filter: Optional[Iterable[str]] = None) -> bool:
'''Is a project active?
If the manifest.project-filter configuration option is set
in any of the system, global, or local configuration files,
and one or more of its elements apply to the project's name,
the return value determined by the option's values in these files:
If the manifest.project-filter configuration option is set,
the return value determined by the option's value:
- The elements of the manifest.project-filter options in each
of these files are checked against the project's name. If the
regular expression in the element matches the project's name,
then the project is active or inactive depending on if the
element begins with + or - respectively.
- The elements of the manifest.project-filter value are checked
against the project's name. If the regular expression in the
element matches the project's name, then the project is active
or inactive depending on if the element begins with + or -
respectively.
- If multiple elements have regular expressions matching the
project's name, the last element which has a match determines
the result.
- This function returns True if the project is active or
- This function returns True or False if the project is active or
inactive according to these rules.
The manifest.project-filter value that was set at the time
this Manifest object was constructed is used.
Otherwise, the return value depends on whether the project has
any groups, and if so, whether they are enabled:
Expand Down Expand Up @@ -1891,25 +1887,8 @@ def get_option(option, default=None):
self.repo_abspath = os.fspath(current_repo_abspath)
self._raw_config_group_filter = get_option('manifest.group-filter')
self._config_path = manifest_path

def project_filter_val(configfile) -> Optional[str]:
return config.get('manifest.project-filter',
configfile=configfile)

# Update our project filter based on the value in all the
# configuration files. This allows us to progressively
# build up a project filter with default settings in
# configuration files with wider scope, refining as
# needed. For example, you could do '-foo,-bar' in the
# global config file, and then '+bar' in the local config
# file, to have a final filter of '-foo'.
for configfile, name in [(ConfigFile.SYSTEM, 'system'),
(ConfigFile.GLOBAL, 'global'),
(ConfigFile.LOCAL, 'local')]:
_update_project_filter(project_filter,
project_filter_val(configfile),
configfile,
name)
_update_project_filter(project_filter,
config.get('manifest.project-filter'))

return _import_ctx(projects={},
project_filter=project_filter,
Expand Down
18 changes: 9 additions & 9 deletions tests/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1313,9 +1313,9 @@ def clean_up_config_files():
pass

def check_error(project_filter, expected_err_contains):
for configfile, name in [(ConfigFile.SYSTEM, 'system'),
(ConfigFile.GLOBAL, 'global'),
(ConfigFile.LOCAL, 'local')]:
for configfile in [ConfigFile.SYSTEM,
ConfigFile.GLOBAL,
ConfigFile.LOCAL]:
clean_up_config_files()
config.set('manifest.project-filter', project_filter,
configfile=configfile)
Expand All @@ -1324,7 +1324,7 @@ def check_error(project_filter, expected_err_contains):
MT(topdir=topdir)

err = str(e.value)
assert (f'invalid {name} "manifest.project-filter" option value '
assert (f'invalid "manifest.project-filter" option value '
f'"{project_filter}":') in err
assert expected_err_contains in err

Expand Down Expand Up @@ -1424,16 +1424,16 @@ def test_project_filter_precedence(config_tmpdir):
# Global has higher precedence than system.
config.set('manifest.project-filter', '-foo,-bar,-baz',
configfile=ConfigFile.SYSTEM)
config.set('manifest.project-filter', '+foo',
config.set('manifest.project-filter', '-foo',
configfile=ConfigFile.GLOBAL)
manifest = Manifest.from_topdir(topdir=topdir, config=config)
foo, bar, baz = manifest.get_projects(['foo', 'bar', 'baz'])
assert manifest.is_active(foo)
assert not manifest.is_active(bar)
assert not manifest.is_active(baz)
assert not manifest.is_active(foo)
assert manifest.is_active(bar)
assert manifest.is_active(baz)

# Local has higher precedence than either.
config.set('manifest.project-filter', '+baz,-f.*',
config.set('manifest.project-filter', '-bar,-f.*',
configfile=ConfigFile.LOCAL)
manifest = Manifest.from_topdir(topdir=topdir, config=config)
foo, bar, baz = manifest.get_projects(['foo', 'bar', 'baz'])
Expand Down

0 comments on commit 31ac603

Please sign in to comment.