west init gives wrong config path for a manifest file in nested directories

[fouge]

when initializing my workspace, i want the manifest file to be located two level below the west top dir, in orb/private

.
├── .west
├── bootloader
│   └── mcuboot
├── modules
│   ├── ...
├── orb
│   ├── private
│   └── public
└── zephyr

When initializing with cd orb && west init -l --mf private/west.yml ., the west config created is:

[manifest]
path = orb
file = private/west.yml

but then zephyr_module.py from Zephyr 4.0 fails because the manifest isn't in orb but orb/private.

If I modify .west/config manually, to have path = orb/private, then it works.

Would it be possible to have the path points to the manifest directory, and file only take the name of the west file?

[pdgendt]

You can make the manifest project path explicit.

manifest:
  self:
    path: orb/private

EDIT: Ah the project is orb, nvm

[fouge]

here is the manifest

manifest:
  remotes:
    - name: worldcoin
      url-base: git@github.com:worldcoin
  defaults:
    remote: worldcoin
  group-filter: [+internal]
  projects:
    - name: orb-firmware
      revision: ...
      import:
        name-allowlist:
          - zephyr
          - ...
      path: orb/public
  self:
    path: orb/private

[pdgendt]

So there are 2 projects? orb/private and orb/public?

In that case your init command is wrong and should be:

$ mkdir orb/private
$ west init -l orb/private --mf west.yml

[fouge]

the problem is that is creates the .west directory inside orb/

I want it to be at the top directory above

root@73b74a36d5cc:/home/ubuntu/firmware# west init -l orb/private --mf west.yml
=== Initializing from existing manifest repository private
--- Creating /home/ubuntu/firmware/orb/.west and local configuration file

[fouge]

the .west directory seems to be hardcoded to be manifest_dir.parent with manifest_dir being the directory passed with the -l argument

https://github.com/zephyrproject-rtos/west/blame/361004d55f5b8300aa828456a046e1e54200c648/src/west/app/project.py#L275

[fouge]

something like that work best

diff --git a/src/west/app/project.py b/src/west/app/project.py
index d5f4c70..a87c2b6 100644
--- a/src/west/app/project.py
+++ b/src/west/app/project.py
@@ -286,8 +286,10 @@ below.
         self.create(west_dir)
         os.chdir(topdir)
         self.config = Configuration(topdir=topdir)
-        self.config.set('manifest.path', os.fspath(rel_manifest))
-        self.config.set('manifest.file', manifest_filename)
+        # relative path from west top dir to manifest file
+        path = os.fspath(manifest_file.parent.relative_to(manifest_dir.parent))
+        self.config.set('manifest.path', path)
+        self.config.set('manifest.file', manifest_file.name)
 
         return topdir

[pdgendt]

Can you open a PR?

[fouge]

yes

I expect more discussion and refactoring... because it's not clear how to use directory passed with -l IMO

#775

[marc-hb]

If I modify .west/config manually, to have path = orb/private, then it works.

To be clear, when you did that you ALSO changed file = west.yml to make it work, right? I mean, you did NOT have private twice in west/.config, once on each line?

Just got an idea from rsync that could provide full flexibility and full backwards compatibility...

the problem is that is creates the .west directory inside orb/

The issue is that you're trying to make -l dir1/dir2/dir3/ parameter have two different and conflicting meanings:

1. As of now, -l dir1/dir2/dir3/ points to where the manifest git clone is currently located on the file system. In other words: dir3/.git/.
2. You would also like -l dir1/dir2/dir3/ to define manifest.path = dir1/dir2/dir3/, that is the relative location of the manifest repo inside the west project. This indirectly defines where .west/ will be created.

