Commit Graph

854 Commits

Author SHA1 Message Date
Kari Hamalainen cf18ce8cbd project.py: extend cache support for submodules
If cache is used try to also use it for submodules.

Signed-off-by: Kari Hamalainen <kari.hamalainen@nordicsemi.no>
2024-04-03 10:33:18 -07:00
Marc Herbert aeca29ac9f project.py: request users to stop IDEs before running `west init`
There have been many users reports in #558 that `west init` fails on the
Windows NTFS filesystem when some IDEs are running concurrently.

There is no simple and reliable fix for this because:

- The lack of concurrency is a core limitation in the NTFS design that
  will not change anytime soon,
- IDEs have no reliable way to know when they should pause scanning the
  filesystem.
- Placing temporary files outside the workspace would invite other, complex
  and nasty issues with cross-filesystem moves (#558)

Short of a fix, explain why this happens very briefly in `west init -h`
and request users to temporarily close their IDEs. It's the only simple
and universal mitigation available.

Mention the issue number in case someone needs more details, some
evidence or smarter workarounds specific to certain IDEs.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2024-01-24 09:24:45 -08:00
Marc Herbert 3260b412a9 Use 'backslashreplace' not to crash on malformed UTF from subprocess
Giant commit
https://github.com/zephyrproject-rtos/hal_nxp/commit/f9f0944bc2b4fce
"Update to SDK 2.14" added files with malformed UTF-8, more precisely
with the DEGREE SIGN (°) encoded in 8bit latin1/CP1252. Maybe others.

This crashes `west grep`.

Before this fix:

```
nxp$ west grep 'TEMPERATURE_CONV_FACTOR.*Will give'

Traceback (most recent call last):
  File ".local/bin/west", line 33, in <module>
    sys.exit(load_entry_point('west', 'console_scripts', 'west')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "west/src/west/app/main.py", line 1085, in main
    app.run(argv or sys.argv[1:])
  File "west/src/west/app/main.py", line 244, in run
    self.run_command(argv, early_args)
  File "west/src/west/app/main.py", line 503, in run_command
    self.run_builtin(args, unknown)
  File "west/src/west/app/main.py", line 611, in run_builtin
    self.cmd.run(args, unknown, self.topdir,
  File "west/src/west/commands.py", line 194, in run
    self.do_run(args, unknown)
  File "west/src/west/app/project.py", line 1765, in do_run
    completed_process = self.run_subprocess(
                        ^^^^^^^^^^^^^^^^^^^^
  File "west/src/west/commands.py", line 325, in run_subprocess
    return subprocess.run(args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 550, in run
    stdout, stderr = process.communicate(input, timeout=timeout)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 1209, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 2146, in _communicate
    stdout = self._translate_newlines(stdout,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 1086, in _translate_newlines
    data = data.decode(encoding, errors)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xb0 in position 385: invalid start byte
```

After this fix, no crash and no interruption and:

mcux/mcux-sdk/middleware/issdk/sensors/fxpq3115_drv.h:#define FXPQ3115_TEMPERATURE_CONV_FACTOR (256) /* Will give \xb0C */
mcux/mcux-sdk/middleware/issdk/sensors/mpl3115_drv.h:#define MPL3115_TEMPERATURE_CONV_FACTOR (256) /* Will give \xb0C */

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-12-20 10:43:13 -08:00
Martí Bolívar 7f842c2b84 commands: fix some docstrings
When interpreted as restructured text, these are causing errors
because the '**' is interpreted as the start of a bold section, which
never ends. This causes build failures in the zephyr docs, which do
treat these as RST.

Wrap them in `` to make the whole thing a literal block to fix this
issue.

Signed-off-by: Martí Bolívar <mbolivar@amperecomputing.com>
2023-10-09 11:11:24 -07:00
Martí Bolívar 7d108ff1ce version: 1.2.99
This is not a west release. It is just a signal that we have forked
off v1.2-branch.

Signed-off-by: Martí Bolívar <mbolivar@amperecomputing.com>
2023-09-27 14:01:11 -07:00
Carles Cufi c936a4a681 manifest: Add a new optional description field
Add a new description optional field to the schema. This field is merely
informative, it has no effect whatsoever in the manifest and/or project
processing.

Because descriptions can be multiline strings, additional code has been
added to support prettier dumping of those. PyYAML by default uses
single-line strings in YAML with '\n' characters in it, which look ugly
when dumped. To avoid this, use block-style literals (i.e. the ones
beginning with '|' in the YAML) but only for multi-line description
fields. The rest of the field handling are left untouched to preserve
backwards compatibility.
Note that if we used ruamel we would not need the additional code, since
it supports this natively.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
2023-09-05 08:59:50 -07:00
Carles Cufi 9ddcf709ba schema: Update instructions when modifying the schema
The instructions were out of date, since the version test now uses the
_VALID_SCHEMA_VERS array from manifest.py.

Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
2023-09-05 08:59:50 -07:00
Marc Herbert 41007648d9 Project.git(list/str): reduce reliance on shlex.split()
For convenience, Project.git() supports passing either a list (good) or
a string with whitespaces (bad). The latter is parsed with shlex.split()

This saves some typing but the caller has to be extremely careful to
never use the shlex.split() convenience with unsanitized inputs.

Fixes commit 3ac600acaa ("git: clean west ref space after fetching")
where the caller was not careful and concatenated `update-ref -d ` with
unsanitized input, possibly containing special characters as found in
bug #679. Fix this bug by converting the string to a list.

While at it, look for a few other, frequent and risky invocations and
convert their string argument to a list too. The following test hack was
used to semi-automate the search for these other locations:

```
--- a/src/west/manifest.py
+++ b/src/west/manifest.py
@@ -897,6 +897,8 @@ class Project:
         :param cwd: directory to run git in (default: ``self.abspath``)
         '''
         if isinstance(cmd, str):
+            print(cmd)
+            breakpoint()
             cmd_list = shlex.split(cmd)
         else:
             cmd_list = list(cmd)
```

While at it, also convert to a list a couple non-risky but very frequent
invocations. This speeds up the test hack above.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-09-01 13:58:57 -07:00
Marc Herbert bcd58e0e38 tox.ini: set flake8 max-line-length to 100
Keep up with the times and new policies.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-09-01 13:58:57 -07:00
Marc Herbert 98800b3d9a compare: always prefix `git status` output with manifest-rev
It's not unusual to make "quick and dirty", temporary changes in a git
repo that is on the manifest: either add some untracked files or create
a branch. As expected, this makes the repository show in `west
compare`. If such repos and the manifest repo are the only repos
appearing in the `west compare` output, then no manifest-rev + HEAD
banner ever appeared. When no banner ever appears it's really not
obvious which repos are on the manifest-rev vs not. In other words: the
main `west compare` feature is defeated.

Clear this doubt by always showing the banner, even when `HEAD` and
`manifest-rev` are the same. See sample output below.

I think the original design idea was to follow diff's "spirit" not to
show anything that's identical and to save as many lines as
possible. However I don't think it works in this particular case because
invoking `git status` for a repo _without_ showing where its HEAD is at
is way too subtle, _especially_ when there is no other repo with a
banner to provide a comparison point and some contrast. Explicitly and
consistently prefixing every `git status` output with a manifest-rev
banner is much more clear and obvious.

Moreover, `git status` output is relatively verbose already so always
prefixing it with a 2 lines long banner makes negligible difference to
the total.

Before:
```
$ west compare

=== hal_xtensa (modules/hal/xtensa):
--- status:
    On branch somebranch
    nothing to commit, working tree clean
```

After:
```
$ west compare

=== hal_xtensa (modules/hal/xtensa):
--- manifest-rev: 41a631d4aeee (somebranch, upstream/master) cmake: enable using SOC_TOOLCHAIN_NAME ...
            HEAD: 41a631d4aeee (somebranch, upstream/master) cmake: enable using SOC_TOOLCHAIN_NAME ...
--- status:
    On branch somebranch
    nothing to commit, working tree clean
```
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-09-01 11:01:23 -07:00
Marc Herbert b7e091d2e9 README.rst: add pytest examples and mention tox limitations
The tox indirection layer is convenient except for the usual problems
caused by too many layers of indirection:

- Add some examples and show how it is still possible to use important
  pytest features. This is also useful to boost people familiar with
  Python but not with pytest.

- To stop developers wasting their time trying, document a major
  limitation with the current approach: impossible to run the west
  code in a debugger when it's started from a test.

- Show how the "printf" alternative can work.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-09-01 10:27:52 -07:00
Marc Herbert 80be0ecb41 tox.ini: move pytest first, before style checkers
This is required to test code that includes temporary test hacks that
don't pass style checks.

People write ugly code first. Then they make it prettier.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-09-01 10:27:52 -07:00
Attie Grande dacb54ba1d west manifest: detect when target directory already exists, and fail
When setting up a project with west, the target directory may not be
initialized correctly. In the typical case, if a directory named
`./zephyr/` already exists, the user may find that checkout files are
located at `./zephyr/manifest-tmp/*` instead of the expected
`./zephyr/*`.

This patch will abort and refuse to complete `west init` if the
destination directory alread exists. This check would ideally occur
before the potentially lengthy clone operation, but `manifest_path` is
derived from the files retrieved...

NOTE: If the project quotes a value other than `zephyr` for
`manifest.self.path` in `/west.yml`, then this will affect that
directory instead.

Steps to reproduce before this patch:

  mkdir ./zephyr/
  west init ./ -m https://github.com/zephyrproject-rtos/zephyr.git
  ls -l ./zephyr/

Possible fix for some of the symptoms described in #558

Signed-off-by: Attie Grande <attie.grande@argentum-systems.co.uk>
2023-08-31 14:43:10 -07:00
Marc Herbert 3e53dd599a grep: fix command hanging forever when grep.{tool}-args is empty
Fixes commit 1d220bbd62 ("commands: add grep")

Fix misplaced closing parenthese in:

```
shlex.split(self.config.get(f'grep.{tool}-args'), '')
```

The empty string is meant as a default value for config.get() but it was
passed as a second argument to shlex.split() by mistake.

Funny enough this hangs forever:

```
>>> shlex.split(None, comments='')
<stdin>:1: DeprecationWarning: Passing None for 's' to shlex.split() is deprecated.
 (hangs forever)
```

PS: debuggers rulez

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-08-31 12:04:42 -07:00
Martí Bolívar 4ba3d99014 tests: add basic test for grep command
Get some minimal coverage.

Signed-off-by: Martí Bolívar <mbolivar@amperecomputing.com>
2023-08-31 09:51:23 -07:00
Martí Bolívar 1d220bbd62 commands: add grep
This has been requested various times. I haven't done it because 'west
forall -c "git grep"' has been a reasonable workaround for me, and I
never had time or need to think about what an ergonomic alternative
might look like.

That's changed now and I wanted something that would only print output
for projects where a result was found, similarly to the way "west
diff" only prints for projects with nonempty "git diff", etc.

Add a "grep" command that does similar, defaulting to use of "git
grep" to get the job done. Also support (my favorite) ripgrep and
plain "grep --recursive". See the GREP_EPILOG variable in the patch
for detailed usage information.

Signed-off-by: Martí Bolívar <mbolivar@amperecomputing.com>
2023-08-31 09:51:23 -07:00
Martí Bolívar a814b26313 WestCommand: extend print helpers with end kwarg
This is useful when we want to be able to control whether a newline is
appended or not. The die() method is intentionally omitted: this is a
"scream and exit the process" method, and always flushing any
line-buffered I/O makes sense to enforce in this context.

Signed-off-by: Martí Bolívar <mbolivar@amperecomputing.com>
2023-08-31 09:51:23 -07:00
Martí Bolívar 4878065298 WestCommand: expand the subprocess interface
The WestCommand API for running subprocesses is OK, but
is missing some useful features; add them now:

- The current recommendation in the subprocess module is
  to use subprocess.run(), so add a helper for doing that
  since it's missing. (The name asymmetry is because
  there is already a WestCommand.run() claimed.)

