`west manifest --freeze` fails since it doesn't ignore excluded groups

[JelmerT]

When a group is disabled in a manifest, those project(s) don't get cloned, so no folder(s) is created.

When doing west manifest --freeze the command fails with

RuntimeError: cannot freeze; project XXXX is uncloned

In #314 , where a repo has no commits, this would be expected behavior. But in this case where projects are specifically excluded, would this still be expected behavior?

If yes -> more robust handling would be nice instead of a stack trace.

[marc-hb]

But in this case where projects are specifically excluded, would this still be expected behavior?

I'm guessing not.

Speaking of stack traces: they're very useful... in bug reports :-)

[JelmerT]

Now, by popular demand! The stack trace:

$ west manifest --freeze
Traceback (most recent call last):
  File "/usr/local/bin/west", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.9/site-packages/west/app/main.py", line 779, in main
    app.run(argv or sys.argv[1:])
  File "/usr/local/lib/python3.9/site-packages/west/app/main.py", line 106, in run
    self.run_command(argv)
  File "/usr/local/lib/python3.9/site-packages/west/app/main.py", line 336, in run_command
    cmd.run(args, unknown, self.topdir, manifest=self.manifest)
  File "/usr/local/lib/python3.9/site-packages/west/commands.py", line 129, in run
    self.do_run(args, unknown)
  File "/usr/local/lib/python3.9/site-packages/west/app/project.py", line 621, in do_run
    self._dump(args, manifest.as_frozen_yaml(**dump_kwargs))
  File "/usr/local/lib/python3.9/site-packages/west/manifest.py", line 1498, in as_frozen_yaml
    return yaml.safe_dump(self.as_frozen_dict(), **kwargs)
  File "/usr/local/lib/python3.9/site-packages/west/manifest.py", line 1475, in as_frozen_dict
    return self._as_dict_helper(pdict=pdict)
  File "/usr/local/lib/python3.9/site-packages/west/manifest.py", line 1430, in _as_dict_helper
    project_dicts = [pdict(p) for p in projects]
  File "/usr/local/lib/python3.9/site-packages/west/manifest.py", line 1430, in <listcomp>
    project_dicts = [pdict(p) for p in projects]
  File "/usr/local/lib/python3.9/site-packages/west/manifest.py", line 1463, in pdict
    raise RuntimeError(f'cannot freeze; project {p.name} '
RuntimeError: cannot freeze; project homekit is uncloned

Apologies for not including it, i thought it didn't give that much extra info to the issue in this case.

[marc-hb]

Apologies for not including it, i thought it didn't give that much extra info to the issue in this case.

Indeed it does not seem to add much this time. As long as it does not add several pages of scrolling I find it simpler and faster to always inline all error messages and error traces. At best it helps, at worst it does not hurt and clears any doubt.

[mbolivar-nordic]

But in this case where projects are specifically excluded, would this still be expected behavior?

I unfortunately think the answer may be "yes, this is still expected behavior", but I'm not totally sure what the right thing to do here is. This is an interesting situation; thanks for the detailed report.

Consider this case:

manifest:
  group-filter: [-disabled]
  projects:
    - name: foo
      groups: [disabled]
      revision: main

On the one hand, since the revision is main, west really does need a clone to produce a frozen manifest, as defined in the documentation:

A “frozen” manifest is a manifest file where every project’s revision is a SHA. You can use --freeze to produce a frozen manifest that’s equivalent to your current manifest file.

https://docs.zephyrproject.org/latest/guides/west/manifest.html#freezing-manifests

I think that freezing the above manifest and just ignoring foo produces something which is clearly not equivalent to the original manifest.

However, I think this is sufficiently likely to confuse people that perhaps we should either ignore and warn, or add a new --freeze-ignore-inactive, or something like that.

No matter what I agree the stack trace needs to be fixed.

Any opinions?

[mbolivar-nordic]

west really does need a clone

... well, either that, or we could try pinging the remote over the network to ask for a SHA, without cloning. Not sure how easy that would be.

[JelmerT]

pinging the remote over the network to ask for a SHA, without cloning.

In my particular case the project that is disabled is disabled because it's only accessible to certain devs with the right credentials. So this would not be a great option.

I vote for ignore and warn.

[marc-hb]

One big problem I see: projects can be inactive for many reasons. Either directly in the manifest where they are listed as in @mbolivar 's example, or because of imports, or from the command line, or from some config file. Group filters made the freezing process (and others?) quite elusive IMHO.

When looking at just @mbolivar-nordic's example you might naively think "easy, just omit that project in the frozen manifest" - except this group could be overriden and enabled by some local config file and that project actually be active. And let's not even get into multi-group projects. There are too many possibilities, it's too complicated.

So here's a simple suggestion: partial freezing.

When a particular project is missing and cannot be frozen, do not block the generation of the manifest but insert the error message inside the generated manifest itself:

manifest:
  group-filter: [-disabled]
  projects:
    - name: foo
      groups: [disabled]
      revision: could_not_freeze_main_branch_cause_project_was_not_present_when_jelmerT_froze_it

+ generate a non-fatal warning.

OK, maybe some of the information should be moved to a comment instead of the revision field but you get the idea.

This postpones the error(s) to the users of the frozen manifest.

If this project is still disabled by someone who uses the frozen manifest, then everything should still work. Profit!

If this project is enabled by someone tries and fails to use the frozen manifest, well too bad: they should get some obvious error message (untested) and go and complain to the person who generated that incomplete manifest.

[mbolivar-nordic]

The more I think about this, the more I don't like the idea of just warning. I feel like the --freeze behavior is pretty well specified: the results have to be equivalent to the input manifest, and all revisions must be SHAs. So could_not_freeze_main_branch_cause_project_was_not_present_when_jelmerT_froze_it is plain invalid output, and ignore-and-warn is non-equivalent.

Would a new west manifest --soft-serve-frozen-manifest (actual name TBD) that ignores-and-warns be acceptable?

[mbolivar-nordic]

For some additional historical context, the initial use case which motivated west manifest --freeze was for manifests with "rolling" projects that tracked a branch instead of a SHA. You could create a "release" manifest with SHAs instead after you were done with all your testing using this command. My previous employer did just that.

I think that in keeping with this motivation, west manifest --freeze is doing the right thing by failing, it just needs to print a better error message.

[marc-hb]

I understand the desire not to degrade the existing "purity" of west manifest --freeze.

However that makes it unusable by second class citizens who don't have full access or simply don't want everything. A new and "best effort" west manifest --melting-point feature sounds like a good idea to me. @JelmerT ?