-
Notifications
You must be signed in to change notification settings - Fork 127
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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''' | ||
|
||
|
@@ -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: | ||
path = os.path.normpath(path) | ||
for restricted_path in WEST_RESTRICTED_FOLDERS: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this be cleaner with a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because we can't do a simple
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strings need to be compared with |
||
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: | ||
|
@@ -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')) | ||
|
||
|
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 |
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 |
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 = '''\ | ||
|
There was a problem hiding this comment.
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?