- Allow subprocess invocations to take any kwargs we want

Signed-off-by: Martí Bolívar <mbolivar@amperecomputing.com>
2023-08-31 09:51:23 -07:00
Martí Bolívar 9e8f5002f2 west manifest: adjust error message
Clarify the user's options a bit more.

Requested-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-02 20:12:06 +02:00
Martí Bolívar 31ac603e03 manifest: change project-filter semantics
Make this option consistent with all others by only having the highest
precedence configuration option take effect. We revisited this
decision following review.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-02 20:12:06 +02:00
Martí Bolívar 50c316a262 manifest: fix comments
The top level comment is not totally accurate. The 'project_importer'
attribute, for example, doesn't change.

The comment around 'project_filter' reflects an idea I had but
abandoned so that project filter behavior would be more consistent
with group filter behavior.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-02 20:12:06 +02:00
Martí Bolívar 74f43a95d7 Revert "tests: fix test_group_filter_imports()"
This reverts commit d0e6b9a54b.

West was right, the docs are wrong/inconsistent.

The commit I'm reverting was following the west documentation it
referenced correctly, but the documentation itself was wrong (and in
fact was inconsistent with earlier documentation on the same page).

The basis for making commit d0e6b9a54 was that the docs currently say:

    In other words, let:

    - the submanifest resolved from self-import have group filter self-filter
    - the top-level manifest file have group filter top-filter
    - the submanifests resolved from import-1 through import-N have
      group filters filter-1 through filter-N respectively

    The final resolved group-filter value is then filter1 + filter-2 +
    ... + filter-N + top-filter + self-filter, where + here refers to
    list concatenation.

