-
Notifications
You must be signed in to change notification settings - Fork 126
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
manifest: Do not allow projects inside the west directory #754
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 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -43,7 +43,7 @@ | |||||||||||
from typing import Any, Dict, Iterable, List, Optional, Tuple, TYPE_CHECKING | ||||||||||||
import warnings | ||||||||||||
|
||||||||||||
from west.util import west_dir, WestNotFound, PathType | ||||||||||||
from west.util import WEST_DIR, west_dir, WestNotFound, PathType | ||||||||||||
|
||||||||||||
class MalformedConfig(Exception): | ||||||||||||
'''The west configuration was malformed. | ||||||||||||
|
@@ -616,7 +616,7 @@ def _location(cfg: ConfigFile, topdir: Optional[PathType] = None, | |||||||||||
return env['WEST_CONFIG_LOCAL'] | ||||||||||||
|
||||||||||||
if topdir: | ||||||||||||
return os.fspath(Path(topdir) / '.west' / 'config') | ||||||||||||
return os.fspath(Path(topdir) / WEST_DIR / 'config') | ||||||||||||
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.
This means we can have a uniform way to always fetch Doing so will also remove the need of
Suggested change
Ref: Lines 71 to 73 in c3aadf5
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. Thanks, applied your suggestion here. 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. Had to revert here, it's possible that the path does not exist yet. So it would require more changes. I'll create a follow-up PR for v1.4 or v2. 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.
As usual, initialization/"bootstrapping" is at the top of complex situations. Always underestimated :-) |
||||||||||||
|
||||||||||||
if find_local: | ||||||||||||
# Might raise WestNotFound! | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# Project paths can't be .west. | ||
|
||
manifest: | ||
projects: | ||
- name: nope | ||
url: ignore-me | ||
path: .west |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# Project paths can't be .west after normalization. | ||
|
||
manifest: | ||
projects: | ||
- name: nope | ||
url: ignore-me | ||
path: foo/../.west |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# Project paths can't be in .west normalized. | ||
|
||
manifest: | ||
projects: | ||
- name: nope | ||
url: ignore-me | ||
path: foo/../.west/some/path |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# Project paths can't be in .west/. | ||
|
||
manifest: | ||
projects: | ||
- name: nope | ||
url: ignore-me | ||
path: .west/some/path |
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.
not wrong, but looks a bit strange to import both
WEST_DIR
andwest_dir
.Same name, just different casing.
Easy to confuse readers.
Why not just use
west_dir()
, when we anyway import it, instead of doing path concatenation.(see other comment).