(Once you add -f path/to/west.yml parameter, that's three pieces of informations total in only two parameters. I digress)

But we can't have a single parameter be two different things at the same time. As you've discovered, the west init -l parameter does NOT support configuring manifest.path right now. As of now, manifest.path is currently "hardcoded" by west init -l to the LAST component: dir3. No "nesting".

What your PR #775 seems to propose is to hardcode manifest.path to the WHOLE -l parameter. But:

1. This would obviously break every script that currently relies on the previous behavior.
2. This is just trading a different manifest.path hardcoding decision against another.

EDIT: sorry, 2. was wrong. This would be more flexible than now. But it would less flexible than below and 1. stands.

---

So this problem reminded me of rsync --relative which has an very similar problem:

https://download.samba.org/pub/rsync/rsync.1#opt--relative

It is also possible to limit the amount of path information that is sent as implied directories for each path you specify. With a modern rsync on the sending side (beginning with 2.6.7), you can insert a dot and a slash into the source path, like this:

rsync -avR /foo/./bar/baz.c remote:/tmp/
That would create /tmp/bar/baz.c on the remote machine

See how the /./ in the middle allows two different values in a single parameter? So based on this, my suggested solution is:

west init -l local/path/to/workspace/./relative/manifest/path/ -f mandir/west.yml

This would create local/path/to/workspace/.west and this:

[manifest]
path = relative/manifest/path
file = mandir/west.yml

When there is no /./, the behavior is unchanged and manifest.path is still "hardcoded" to the last directory. So west init -l dir1/dir2/dir3/ would be equivalent to west init -l dir1/dir2/./dir3/

[marc-hb]

If I modify .west/config manually, to have path = orb/private, then it works.

To be clear, when you did that you ALSO changed file = west.yml to make it work, right? I mean, you did NOT have private twice in west/.config, once on each line?

Forgot to ask: after this manual west.yml edit, you used that workspace "in anger" and never experienced any issue? So this is purely a west init issue, correct?

[fouge]

If I modify .west/config manually, to have path = orb/private, then it works.

To be clear, when you did that you ALSO changed file = west.yml to make it work, right? I mean, you did NOT have private twice in west/.config, once on each line?

yes

[manifest]
path = orb/private
file = west.yml

regarding #775

1. This would obviously break every script that currently relies on the previous behavior.

I don't have the full context and you do, but the previous behavior should still work the same 🤔
do you have examples where it doesn't work anymore?

one case that I didn't take into account is if the west.yml file is inside a subdirectory of the repo (is it handled by west in any case? 🤔 )

Ultimately, the solution you propose with the /./ should do the trick, so if you think it gives a viable long-term solution, let's do it 😃 .

Forgot to ask: after this manual west.yml edit, you used that workspace "in anger" and never experienced any issue? So this is purely a west init issue, correct?

When modifying .west/config, zephyr_module.py works as expected. It wasn't an issue until recently because I am updating Zephyr to 4.0.0 and zephyr_module.py now uses the .west/config to fetch data from the manifest.

I have to hack the west config in CI so that zephyr_module.py doesn't fail on a fresh workspace.

[marc-hb]

I don't have the full context and you do, but the previous behavior should still work the same 🤔
do you have examples where it doesn't work anymore?

I think I misunderstood #775. That's partly because the commit message is mostly a bug report with very little about the new behavior. It should have at least an example of the new behavior.

So now I understand you're "hijacking" the -mf option to specify the manifest.path, sorry I missed that earlier. While I'm suggesting a new syntax to pass 2 values in the single option -l, #775 proposes to pass 2 values using -mf instead. As @pdgendt already found in the #775 review, this would break all manifest repositories that nest their manifest file. I don't think the behavior -mf should change. Right now its value goes unmodified into manifest.file; that's simple and easy to understand.

Instead of trying to shoehorn two values in some single --option, we could also add some new option. Something like:

west init -t all_projs/west_top_dir/  -l all_projs/west_top_dir/dir1/manifestrepo/  -f nested/west.yml

[manifest]
path = dir1/manifestrepo
file = nested/west.yml

Ultimately, the solution you propose with the /./ should do the trick, so if you think it gives a viable long-term solution, let's do it 😃 .

Thanks! Are you volunteering? Don't forget we need more test coverage :-)

after this manual west.yml edit, you used that workspace "in anger" and never experienced any issue?

When modifying .west/config, zephyr_module.py works as expected.

Thanks! And everything else too? Including all the usual west commands?

I want to be really sure this is a problem with the west init command only.