From a3b9cd875d2590d1992f837b9079ff7d340e35f8 Mon Sep 17 00:00:00 2001 From: Marti Bolivar Date: Fri, 24 May 2019 18:48:48 -0600 Subject: [PATCH] configuration: create missing config files 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 --- src/west/commands/config.py | 9 ++++- src/west/configuration.py | 26 ++++++++++++- tests/test_config.py | 75 +++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/src/west/commands/config.py b/src/west/commands/config.py index 4e120cc..2ea136f 100644 --- a/src/west/commands/config.py +++ b/src/west/commands/config.py @@ -121,4 +121,11 @@ class Config(WestCommand): if configfile == ConfigFile.ALL: # No file given, thus writing defaults to LOCAL configfile = ConfigFile.LOCAL - configuration.update_config(section, key, args.value, configfile) + try: + configuration.update_config(section, key, args.value, + configfile) + except PermissionError as pe: + log.die("can't set {}.{}: permission denied when writing {}{}". + format(section, key, pe.filename, + ('; are you root/administrator?' + if configfile == ConfigFile.SYSTEM else ''))) diff --git a/src/west/configuration.py b/src/west/configuration.py index 52ac94d..2890a85 100644 --- a/src/west/configuration.py +++ b/src/west/configuration.py @@ -34,6 +34,7 @@ lowest. import configparser import os +import pathlib import platform from enum import Enum try: @@ -112,7 +113,7 @@ def update_config(section, key, value, configfile=ConfigFile.LOCAL): # Not possible to update ConfigFile.ALL, needs specific conf file here. raise ValueError('invalid configfile: {}'.format(configfile)) - filename = _location(configfile) + filename = _ensure_config(configfile) if use_configobj: updater = configobj.ConfigObj(filename) @@ -182,6 +183,29 @@ def _gather_configs(cfg): return ret +def _ensure_config(configfile): + # Ensure the given configfile exists, returning its path. May + # raise permissions errors, WestNotFound, etc. + # + # Uses pathlib as this is hard to implement correctly without it. + loc = _location(configfile) + path = pathlib.Path(loc) + + if path.is_file(): + return loc + + # Create the directory. We can't use + # path.parent.mkdir(..., exist_ok=True) + # in Python 3.4, so roughly emulate its behavior. + try: + path.parent.mkdir(parents=True) + except FileExistsError: + pass + + path.touch(exist_ok=True) + return canon_path(str(path)) + + def use_colors(): # Convenience function for reading the color.ui setting return config.getboolean('color', 'ui', fallback=True) diff --git a/tests/test_config.py b/tests/test_config.py index c2fb909..27fd377 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -5,6 +5,7 @@ import configparser import os import subprocess +from unittest.mock import patch import pytest @@ -24,6 +25,22 @@ def cfg(f=ALL): config.read_config(config_file=f, config=cp) return cp +def tstloc(cfg): + # Monkeypatch for the config file location. assumes we are called + # in a tmpdir. + # + # Important: This is only safe to use to test the API in this process. + # DON'T call cmd('config ...'); subprocesses aren't patched, + # so the system location is the "real" one. + if cfg == SYSTEM: + return os.path.join(os.getcwd(), 'system', 'config') + elif cfg == GLOBAL: + return os.path.join(os.getcwd(), 'global', 'config') + elif cfg == LOCAL: + return os.path.join(os.getcwd(), 'local', 'config') + else: + raise ValueError('cfg: {}'.format(cfg)) + @pytest.fixture(autouse=True) def config_tmpdir(tmpdir): # Fixture for running from a temporary directory with a .west @@ -128,6 +145,64 @@ def test_config_local(): assert 'pytest' not in glb assert lcl['pytest']['local2'] == 'foo2' +@patch('west.configuration._location', new=tstloc) +def test_system_creation(): + # Test that the system file -- and just that file -- is created on + # demand. + # + # Since we use tstloc(), we can't call cmd('config ...') here. + assert not os.path.isfile(tstloc(SYSTEM)) + assert not os.path.isfile(tstloc(GLOBAL)) + assert not os.path.isfile(tstloc(LOCAL)) + + config.update_config('pytest', 'key', 'val', configfile=SYSTEM) + + assert os.path.isfile(tstloc(SYSTEM)) + assert not os.path.isfile(tstloc(GLOBAL)) + assert not os.path.isfile(tstloc(LOCAL)) + assert cfg(f=ALL)['pytest']['key'] == 'val' + assert cfg(f=SYSTEM)['pytest']['key'] == 'val' + assert 'pytest' not in cfg(f=GLOBAL) + assert 'pytest' not in cfg(f=LOCAL) + +@patch('west.configuration._location', new=tstloc) +def test_global_creation(): + # Like test_system_creation, for global config options. + # + # Since we use tstloc(), we can't call cmd('config ...') here. + assert not os.path.isfile(tstloc(SYSTEM)) + assert not os.path.isfile(tstloc(GLOBAL)) + assert not os.path.isfile(tstloc(LOCAL)) + + config.update_config('pytest', 'key', 'val', configfile=GLOBAL) + + assert not os.path.isfile(tstloc(SYSTEM)) + assert os.path.isfile(tstloc(GLOBAL)) + assert not os.path.isfile(tstloc(LOCAL)) + assert cfg(f=ALL)['pytest']['key'] == 'val' + assert 'pytest' not in cfg(f=SYSTEM) + assert cfg(f=GLOBAL)['pytest']['key'] == 'val' + assert 'pytest' not in cfg(f=LOCAL) + +@patch('west.configuration._location', new=tstloc) +def test_local_creation(): + # Like test_system_creation, for local config options. + # + # Since we use tstloc(), we can't call cmd('config ...') here. + assert not os.path.isfile(tstloc(SYSTEM)) + assert not os.path.isfile(tstloc(GLOBAL)) + assert not os.path.isfile(tstloc(LOCAL)) + + config.update_config('pytest', 'key', 'val', configfile=LOCAL) + + assert not os.path.isfile(tstloc(SYSTEM)) + assert not os.path.isfile(tstloc(GLOBAL)) + assert os.path.isfile(tstloc(LOCAL)) + assert cfg(f=ALL)['pytest']['key'] == 'val' + assert 'pytest' not in cfg(f=SYSTEM) + assert 'pytest' not in cfg(f=GLOBAL) + assert cfg(f=LOCAL)['pytest']['key'] == 'val' + def test_default_config(): # Writing to a value without a config destination should default # to --local.