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

project: Restricting projects from being located in west folder #132

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/west/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
'''Names of the special "meta-projects", which are reserved and cannot
be used to name a project in the manifest file.'''

WEST_RESTRICTED_FOLDERS = ['west']
'''Restricted folders that are used by "west" itself for "meta-projects"'''

MANIFEST_SECTIONS = ['manifest', 'west']
'''Sections in the manifest file'''

Expand Down Expand Up @@ -216,6 +219,18 @@ def _load(self, data, sections):
format(name) +
'be used to name a manifest project')

path = mp.get('path')
if path is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

If not given, the implied path is the name of the project. What if I have a project named west/subproject or so? Do we still fail?

If so, would you mind adding some test coverage (another invalid_xxx.yaml should do it) to show that is true?

path = os.path.normpath(path)
for restricted_path in WEST_RESTRICTED_FOLDERS:
Copy link
Member

Choose a reason for hiding this comment

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

Would this be cleaner with a if X in Y construction?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because we can't do a simple X in Y here.

west is restricted, but it could be that manifest states west/subfolder and that is also forbidden, and wouldn't be matched in a X in Y.
Thus we loop over WEST_RESTRICTED_FOILDERS to see if there is a match on commonpath between restricted and the path specified for the folder.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

commonpath now changed to commonprefix to support 3.4, as commonpath is new in python 3.5

# Appending os.path.sep to ensure that eg. restricted path 'west/' and
# 'west-ext/' common prefix does not match on the restricted path 'west/'
restricted_path += os.path.sep
if os.path.commonprefix([restricted_path, path + os.path.sep]) is restricted_path:
Copy link
Contributor

Choose a reason for hiding this comment

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

Strings need to be compared with ==, not is.

self._malformed('the folder "{}" is restricted and '.
format(restricted_path) +
'cannot be used for local projects')

# Validate the project remote.
remote_name = mp.get('remote', default_remote_name)
if remote_name is None:
Expand All @@ -227,7 +242,7 @@ def _load(self, data, sections):
project = Project(name,
remotes_dict[remote_name],
defaults,
path=mp.get('path'),
path=path,
clone_depth=mp.get('clone-depth'),
revision=mp.get('revision'))

Expand Down
11 changes: 11 additions & 0 deletions tests/west/manifest/invalid_restricted_project_normalized_path.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
manifest:
defaults:
remote: testremote

remotes:
- name: testremote
url-base: https://example.com

projects:
- name: foo1
path: foo/../west
11 changes: 11 additions & 0 deletions tests/west/manifest/invalid_restricted_project_path.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
manifest:
defaults:
remote: testremote

remotes:
- name: testremote
url-base: https://example.com

projects:
- name: foo1
path: west
11 changes: 11 additions & 0 deletions tests/west/manifest/invalid_restricted_project_sub_path.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
manifest:
defaults:
remote: testremote

remotes:
- name: testremote
url-base: https://example.com

projects:
- name: foo1
path: west/foo
23 changes: 23 additions & 0 deletions tests/west/manifest/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,29 @@ def test_path():
assert manifest.projects[0].path == 'sub/directory'
assert manifest.projects[0].abspath == os.path.realpath('/west_top/sub/directory')

def test_restricted_path():
# Projects are restricted from west/ folder, but folders containing west as
# part of path name, e.g. west-ext arte still allowed.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: s/arte/are/

content = '''\
manifest:
remotes:
- name: testremote
url-base: https://example.com
projects:
- name: testproject
remote: testremote
path: west-ext
- name: testsubproject
remote: testremote
path: west-ext/sub
'''
with patch('west.util.west_topdir', return_value=os.path.realpath('/west_top')):
manifest = Manifest.from_data(yaml.safe_load(content))
assert manifest.projects[0].path == 'west-ext'
assert manifest.projects[0].abspath == os.path.realpath('/west_top/west-ext')
assert manifest.projects[1].path == 'west-ext/sub'
assert manifest.projects[1].abspath == os.path.realpath('/west_top/west-ext/sub')

def test_sections():
# Projects must be able to override their default paths.
content_wrong_west = '''\
Expand Down