app: more versatile initialization for path in config #775
Conversation
fouge
force-pushed
the
fouge/west-manifest-path
branch
2 times, most recently
from
bebc716 to
dbb2fdc
Compare
|
IMO to go even further, I think
but
the command would become this is a breaking change though, thoughts? |
fouge
force-pushed
the
fouge/west-manifest-path
branch
from
dbb2fdc to
8631ad2
Compare
|
Verify that |
in case a manifest file is contained into some sub directories, the command to init a workspace locally should be ``` west init -l sub1/sub2 --mf west.yml ``` but in that case, the west top dir is going to be into `sub1` which is an issue. one workaround is to use: ``` west init -l sub1 --mf sub2/west.yml ``` but in that case, the config is initialized incorrectly: `manifest.path` contains only `sub1` and `manifest.file` is `sub2/west.yml`. `manifest.path` should contain a path, even if `--mf` is a relative path. `manifest.file` should only contain the filename. this change allows any usage so that the workspace top dir is created next to `sub1` and the manifest repo can be located into nested directories Signed-off-by: Cyril Fougeray <[email protected]>
fouge
force-pushed
the
fouge/west-manifest-path
branch
from
8631ad2 to
92629a8
Compare
|
@pdgendt tox fails with errors even on |
| self.config = Configuration(topdir=topdir) | ||
| self.config.set('manifest.path', os.fspath(rel_manifest)) | ||
| self.config.set('manifest.file', manifest_filename) | ||
| self.config.set('manifest.file', manifest_file.name) |
There was a problem hiding this comment.
This doesn't allow a nested manifest file, I think? It should be relative to the manifest path.
There was a problem hiding this comment.
Thanks for catching this issue and submitting a PR. I haven't time to look at it in detail yet but I already know we definitely need additional test coverage for this. Either adding some new test(s) or tweaking existing tests or a combination of both.
|
Again, thanks for the excellent report.
Interestingly, even your very first I'm afraid we need to clearly and formally define the user interface before even looking at the code. EDIT: see rsync-based suggestion in #774 (comment)
Please file a separate bug for this. Don't forget the OS and OS version. |
|
@fouge please test and review newer |
3 participants
in case a manifest file is contained into some sub directories, the command to init a workspace locally should be
but in that case, the west top dir is going to be into
sub1which is an issue.one workaround is to use:
but in that case, the config is initialized incorrectly:
manifest.pathcontains onlysub1andmanifest.fileissub2/west.yml.manifest.pathshould contain a path, even if--mfis a relative path.manifest.fileshould only contain the filename.this change allows any usage so that the workspace top dir is created next to
sub1and the manifest repo can be located into nested directoriesfixes #774