Skip to content

Commit

Permalink
Fixed an issue when no error is raised if referred parameter (interpo…
Browse files Browse the repository at this point in the history
…lation) is missing in a project configuration file // Resolve #3279
  • Loading branch information
ivankravets committed Feb 8, 2020
1 parent a78b461 commit 2763853
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 46 deletions.
2 changes: 1 addition & 1 deletion HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ PlatformIO Core 4.0
* Fixed an issue when Project Inspector crashes when flash use > 100% (`issue #3368 <https://github.com/platformio/platformio-core/issues/3368>`_)
* Fixed a "UnicodeDecodeError" when listing built-in libraries on macOS with Python 2.7 (`issue #3370 <https://github.com/platformio/platformio-core/issues/3370>`_)
* Fixed an issue with improperly handled compiler flags with space symbols in VSCode template (`issue #3364 <https://github.com/platformio/platformio-core/issues/3364>`_)

* Fixed an issue when no error is raised if referred parameter (interpolation) is missing in a project configuration file (`issue #3279 <https://github.com/platformio/platformio-core/issues/3279>`_)

4.1.0 (2019-11-07)
~~~~~~~~~~~~~~~~~~
Expand Down
78 changes: 44 additions & 34 deletions platformio/project/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import click

from platformio import fs
from platformio.compat import PY2, WINDOWS, hashlib_encode_data
from platformio.compat import PY2, WINDOWS, hashlib_encode_data, string_types
from platformio.project import exception
from platformio.project.options import ProjectOptions

Expand All @@ -43,6 +43,9 @@
"""


MISSING = object()


class ProjectConfigBase(object):

INLINE_COMMENT_RE = re.compile(r"\s+;.*$")
Expand Down Expand Up @@ -242,22 +245,51 @@ def set(self, section, option, value):
value = "\n" + value
self._parser.set(section, option, value)

def getraw(self, section, option):
def getraw( # pylint: disable=too-many-branches
self, section, option, default=MISSING
):
if not self.expand_interpolations:
return self._parser.get(section, option)

value = None
found = False
value = MISSING
for sec, opt in self.walk_options(section):
if opt == option:
value = self._parser.get(sec, option)
found = True
break

if not found:
value = self._parser.get(section, option)
option_meta = ProjectOptions.get("%s.%s" % (section.split(":", 1)[0], option))
if not option_meta:
if value == MISSING:
value = (
default if default != MISSING else self._parser.get(section, option)
)
return self._expand_interpolations(value)

if option_meta.sysenvvar:
envvar_value = os.getenv(option_meta.sysenvvar)
if not envvar_value and option_meta.oldnames:
for oldoption in option_meta.oldnames:
envvar_value = os.getenv("PLATFORMIO_" + oldoption.upper())
if envvar_value:
break
if envvar_value and option_meta.multiple:
value += ("" if value == MISSING else "\n") + envvar_value
elif envvar_value and value == MISSING:
value = envvar_value

if value == MISSING:
value = option_meta.default or default
if value == MISSING:
return None

return self._expand_interpolations(value)

if "${" not in value or "}" not in value:
def _expand_interpolations(self, value):
if (
not value
or not isinstance(value, string_types)
or not all(["${" in value, "}" in value])
):
return value
return self.VARTPL_RE.sub(self._re_interpolation_handler, value)

Expand All @@ -267,41 +299,19 @@ def _re_interpolation_handler(self, match):
return os.getenv(option)
return self.getraw(section, option)

def get(self, section, option, default=None): # pylint: disable=too-many-branches
def get(self, section, option, default=MISSING):
value = None
try:
value = self.getraw(section, option)
except (ConfigParser.NoSectionError, ConfigParser.NoOptionError):
pass # handle value from system environment
value = self.getraw(section, option, default)
except ConfigParser.Error as e:
raise exception.InvalidProjectConfError(self.path, str(e))

option_meta = ProjectOptions.get("%s.%s" % (section.split(":", 1)[0], option))
if not option_meta:
return value or default
return value

if option_meta.multiple:
value = self.parse_multi_values(value)

if option_meta.sysenvvar:
envvar_value = os.getenv(option_meta.sysenvvar)
if not envvar_value and option_meta.oldnames:
for oldoption in option_meta.oldnames:
envvar_value = os.getenv("PLATFORMIO_" + oldoption.upper())
if envvar_value:
break
if envvar_value and option_meta.multiple:
value = value or []
value.extend(self.parse_multi_values(envvar_value))
elif envvar_value and not value:
value = envvar_value

# option is not specified by user
if value is None or (
option_meta.multiple and value == [] and option_meta.default
):
return default if default is not None else option_meta.default

value = self.parse_multi_values(value or [])
try:
return self.cast_to(value, option_meta.type)
except click.BadParameter as e:
Expand Down
32 changes: 21 additions & 11 deletions tests/test_projectconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import pytest

from platformio.project.config import ConfigParser, ProjectConfig
from platformio.project.exception import UnknownEnvNamesError
from platformio.project.exception import InvalidProjectConfError, UnknownEnvNamesError

BASE_CONFIG = """
[platformio]
Expand All @@ -34,6 +34,7 @@
Lib1 ; inline comment in multi-line value
Lib2
lib_ignore = ${custom.lib_ignore}
custom_builtin_option = ${env.build_type}
[strict_ldf]
lib_ldf_mode = chain+
Expand All @@ -54,7 +55,7 @@
[env:base]
build_flags = ${custom.debug_flags} ${custom.extra_flags}
lib_compat_mode = ${strict_ldf.strict}
lib_compat_mode = ${strict_ldf.lib_compat_mode}
targets =
[env:test_extends]
Expand Down Expand Up @@ -100,13 +101,10 @@ def config(tmpdir_factory):