But earlier in the same section, they also say:

    The resolved manifest has a group-filter value which is the result
    of concatenating the group-filter values in the top-level manifest
    and any imported manifests.

    Manifest files which appear earlier in the import order have
    higher precedence and are therefore concatenated later into the
    final group-filter.

Where the "import order" is defined higher up as:

    Importing is done in this order:

    1. Manifests from self-import are imported first.
    2. The top-level manifest file’s definitions are handled next.
    3. Manifests from import-1, …, import-N, are imported in that order.

This means that import-1 happens **earlier** than import-N, and should
therefore have **higher** precedence when it comes to computing the
final group filter. But putting import-N **later** in the concanated
group filter actually means import-1 has **lower** precedence. So the
docs are inconsistent.

To fix the docs so they are consistent and match west's current
behavior, we should replace this:

    The final resolved group-filter value is then filter1 + filter-2 + ...
    + filter-N + top-filter + self-filter, where + here refers to list
    concatenation.

with this:

    The final resolved group-filter value is then filter-N + ... +
    filter-1 + top-filter + self-filter, where + here refers to list
    concatenation.

That is:

- docs currently go from: filter-1 to filter-N
- they should go from:    filter-N to filter-1.

Fixes: #663
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-02 20:12:06 +02:00
Martí Bolívar 29689f67dc manifest: improve group filter debugging
Use a deque instead of a list when accumulating group filters during
imports. This allows us to preserve the boundaries of each filter
list, which in turn allows us to improve the debug output.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-02 20:12:06 +02:00
Martí Bolívar e456d50c55 manifest: readability and maintainability tweaks
Clarify some attribute contents and reorganize so related
attributes go together.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-02 20:12:06 +02:00
Martí Bolívar 698e0e85bb manifest: add tests for manifest.project-filter
Add test cases for behavior and matching rules.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 9f551ef273 west manifest: forbid --freeze/--resolve + manifest.project-filter
Internal discussion at Nordic indicates that the semantics for 'west
manifest --resolve' should probably be something along the lines of
"resolve the manifest but ignore the value of manifest.project-filter,
and print the resolved result". This is consistent with the way that
these commands are not affected by the value of manifest.group-filter.

