From 61374f15f1ff9cac59b563d512846c3ae8418317 Mon Sep 17 00:00:00 2001 From: Ivan Kravets Date: Tue, 20 Mar 2018 16:06:39 +0200 Subject: [PATCH] Fix issue with ``build_unflags`` option when a macro contains value --- .pytest_cache/v/cache/lastfailed | 1 + HISTORY.rst | 2 + platformio/builder/tools/platformio.py | 77 +++++++++++++----------- tests/test_builder.py | 81 ++++++++++++++++++++++++++ 4 files changed, 127 insertions(+), 34 deletions(-) create mode 100644 .pytest_cache/v/cache/lastfailed create mode 100644 tests/test_builder.py diff --git a/.pytest_cache/v/cache/lastfailed b/.pytest_cache/v/cache/lastfailed new file mode 100644 index 00000000..9e26dfee --- /dev/null +++ b/.pytest_cache/v/cache/lastfailed @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/HISTORY.rst b/HISTORY.rst index 252c3000..644d7066 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -19,6 +19,8 @@ PlatformIO 3.0 `PlatformIO Home `_) * Fixed issue with useless project rebuilding for case insensitive file systems (Windows) +* Fixed issue with ``build_unflags`` option when a macro contains value + (e.g., ``-DNAME=VALUE``) 3.5.2 (2018-03-13) ~~~~~~~~~~~~~~~~~~ diff --git a/platformio/builder/tools/platformio.py b/platformio/builder/tools/platformio.py index da82de85..00649dd9 100644 --- a/platformio/builder/tools/platformio.py +++ b/platformio/builder/tools/platformio.py @@ -119,38 +119,47 @@ def BuildProgram(env): return program -def ProcessFlags(env, flags): # pylint: disable=too-many-branches - if not flags: - return +def ParseFlagsExtended(env, flags): if isinstance(flags, list): flags = " ".join(flags) - parsed_flags = env.ParseFlags(str(flags)) - for flag in parsed_flags.pop("CPPDEFINES"): - if not Util.is_Sequence(flag): - env.Append(CPPDEFINES=flag) + result = env.ParseFlags(str(flags)) + + cppdefines = [] + for item in result['CPPDEFINES']: + if not Util.is_Sequence(item): + cppdefines.append(item) continue - _key, _value = flag[:2] - if '\"' in _value: - _value = _value.replace('\"', '\\\"') - elif _value.isdigit(): - _value = int(_value) - elif _value.replace(".", "", 1).isdigit(): - _value = float(_value) - env.Append(CPPDEFINES=(_key, _value)) - env.Append(**parsed_flags) + name, value = item[:2] + if '\"' in value: + value = value.replace('\"', '\\\"') + elif value.isdigit(): + value = int(value) + elif value.replace(".", "", 1).isdigit(): + value = float(value) + cppdefines.append((name, value)) + result['CPPDEFINES'] = cppdefines # fix relative CPPPATH & LIBPATH for k in ("CPPPATH", "LIBPATH"): - for i, p in enumerate(env.get(k, [])): + for i, p in enumerate(result.get(k, [])): if isdir(p): - env[k][i] = realpath(p) + result[k][i] = realpath(p) + # fix relative path for "-include" - for i, f in enumerate(env.get("CCFLAGS", [])): + for i, f in enumerate(result.get("CCFLAGS", [])): if isinstance(f, tuple) and f[0] == "-include": - env['CCFLAGS'][i] = (f[0], env.File(realpath(f[1].get_path()))) + result['CCFLAGS'][i] = (f[0], env.File(realpath(f[1].get_path()))) + + return result + + +def ProcessFlags(env, flags): # pylint: disable=too-many-branches + if not flags: + return + env.Append(**env.ParseFlagsExtended(flags)) # Cancel any previous definition of name, either built in or - # provided with a -D option // Issue #191 + # provided with a -U option // Issue #191 undefines = [ u for u in env.get("CCFLAGS", []) if isinstance(u, basestring) and u.startswith("-U") @@ -164,19 +173,18 @@ def ProcessFlags(env, flags): # pylint: disable=too-many-branches def ProcessUnFlags(env, flags): if not flags: return - if isinstance(flags, list): - flags = " ".join(flags) - parsed_flags = env.ParseFlags(str(flags)) - all_flags = [] - for items in parsed_flags.values(): - all_flags.extend(items) - all_flags = set(all_flags) - - for key in parsed_flags: - cur_flags = set(env.Flatten(env.get(key, []))) - for item in cur_flags & all_flags: - while item in env[key]: - env[key].remove(item) + for key, unflags in env.ParseFlagsExtended(flags).items(): + for unflag in unflags: + if not isinstance(unflag, (list, tuple)): + unflag = tuple([unflag]) + for current in env.get(key, []): + conditions = [ + unflag == current, + isinstance(current, (tuple, list)) + and unflag[0] == current[0] + ] + if any(conditions): + env[key].remove(current) def IsFileWithExt(env, file_, ext): # pylint: disable=W0613 @@ -298,6 +306,7 @@ def exists(_): def generate(env): env.AddMethod(BuildProgram) + env.AddMethod(ParseFlagsExtended) env.AddMethod(ProcessFlags) env.AddMethod(ProcessUnFlags) env.AddMethod(IsFileWithExt) diff --git a/tests/test_builder.py b/tests/test_builder.py new file mode 100644 index 00000000..675dbf65 --- /dev/null +++ b/tests/test_builder.py @@ -0,0 +1,81 @@ +# Copyright (c) 2014-present PlatformIO +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from platformio.commands.run import cli as cmd_run + + +def test_build_flags(clirunner, validate_cliresult, tmpdir): + build_flags = [("-D TEST_INT=13", "-DTEST_INT=13"), ("-DTEST_SINGLE_MACRO", "-DTEST_SINGLE_MACRO"), + ('-DTEST_STR_SPACE="Andrew Smith"', '"-DTEST_STR_SPACE=Andrew Smith"')] + + tmpdir.join("platformio.ini").write(""" +[env:native] +platform = native +build_flags = %s + """ % " ".join([f[0] for f in build_flags])) + + tmpdir.mkdir("src").join("main.cpp").write(""" +#if !defined(TEST_INT) || TEST_INT != 13 +#error "TEST_INT" +#endif + +#ifndef TEST_STR_SPACE +#error "TEST_STR_SPACE" +#endif + +int main() { +} +""") + + result = clirunner.invoke( + cmd_run, ["--project-dir", str(tmpdir), "--verbose"]) + validate_cliresult(result) + build_output = result.output[result.output.find( + "Scanning dependencies..."):] + for flag in build_flags: + assert flag[1] in build_output, flag + + +def test_build_unflags(clirunner, validate_cliresult, tmpdir): + tmpdir.join("platformio.ini").write(""" +[env:native] +platform = native +build_unflags = -DTMP_MACRO1=45 -I. -DNON_EXISTING_MACRO +extra_scripts = pre:extra.py +""") + + tmpdir.join("extra.py").write(""" +Import("env") +env.Append(CPPPATH="%s") +env.Append(CPPDEFINES="TMP_MACRO1") +env.Append(CPPDEFINES=["TMP_MACRO2"]) +env.Append(CPPDEFINES=("TMP_MACRO3", 13)) + """ % str(tmpdir)) + + tmpdir.mkdir("src").join("main.c").write(""" +#ifdef TMP_MACRO1 +#error "TMP_MACRO1 should be removed" +#endif + +int main() { +} +""") + + result = clirunner.invoke( + cmd_run, ["--project-dir", str(tmpdir), "--verbose"]) + validate_cliresult(result) + build_output = result.output[result.output.find( + "Scanning dependencies..."):] + assert "-DTMP_MACRO1" not in build_output + assert str(tmpdir) not in build_output