configuration: fix round-trip bugs by removing configobj

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>

setup.py

@@ -44,7 +44,6 @@
         'colorama',
         'PyYAML>=5.1',
         'pykwalify',
-        'configobj',
         'setuptools',
         'packaging',
     ],

src/west/configuration.py

@@ -42,8 +42,6 @@
 from enum import Enum
 from typing import Any, Optional, List
 
-import configobj
-
 from west.util import west_dir, WestNotFound, PathType
 
 def _configparser():            # for internal use
@@ -129,11 +127,13 @@ def update_config(section: str, key: str, value: Any,
         raise ValueError(f'invalid configfile: {configfile}')
 
     filename = _ensure_config(configfile, topdir)
-    updater = configobj.ConfigObj(filename)
-    if section not in updater:
-        updater[section] = {}
-    updater[section][key] = value
-    updater.write()
+    config = _configparser()
+    config.read(filename)
+    if section not in config:
+        config[section] = {}
+    config[section][key] = value
+    with open(filename, 'w') as f:
+        config.write(f)
 
 def delete_config(section: str, key: str,
                   configfile: Optional[ConfigFile] = None,
@@ -180,14 +180,16 @@ def delete_config(section: str, key: str,
 
     found = False
     for path in to_check:
-        cobj = configobj.ConfigObj(path)
-        if section not in cobj or key not in cobj[section]:
+        config = _configparser()
+        config.read(path)
+        if section not in config or key not in config[section]:
             continue
 
-        del cobj[section][key]
-        if not cobj[section].items():
-            del cobj[section]
-        cobj.write()
+        del config[section][key]
+        if not config[section].items():
+            del config[section]
+        with open(path, 'w') as f:
+            config.write(f)
         found = True
         if stop:
             break

tests/test_config.py

@@ -498,7 +498,6 @@ def sorted_list(other_args=''):
     assert sorted_list('--local') == ['pytest.bar=what',
                                       'pytest.foo=who']
 
-@pytest.mark.xfail()
 def test_round_trip():
     cmd('config pytest.foo bar,baz')
     assert cmd('config pytest.foo').strip() == 'bar,baz'