However, we don't have a way to support this right now: west.manifest
has no API for loading itself but with manifest.project-filter
ignored. Such an API would be straightforward to add, but we don't
have time for that right now, as we're under time pressure to add
support for this option to resolve an issue during the zephyr v3.4
stabilization period.

For now, work around the issue by just erroring out if the option is
set at all, telling the user that they can get in touch with us
if they need this. We'll let the level of noise that results be
our guide in prioritizing this enhancement.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 9e41f68791 manifest: support manifest.project-filter option
Add support for this option. The option's value is a comma-separated
list of regular expressions, each prefixed with '+' or '-', like this:

    +re1,-re2,-re3

Project names are matched against each regular expression (re1, re2,
re3, ...) in the list, in order. If the entire project name matches
the regular expression, that element of the list either deactivates or
activates the project. The project is deactivated if the element
begins with '-'. The project is activated if the element begins with
'+'. (Project names cannot contain ',' if this option is used, so the
regular expressions do not need to contain a literal ',' character.)

If a project's name matches multiple regular expressions in the list,
the result from the last regular expression is used. For example,
if manifest.project-filter is:

    -hal_.*,+hal_foo

Then a project named 'hal_bar' is inactive, but a project named
'hal_foo' is active.

If a project is made inactive or active by a list element, the project
is active or not regardless of whether any or all of its groups are
disabled. (This is also now the only way to make a project that has no
groups inactive.)

Otherwise, i.e. if a project does not match any regular expressions in
the list, it is active or inactive according to the usual rules
related to its groups.

Within an element of a manifest.project-filter list, leading and
trailing whitespace are ignored. That means these example values
are equivalent:

    +foo,-bar
    +foo , -bar

Any empty elements are ignored. That means these example values are
equivalent:

    +foo,,-bar
    +foo,-bar

The lists in the manifest.project-filter options in the system,
global, and local configuration files are concatenated together.
For example, on Linux, with:

    /etc/westconfig:
    [manifest]
    project-filter = +foo

    ~/.westconfig:
    [manifest]
    project-filter = -bar_.*

    <workspace>/.west/config:
    [manifest]
    project-filter = +bar_local

Then the overall project filter when within the workspace <workspace>
is:

    +foo,-bar_.*,+bar_local

This lets you set defaults in the system or global files that can be
overridden within an individual workspace as needed, without having to
maintain the defaults across multiple workspaces.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 4db155a9a9 manifest: load and validate manifest.project-filter option
This adds initial support for loading a 'manifest.project-filter'
configuration option.

This option is special in that its values in the system, global,
and local configuration files are all considered at the same time,
rather than just whichever one is defined at highest precedence.

This is because the eventual purpose of this configuration option
is to allow people to deactivate or activate projects using the
configuration file system, and it seems nicer to allow people to
progressively refine a filter instead of having to synchronize
global settings that they always want applied into each workspace
they have or may create.

This patch doesn't actually do anything with the option values besides
defining the internal representation, validating the options on the
file system, and saving the results in the import context state that
we carry around while importing projects. Applying the filter itself
will come in future patches. See source code comments for details on
behavior (these will eventually make their way into the
documentation).

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar a5814f5972 manifest: initial manifest.project-filter project name validation
In order to support the manifest.project-filter configuration option,
we need to forbid certain characters from occurring in project names.

Making that happen is a backwards incompatible change, however, so
phase it in by emitting a warning.

We will make it a hard error if manifest.project-filter is set later
on. That way, users will only see an error if they are opting into the
latest west for now, and other users will get a warning and have time
to migrate their projects.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar cd42505aec manifest: check project names as we go
This will still fail or succeed in the same situations,
but it is convenient to do it this way as prep work for
future enhancements.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar fcbb0a209a app: tweak logging on malformed manifest or configuration
Put the first argument on the same line as the string "error:", to
make it easier to grep.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 23db6e1f3c app: re-work logging yet again
Commit 92c18ac55a
("main: move verbose manifest logging to project.py")
did not achieve its purpose.

We are loading the manifest long before we call the setup_logging()
methods in each of the project.py classes, so any messages from the
manifest class have long been discarded by the time that we enable
them.

Now that we have EarlyArgs in main.py, we can use that to centralize
logging and enforce the following:

- warnings and above for west APIs are emitted by default
- info and above emitted with west -v
- debug and above with west -vv or higher

Fixes: #665
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar d842371089 app: handle unexpected command name better
Right now if you run a zephyr extension like 'west build' outside of a
workspace, argparse says:

  [...] invalid choice: ‘build’ [...]

This is because argparse's subcommand parser doesn't seem to have any
API to add a catch-all value for when the user provides an unknown
command, so it expects that exactly the subcommands we told it about
are available.

This is confusing to users, and now that we have our own EarlyArgs
parsed, we can do better by printing some west-specific help if we
aren't in a workspace:

  usage: west [-h] [-z ZEPHYR_BASE] [-v] [-V] <command> ...
  west: unknown command "build"; do you need to run this inside a
  workspace?

as well as if you are:

  usage: west [-h] [-z ZEPHYR_BASE] [-v] [-V] <command> ...
  west: unknown command "foo"; workspace /home/mbolivar/zp does
  not define this extension command -- try "west help"

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar f9630e37be app: add hand-rolled partial argument parser
We've been avoiding this for a long time, but we can't any longer
and it's time to bite the bullet.

There are some chicken-and-egg problems in our argument parsing that
can only be handled if we do some manual command line argument parsing
to determine what the common options (like --verbose) are set to,
and to determine what the command name (if any) is.

For example, we would ideally like to know the verbosity level of the
west command *before* we set up logging for the west.manifest module,
so that we can set the log level for that module appropriately.

Right now, we can't do that, because we load the manifest to figure
out what the extension commands are, and from there delegate to
argparse to count the '--verbose' options. By the time argparse runs,
then, it's too late: we already loaded the manifest, and any log
messages from west.manifest are lost.

Enable solving this and other problems by writing our own, very
limited, command line parser that just figures out the global
options (west -hvVz) and the command name. We won't need all of this
right away, but we might as well try to be complete from the start.

This is prep work only; we'll put it to work in later patches.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 951fa7e315 app: move some code around
Moving all the "work" of main() into WestApp.run() will
help us in future patches where we need to set up logging
in a way that requires internal state from that object.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 4736156a0d tests: manifest: improve 'import: <map>' coverage
The error handling in the _load_imap() function is not being tested.
This was discovered by manually inspecting the HTML coverage data.

Add statement coverage for the error handling.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 16161c62d7 manifest: remove dead stores
We're not using the 'i' variable.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar dedaa51825 manifest: handle missing top level manifest file
Improve error handling to avoid dumping stack.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar b9aebfec64 tests: add Manifest.from_topdir() helper
This is a more convenient way to construct manifests, so we might as
well have it around and use it to save some typing.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar d0e6b9a54b tests: fix test_group_filter_imports()
Fixing the test case logic causes it to fail, so mark it xfail.
This is bug #663.

The test case test_group_filter_self_import() is incorrect, which
conveniently enough provides steps to reproduce.

The test case should read as follows (patch applies to f6f5cf6):

diff --git a/tests/test_manifest.py b/tests/test_manifest.py
index 14f2941..e934b71 100644
--- a/tests/test_manifest.py
+++ b/tests/test_manifest.py
@@ -2828,7 +2828,7 @@ def test_group_filter_imports(manifest_repo):
     sha2 = setup_project('project2', '[+gy,+gy,-gz]')

     v0_9_expected = ['+ga', '-gc']
-    v0_10_expected = ['-ga', '-gb', '-gc', '-gw', '-gy', '-gz']
+    v0_10_expected = ['-ga', '-gb', '-gc', '-gw', '-gz']

     #
     # Basic tests of the above setup.

In other words, west incorrectly concludes that group 'gy' is disabled
in this scenario, when it should be enabled.

The test creates the following layout, where ./mp/west.yml is the main
manifest:

    ───────────────────────────────────────
     File: ./mp/west.yml
    ───────────────────────────────────────
     manifest:
       version: "0.10"

       group-filter: [+ga,-gc]

       projects:
         - name: project1
           revision: ...
           import: true
         - name: project2
           revision: ...
           import: true

       self:
         import: self-import.yml
    ..
    ───────────────────────────────────────

    ───────────────────────────────────────
     File: ./project1/west.yml
    ───────────────────────────────────────
     manifest:
       group-filter: [-gw,-gw,+gx,-gy]
    ───────────────────────────────────────

    ───────────────────────────────────────
     File: ./project2/west.yml
    ───────────────────────────────────────
     manifest:
       group-filter: [+gy,+gy,-gz]
    ───────────────────────────────────────

    ───────────────────────────────────────
     File: ./mp/self-import.yml
    ───────────────────────────────────────
     manifest:
       group-filter: [-ga,-gb]
    ───────────────────────────────────────

The west docs say:

  In other words, let:

  - the submanifest resolved from self-import have group filter self-filter
  - the top-level manifest file have group filter top-filter
  - the submanifests resolved from import-1 through import-N have
    group filters filter-1 through filter-N respectively

  The final resolved group-filter value is then filter1 + filter-2 +
  ... + filter-N + top-filter + self-filter, where + here refers to
  list concatenation.

  - https://docs.zephyrproject.org/latest/develop/west/manifest.html

Applying these rules, the final filter should be concatenated from
./project1/west.yml, ./project2/west.yml, ./mp/west.yml,
./mp/self-import.yml, in that order. Since neither ./mp/west.yml nor
./mp/self-import.yml have a group filter which affects gy, the result
should be that it is enabled, since ./project2/west.yml enables it
explicitly.

