platformio-core icon indicating copy to clipboard operation
platformio-core copied to clipboard

Fix: Handle -U flags with spaces in build_flags

Open SpacePlushy opened this issue 2 months ago • 2 comments

Summary

Fixes #5237

When using -U MACRO (with space) in build_flags, SCons' ParseFlags() splits it into two separate items in CCFLAGS: ['-U', 'MACRO']. The previous code only captured '-U' and attempted undef[2:] which resulted in an empty string.

Changes

This PR modifies the ProcessFlags function in platformio/builder/tools/piobuild.py to:

  1. Iterate through CCFLAGS with an index to detect standalone -U flags
  2. Pair -U with the following macro name when they're separate items
  3. Handle both formats:
    • -UMACRO (no space) - already working ✓
    • -U MACRO (with space) - now fixed ✓

The solution follows a similar pattern to how SCons handles -D flags with spaces.

Testing

Verified the fix handles:

  • Single -U MACRO format with space
  • Single -UMACRO format without space
  • Mixed formats in the same build_flags
  • Multiple undefines

Example

Before (broken):

build_flags = -U LORA_TX_POWER

Result: Only -U captured, macro name lost

After (fixed):

build_flags = -U LORA_TX_POWER

Result: Correctly passes -U LORA_TX_POWER to gcc

SpacePlushy avatar Oct 29 '25 17:10 SpacePlushy

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Oct 29 '25 17:10 CLAassistant

Note on testing:

In my initial comment on #5237, I mentioned including tests with this PR. After exploring the codebase, I noticed there aren't existing unit tests for the builder tools (specifically for ProcessFlags and related functions in piobuild.py).

I didn't want to add a new test structure without understanding your preferred testing approach for the builder module. I'm happy to add tests if you'd like - just let me know:

  1. Should I create a new test file for builder tools?
  2. Is there a preferred testing pattern you'd like me to follow?
  3. Or would you prefer to handle testing separately?

I manually verified the fix handles all edge cases (both -U MACRO and -UMACRO formats, mixed usage, etc.), but I'm ready to formalize that into proper tests if needed.

SpacePlushy avatar Oct 29 '25 17:10 SpacePlushy