def test_empty_config():
config = ProjectConfig("/non/existing/platformio.ini")

# unknown section
with pytest.raises(ConfigParser.NoSectionError):
config.getraw("unknown_section", "unknown_option")

with pytest.raises(InvalidProjectConfError):
config.get("unknown_section", "unknown_option")
assert config.sections() == []
assert config.get("section", "option") is None
assert config.get("section", "option", 13) == 13


Expand Down Expand Up @@ -159,6 +157,7 @@ def test_options(config):
"custom_monitor_speed",
"lib_deps",
"lib_ignore",
"custom_builtin_option",
]
assert config.options(env="test_extends") == [
"extends",
Expand All @@ -169,6 +168,7 @@ def test_options(config):
"custom_monitor_speed",
"lib_deps",
"lib_ignore",
"custom_builtin_option",
]


Expand All @@ -180,7 +180,7 @@ def test_has_option(config):


def test_sysenv_options(config):
assert config.get("custom", "extra_flags") is None
assert config.getraw("custom", "extra_flags") == ""
assert config.get("env:base", "build_flags") == ["-D DEBUG=1"]
assert config.get("env:base", "upload_port") is None
assert config.get("env:extra_2", "upload_port") == "/dev/extra_2/port"
Expand All @@ -205,6 +205,7 @@ def test_sysenv_options(config):
"custom_monitor_speed",
"lib_deps",
"lib_ignore",
"custom_builtin_option",
"upload_port",
]

Expand All @@ -227,6 +228,10 @@ def test_getraw_value(config):
with pytest.raises(ConfigParser.NoOptionError):
config.getraw("platformio", "monitor_speed")

# default
assert config.getraw("unknown", "option", "default") == "default"
assert config.getraw("env:base", "custom_builtin_option") == "release"

# known
assert config.getraw("env:base", "targets") == ""
assert config.getraw("env:extra_1", "lib_deps") == "574"
Expand Down Expand Up @@ -258,17 +263,18 @@ def test_items(config):
assert config.items("custom") == [
("debug_flags", "-D DEBUG=1"),
("lib_flags", "-lc -lm"),
("extra_flags", None),
("extra_flags", ""),
("lib_ignore", "LibIgnoreCustom"),
]
assert config.items(env="base") == [
("build_flags", ["-D DEBUG=1"]),
("lib_compat_mode", "soft"),
("lib_compat_mode", "strict"),
("targets", []),
("monitor_speed", 9600),
("custom_monitor_speed", "115200"),
("lib_deps", ["Lib1", "Lib2"]),
("lib_ignore", ["LibIgnoreCustom"]),
("custom_builtin_option", "release"),
]
assert config.items(env="extra_1") == [
(
Expand All @@ -279,6 +285,7 @@ def test_items(config):
("monitor_speed", 9600),
("custom_monitor_speed", "115200"),
("lib_ignore", ["LibIgnoreCustom"]),
("custom_builtin_option", "release"),
]
assert config.items(env="extra_2") == [
("build_flags", ["-Og"]),
Expand All @@ -287,6 +294,7 @@ def test_items(config):
("monitor_speed", 9600),
("custom_monitor_speed", "115200"),
("lib_deps", ["Lib1", "Lib2"]),
("custom_builtin_option", "release"),
]
assert config.items(env="test_extends") == [
("extends", ["strict_settings"]),
Expand All @@ -297,6 +305,7 @@ def test_items(config):
("custom_monitor_speed", "115200"),
("lib_deps", ["Lib1", "Lib2"]),
("lib_ignore", ["LibIgnoreCustom"]),
("custom_builtin_option", "release"),
]


Expand Down Expand Up @@ -393,6 +402,7 @@ def test_dump(tmpdir_factory):
("custom_monitor_speed", "115200"),
("lib_deps", ["Lib1", "Lib2"]),
("lib_ignore", ["${custom.lib_ignore}"]),
("custom_builtin_option", "${env.build_type}"),
],
),
("strict_ldf", [("lib_ldf_mode", "chain+"), ("lib_compat_mode", "strict")]),
Expand All @@ -414,7 +424,7 @@ def test_dump(tmpdir_factory):
"env:base",
[
("build_flags", ["${custom.debug_flags} ${custom.extra_flags}"]),
("lib_compat_mode", "${strict_ldf.strict}"),
("lib_compat_mode", "${strict_ldf.lib_compat_mode}"),
("targets", []),
],
),
Expand Down

5 comments on commit 2763853

@mcspr
Copy link
Contributor

@mcspr mcspr commented on 2763853 Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: xoseperez/espurna#2146
this commit also causes our esp8266 environment to link with AsyncTCP (non-compatible, since it is only esp32) as dependency of fauxmoESP, ESPAsyncWebServer and async-mqtt-client
rolling back git checkout HEAD~1 from this one fixes the issue

@ivankravets
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, do we have problem with this commit?

@mcspr
Copy link
Contributor

@mcspr mcspr commented on 2763853 Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry, this is the wrong one
0a4bc1d4e#diff-553e8530815b35759c21d93b2f28baaaR517
we don't specify lib_compat_mode, so the default soft causes AsyncTCP to build for us
adding lib_compat_mode = strict to the .ini fixes the issue

@ivankravets
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcspr
Copy link
Contributor

@mcspr mcspr commented on 2763853 Feb 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did, yes. AsyncTCP will still be installed, but no longer builds

Please sign in to comment.