Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppendUnique breaks CPPScanner when using defs without value #4108

Closed
Inviz opened this issue Mar 3, 2022 · 3 comments · Fixed by #4110
Closed

AppendUnique breaks CPPScanner when using defs without value #4108

Inviz opened this issue Mar 3, 2022 · 3 comments · Fixed by #4110
Labels

Comments

@Inviz
Copy link

Inviz commented Mar 3, 2022

AppendUnique turns string values into tuples with single values, breaking dictify_CPPDEFINES, and thus CPPScanner.

Details of my investigation here: https://community.platformio.org/t/libraries-included-inside-conditional-guard-are-still-added-to-the-dep-graph-by-ldf/26326/4

  1. Platformio inside its ProcessFlags parses defs like -DA -DB=C into ('A', ('B', 'C')) for env[“CPPDEFINES”]
  2. libopencm3.py script tries to add -DSTM32F4 def via env.ApppendUnique(CPPDEFINES=["STM32F4"]) (which is not really necessary, but that’s irrelevant)
  3. env.AppendUnique turns all previous defs without value (that were strings) into tuples with single value, and adds new values also as tuples: (('A', ), ('B', 'C'), ('STM32F4',))
  4. CPPScanner expects defs without value to be strings, but any invocation of env.AppendUnique breaks that expectation
  5. Scons crashes:
def dictify_CPPDEFINES(env):
    cppdefines = env.get('CPPDEFINES', {})
    if cppdefines is None:
        return {}
    if SCons.Util.is_Sequence(cppdefines):
        result = {}
        for c in cppdefines:
            if SCons.Util.is_Sequence(c):
                result[c[0]] = c[1] # <-- [1] tuple index out of range
            else:
                result[c] = None
        return result

Required information

  • Version of SCons 4.3
  • Version of Python 3.9
  • Which python distribution if applicable (python.org, cygwin, anaconda, macports, brew,etc) brew
  • How you installed SCons viaPIO
  • What Platform are you on? (Linux/Windows and which version) mac
@Inviz Inviz changed the title AppendUnique breaks CPPScanner AppendUnique breaks CPPScanner when using defs without value Mar 3, 2022
@mwichmann
Copy link
Collaborator

Looks like it's easy enough to patch the hole here, without necessarily tackling the deeper question of why we have keep converting the type of CPPDEFINES.

Do you have a simple example that actually tickles the IndexError

Meanwhile, /me scratches head over:

env = Environment()
env.Append(CPPDEFINES="A")
env.Append(CPPDEFINES="B=C")
env.Append(CPPDEFINES=["STM32F4"])
env.AppendUnique(CPPDEFINES=["STM32F4"])

Showing

env['CPPDEFINES']=['A']
env['CPPDEFINES']=['A', 'B=C']
env['CPPDEFINES']=['A', 'B=C', 'STM32F4']
env['CPPDEFINES']=[('A',), ('B=C',), ('STM32F4',)]

as in - why does Append's speciai-case handling for CPPDEFINES not tuple-ify and AppendUnique's does?

@mwichmann mwichmann added the bug label Mar 3, 2022
@Inviz
Copy link
Author

Inviz commented Mar 3, 2022

I think dictify_CPPDEFINES(env) is enough to invoke the issue:

env = Environment()
env.AppendUnique(CPPDEFINES="A")
dictify_CPPDEFINES(env)

I think more proper example of difference would be to env.Append(CPPDEFINES=("B", "C")) (or whatever syntax is to append the tuple, I'm not fluent in python). You'll see that it'll add tuble for ("B", "C"). But for case with no value (e.g. "B") it will keep the string in it. So that means AppendUnique doesn't simplify the values, while Append does (and then other things rely on it)

@mwichmann
Copy link
Collaborator

Indeed, it is...

from SCons.Scanner.C import dictify_CPPDEFINES
d = dictify_CPPDEFINES(env)

mwichmann added a commit to mwichmann/scons that referenced this issue Mar 3, 2022
In converting CPPDEFINES to a dict, if an element is a single-item
sequence c, it would take an IndexError trying to access c[1]. This
could happen if AppendUnique has been called as it converts to tuples.

Fixes SCons#4108

Signed-off-by: Mats Wichmann <mats@linux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants