The documentation says that the WEST_CONFIG_LOCAL environment variable
must override the default file location, <topdir>/.west/config, if
set. It doesn't, though; fix that.
This requires us to be a bit more careful in how we write the fixture
for the west.configuration test suite, and requires updating a test
case to match the documentation.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Otherwise, setup functions that set git config variables like
user.name are potentially dangerous to run.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
West has historically relied on the third-party configobj library for
writing configuration files instead of using the standard library's
configparser module. The reason why is that configobj has round-trip
support for comments.
However, the public reading API uses configparser. Up until now, we
were assuming that the two were compatible for the simple purposes we
needed, and indeed they've proven compatible "enough" that the
different code paths on read vs. write haven't been an issue.
This has become a problem now that we are introducing the
manifest.groups configuration option, though, because its value
contains commas. The configparser and configobj file formats have a
semantic difference between these two options, though:
[section]
foo = "one,two"
bar = one,two
The difference is:
- in configobj, 'foo' is the string "one,two" and 'bar' is the list
['one', 'two']
- in configparser, 'foo' is the string '"one,two"' and bar is the string
'one,two'
Further, the configobj library automatically adds quotes around any
string that contains commas to enforce this distinction.
This is breaking round-trips, since:
west config section.foo one,two # configobj writes "one,two"
west config section.foo # configparser reads '"one,two"'
Looking at it further, configobj development seems to have stalled in
2014, and the most significant user it claims in its
documentation (IPython) has moved on to .py and .json configuration
files.
This isn't worth the hassle. Just drop the configobj dependency and
use configparser everywhere. This will delete comments that users have
added to their configuration files and may otherwise reorder sections,
but having the round-trip semantics correct is more important than
that.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is currently failing because we are using configparser to read,
but configobj to write. The two don't seem to be compatible.
Setting an option to 'bar,baz' uses configobj, which writes the string
"bar,baz" -- with quotes -- to the config file. Then when we read it
back with configparser, we get the value '"bar,baz"' -- quotes are
not stripped.
This will need to be fixed.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add a topdir argument to each of these functions:
- read_config()
- update_config()
- delete_config()
If set, then topdir/.west/config will be used as the local
configuration file if necessary. This allows users of this module to
decouple themselves from west.util.west_dir() without having to resort
to temporarily setting environment variables.
Keep the tests up to date, adding a couple of test cases to make sure
the new kwarg works as expected.
While we're here, re-work the fixture setup so we don't ever have to
monkey-patch west.configuration._location() by always pointing the
WEST_CONFIG_xxx environment variables inside the tmpdir we're running
each test case from. This lets us avoid some extra work in tox.ini and
makes the resulting test code easier to read, without sacrificing any
safety in terms of touching the user's actual files.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
We currently have no way to delete options once set. This is not good
for users that want to delete things so they go back to their default
values. Add a library API and tests for this; we'll extend the config
command with options that use them next.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add three new environment variables which can be used to override the
configuration file locations: WEST_CONFIG_SYSTEM, WEST_CONFIG_GLOBAL,
and WEST_CONFIG_LOCAL.
Use them in the tests. This also has the beneficial side effect of
making sure that "real" system settings do not interfere with the test
environment.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This prints all existing config options and their values.
It can be combined with the --system, --global, and --local options
to just list the contents of those files.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
This makes sense, and matches git config's behavior too. Stay silent
by default, but log.dbg() what happened.
Add a test case, too.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add basic tests for the behavior of the west.configuration APIs for
ConfigFile.SYSTEM, which were missing since we couldn't patch out the
location to somewhere safe.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Add a helper to create the configuration file if it doesn't already
exist, and call it from update_config().
Add test cases for the API-level changes, which we can do now that we
can patch out all the config file locations.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
The XDG_CONFIG_HOME variable, if set, does not specify an additional
place to look for config options: it says they should go exactly
there. Fix that. Add support for BSD platforms while we are here.
To improve testability in future patches, move the code which gets the
location for a config file into a function that we can monkeypatch
later. This will make it easier to test system level config settings.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Improve the test coverage and make sure that the command line
interface and module API work together as documented.
Improve readability by adding helpers, decreasing verbosity, and
marking the mandatory setup fixture autouse=True to cut boilerplate.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Instead of bothering to create a proper west init-based installation
when testing the configuration commands, use a fixture that just
creates what is needed, and also validates module-wide assumptions
instead of repeating them in each test case.
This improves the quality of the test by only testing what is needed,
helping to isolate these tests from unrelated errors in the manifest
handling code.
It also drops the test time from ~3.5 seconds to about ~2.5 seconds in
my Linux environment.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
'west config' testing is giving false negatives on Windows. To fix:
- canonicalize and fix up global config path testing
- avoid passing paths to cmd(); they don't play well with
shlex.split() and aren't necessary to make sure the config
command is behaving properly
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
There's no good reason to have these tests in subdirectories. Flatten
them out. Keep the directory of invalid manifests separate to keep the
directory listing clean, though.
Signed-off-by: Marti Bolivar <marti.bolivar@nordicsemi.no>
Martí Bolívar