-
Notifications
You must be signed in to change notification settings - Fork 150
manifest: restore "path: ." ability but with a warning #911
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
base: main
Are you sure you want to change the base?
Conversation
"path" can have whitespace or can look like regular words: quote them in error messages. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
b3ebba7 to
d8a8841
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #911 +/- ##
==========================================
- Coverage 85.93% 85.92% -0.01%
==========================================
Files 11 11
Lines 3455 3461 +6
==========================================
+ Hits 2969 2974 +5
- Misses 486 487 +1
|
|
Tested this and for the most part it works well, thanks! Only found this when running it on I suppose you wouldn't want the warning to print twice. |
| # Unsupported, see https://github.com/zephyrproject-rtos/zephyr/commit/7d40091fbfedfc | ||
| if _path == Path('.'): | ||
| _logger.warning("Making the topdir a git clone is not supported; do not report bugs.") | ||
| return ret |
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.
We don't allow the manifest project to be in the topdir directory, I would argue that we shouldn't allow "regular" projects to do it either.
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.
We don't allow the manifest project to be in the topdir directory,
Is that actually blocked or just discouraged?
As far as I understand,
2b2d3f2 says "it is discouraged for the topdir to be a git repo". Anything else on that topic?
Note the manifest does not have to be in git at all: that is documented and works fine.
marc-hb
left a comment
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.
I suppose you wouldn't want the warning to print twice.
Ideally not, but it does not really matter because the workspace is not supposed to be a git repo.
It may not be simple to print this only once. If you find a very simple way with barely additional code then please share.
| # Unsupported, see https://github.com/zephyrproject-rtos/zephyr/commit/7d40091fbfedfc | ||
| if _path == Path('.'): | ||
| _logger.warning("Making the topdir a git clone is not supported; do not report bugs.") | ||
| return ret |
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.
We don't allow the manifest project to be in the topdir directory,
Is that actually blocked or just discouraged?
As far as I understand,
2b2d3f2 says "it is discouraged for the topdir to be a git repo". Anything else on that topic?
Note the manifest does not have to be in git at all: that is documented and works fine.
Restore the ability to have a project at the top level but with an "unsupported" warning printed every time. Fixes v1.3 commit 2b2d3f2 ("manifest: Do not allow projects inside the west directory") which accidentally broke it with an pretty obscure backtrace: File "python/site-packages/west/manifest.py", line 2460, in _load_project if Path(ret_norm).parts[0] == util.WEST_DIR: ~~~~~~~~~~~~~~~~~~~~^^^ IndexError: tuple index out of range Fixes issue number zephyrproject-rtos#910 This is STILL not supported! Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Test coverage for the previous commit that fixes zephyrproject-rtos#910. Will catch any regression like what happened in v1.3 commit 2b2d3f2.
d8a8841 to
a07cd25
Compare
3 commits. Main commit:
manifest: restore "path: ." ability but with a warning
Restore the ability to have a project at the top level but with an
"unsupported" warning printed every time.
Fixes v1.3 commit 2b2d3f2 ("manifest: Do not allow projects inside
the west directory") which accidentally broke it with an pretty obscure
backtrace:
Fixes issue number #910
This is STILL not supported!