Fix the test so it matches the expected behavior. We'll fix the
implementation in a separate commit.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar 1970942895 commands: manifest: fix error path
The filename attribute no longer exists. Instead, we have a better
str() result for a ManifestImportFailed. Use it instead.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Martí Bolívar cab2b833c1 manifest: clarify an internal attribute's initialization
There's a field whose initialization procedure differs enough
from the comment describing it that it's worth clarifying.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-06-01 12:37:45 +02:00
Marc Herbert f6f5cf686f compare: add --decorate
When a project is not on the `manifest-rev` it will often be on a branch
or tag. Use the `%d` format to make `west compare` show that
information.

As discussed in #643 and before, git status cannot be fully trusted to
display branch information.

Sample new output:
```
west compare
=== zephyr (zephyr):
--- manifest-rev: e40859f78712 (some_tag) Revert "dma: dw: Do ...
            HEAD: e803b77463b4 (zephyrproject/main) samples: net: ...
--- status:
    HEAD detached at zephyrproject/main
    nothing to commit, working tree clean
```

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-05-15 09:03:21 -07:00
Marc Herbert 421d1228b9 test_compare: add config_tmpdir fixture to block `ignore-branches`
Isolate test_compare() from the environment's west config. This makes
the test pass again when the user sets `west config --global
ignore-branches True`. It was failing like this:

```
  # By default, a checked-out branch should print output, even if
  # the tree is otherwise clean...
  check_output(['git', 'checkout', '-b', 'mybranch'], cwd=kconfiglib)
  actual = cmd('compare')
>    assert actual.startswith('=== Kconfiglib (subdir/Kconfiglib):')
E    AssertionError: assert False
```

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-05-10 10:11:46 -07:00
Martí Bolívar 28809c613d tests: move config_tmpdir fixture to conftest
It will be convenient to manipulate configuration files in a safe
way from outside of test_config.py. To do this, make the config_tmpdir
global to all test modules by migrating it to conftest.py.

We still want to keep that fixture as an autouse in test_config.py,
so add a shim to keep that as-is.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-05-10 10:11:46 -07:00
Marc Herbert f44debcee2 configuration.py: clarify Configuration constructor's behavior
Also rename internal parameter `search_for_local` to
`find_local`. `search_for_local` is easier to misinterpret as optional
(not finding a local file is fatal when True).

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2023-05-09 21:41:24 -07:00
Martí Bolívar de8590494b manifest: improve ManifestImportError __str__
Try to provide more helpful text.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-05-05 14:10:14 -07:00
Martí Bolívar b6b130bac5 main: handle ManifestImportError from extensions better
The default west manifest file in zephyrproject-rtos/zephyr
recently added a project with an 'import'.

That means that 'west build' will fail with the confusing
'invalid choice: "build"' error if you pull zephyr without
running 'west update'. This is a worse experience than what happens
when you run e.g. 'west list': running a built-in command that needs
the manifest to do its work will happily tell you you need to run
'west update' to resolve the failed import.

This is confusing people who pull and run 'west build', and we do have
enough information to do better from WestArgumentParser.error(), which
is the official argparse callback API invoked when
parse_known_arguments() fails in this situation.

Extract the error message formatting for this situation into a helper
function and use it from error() to print the same message when we
get ManifestImportError in these situations.

Make the error message take up fewer lines while we are here.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-05-05 14:10:14 -07:00
Martí Bolívar 51ad8866fb compare: output improvements
A few concerns have been voiced about the west compare output:

   1. its signal:noise ratio is too low (too many lines per
      project without enough useful information per line)
   2. the end of the status output from one project is hard to
      separate from the status output for the next
   3. it isn't obvious enough whether HEAD == manifest-rev or not

To try to resolve 2. without making 1. any worse, explicitly print
HEAD and manifest-rev if they differ, without adding any extra lines,
by consolidating some of the output into the relevant banner line.

To solve 3., indent the status output to the same level as the
'status:' text in the banner.

Fixes: #650
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-04-17 12:29:15 -07:00
Martí Bolívar c0de98350d compare: add command
This command prints information about manifest-rev and "git status"
output for a project if (and only if) at least one of the following is
true:

1. its checked out commit is different than the commit checked
   out by the most recent successful "west update"
2. its working tree is not clean (i.e. there are local uncommitted
   changes)
3. it has a local checked out branch (unless overridden via
   the compare.ignore-branches config option or the --ignore-branches
   command line option)

It does something similar for the manifest repository.

The purpose of this is to allow people who are developing on one or
more projects to quickly see if they have anything interesting going
on, or if their projects match the manifest-rev versions.

Fixes: #642
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
2023-04-11 23:33:09 -07:00