Fix issue with `build_unflags` option when a macro contains value

.pytest_cache/v/cache/lastfailed

@@ -0,0 +1 @@
+{}

HISTORY.rst

@@ -19,6 +19,8 @@ PlatformIO 3.0
   `PlatformIO Home <http://docs.platformio.org/page/home/index.html>`_)
 * 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)
 ~~~~~~~~~~~~~~~~~~

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)

tests/test_builder.py

@@ -0,0 +1,81 @@
+# Copyright (c) 2014-present PlatformIO <contact@platformio.org>
+#
+# 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