optionally pass env to AddBuildMiddleware callback

[bkleiner]

It appears env is not yet fully when the pre-script is ran (env vs projenv?), however when Build Middlewares are employed as described, this incomplete environment is used to construct the env.Object entries, leading to missing compiler arguments.

Similar problems are mentioned here

This PR optionally, as to not break compatibility, passes the current environment to the callback, in-line with how its handled with Pre & Post action callbacks

To reproduce the issue:

def rewrite_source(node):
    return env.Object(node)

env.AddBuildMiddleware(rewrite_source)

leads to:

arm-none-eabi-gcc -o .pio/build/matekf405/src/config/profile.o -c -std=gnu11 -mfloat-abi=softfp -mfpu=fpv4-sp-d16 -s -O3 -flto -Wdouble-promotion -Wunsafe-loop-optimizations -fsingle-precision-constant -fno-exceptions -fno-strict-aliasing -fstack-usage -fno-stack-protector -fomit-frame-pointer -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-math-errno -fmerge-all-constants -funsafe-loop-optimizations -mthumb -mcpu=cortex-m4 -ffunction-sections -fdata-sections -Wall -nostdlib
-DPLATFORMIO=60104 -DSTM32F40_41xxx -DSTM32F405xx -DSTM32F4xx -DSTM32F405 -DSTM32F4 -DMCU_NAME=stm32f405 -DHSE_VALUE=8000000U -DUSE_FAST_RAM -DUSBD_SOF_DISABLED -DUSE_FULL_LL_DRIVER -DHSE_STARTUP_TIMEOUT=5000 -DUSE_HAL_DRIVER -DF_CPU=168000000L
-Isrc -Isrc/rx -Isrc/osd -Isrc/config -Isrc/drivers -Isrc/system/stm32f405 -Isrc/targets/matekf405 -Isrc -Isrc -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/Include -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/Device/ST/STM32F4xx/Include -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/STM32F4xx_HAL_Driver/Inc -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/STM32F4xx_HAL_Driver/Src -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/DSP/Include src/config/profile.c

Whereas:

def rewrite_source(node, localenv):
    return localenv.Object(node)

env.AddBuildMiddleware(rewrite_source)

leads to

arm-none-eabi-gcc -o .pio/build/matekf405/src/config/profile.o -c -std=gnu11 -mfloat-abi=softfp -mfpu=fpv4-sp-d16 -s -O3 -flto -Wdouble-promotion -Wunsafe-loop-optimizations -fsingle-precision-constant -fno-exceptions -fno-strict-aliasing -fstack-usage -fno-stack-protector -fomit-frame-pointer -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-math-errno -fmerge-all-constants -funsafe-loop-optimizations -mthumb -mcpu=cortex-m4 -ffunction-sections -fdata-sections -Wall -nostdlib 
-DPLATFORMIO=60104 -DSTM32F40_41xxx -DSTM32F405xx -DSTM32F4xx -DSTM32F405 -DSTM32F4 -DMCU_NAME=stm32f405 -DHSE_VALUE=8000000U -DUSE_FAST_RAM -DUSBD_SOF_DISABLED -DUSE_FULL_LL_DRIVER -DHSE_STARTUP_TIMEOUT=5000 -DUSE_HAL_DRIVER -DF_CPU=168000000L -DTARGET=stm32f405 -DGIT_VERSION=d4d201d 
-Ilib/cbor/include -Ilib/cbor/src -Ilib/libusb_stm32/inc -Ilib/libusb_stm32/src -Isrc -Isrc/rx -Isrc/osd -Isrc/config -Isrc/drivers -Isrc/system/stm32f405 -Isrc/targets/matekf405 -Isrc -Isrc -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/Include -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/Device/ST/STM32F4xx/Include -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/STM32F4xx_HAL_Driver/Inc -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/STM32F4xx_HAL_Driver/Src -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/DSP/Include src/config/profile.c

Take note of -DTARGET and the include dirs for cbor and libusb_stm32

[ivankravets]

Thanks for the PR! Could you try node.env.Object(...?

[ivankravets]

Ahh, sorry, it should be node.get_env()... See internal of SCons https://github.com/SCons/scons/search?q=get_env

If it works, we will update our docs. Sorry.

[bkleiner]

uhmm, no, with

def rewrite_source(node):
    localenv = node.get_env()
    return localenv.Object(node)

i still end up missing a bunch of arguments:

arm-none-eabi-gcc -o .pio/build/matekf405/src/config/profile.o -c -std=gnu11 -mfloat-abi=softfp -mfpu=fpv4-sp-d16 -s -O3 -flto -Wdouble-promotion -Wunsafe-loop-optimizations -fsingle-precision-constant -fno-exceptions -fno-strict-aliasing -fstack-usage -fno-stack-protector -fomit-frame-pointer -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-math-errno -fmerge-all-constants -funsafe-loop-optimizations -mthumb -mcpu=cortex-m4 -ffunction-sections -fdata-sections -Wall -nostdlib 
-DPLATFORMIO=60103 -DSTM32F40_41xxx -DSTM32F405xx -DSTM32F4xx -DSTM32F405 -DSTM32F4 -DMCU_NAME=stm32f405 -DHSE_VALUE=8000000U -DUSE_FAST_RAM -DUSBD_SOF_DISABLED -DUSE_FULL_LL_DRIVER -DHSE_STARTUP_TIMEOUT=5000 -DUSE_HAL_DRIVER -DF_CPU=168000000L
-Isrc -Isrc/rx -Isrc/osd -Isrc/config -Isrc/drivers -Isrc/system/stm32f405 -Isrc/targets/matekf405 -Isrc -Isrc -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/Include -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/Device/ST/STM32F4xx/Include -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/STM32F4xx_HAL_Driver/Inc -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/STM32F4xx_HAL_Driver/Src -I/home/hanfer/.platformio/packages/framework-stm32cubef4/Drivers/CMSIS/DSP/Include src/config/profile.c

which is curios.....with the nodes created just a couple lines prior you would think that should work.

[ivankravets]

Should it be the same env? Could you print it? Please use your PR.

def rewrite_source(node, localenv):
    print("%s=%s" % (id(env), id(localenv)))

[bkleiner]

yeah, ok, just ran it with the debugger, regrettably scons is just throwing the default env back at us.
https://github.com/SCons/scons/blob/4.4.0/SCons/Node/__init__.py#L1421

[ivankravets]

I don't know if this is a bug or a feature. I've just asked core developers, see SCons/scons#4211

Meanwhile, if we plan to merge this PR to the PlatformIO Core, I would recommend following SCons architecture and passing the env as the first argument. Could you swap arguments to (env, node)?

[bkleiner]

Should it be the same env? Could you print it? Please use our PR.

def rewrite_source(node, localenv):
    print("%s=%s" % (id(env), id(localenv)))

huh, it appears CollectBuildFiles is called multiple times. the first time around, from BuildSources the envs are indeed identical. however it is called a couple more times from ProcessProjectDeps with the env being modified.

Please use our PR.

what do mean by that?

[ivankravets]

Do you mean this SCons/scons#4211 is not a bug and I can close it?

[ivankravets]

Ok, I've just re-tested and reproduced this behavior in the SCons, see updated issue SCons/scons#4211

[bkleiner]

Meanwhile, if we plan to merge this PR to the PlatformIO Core, I would recommend following SCons architecture and passing the env as the first argument. Could you swap arguments to (env, node)?

sure, fixed.

Do you mean this SCons/scons#4211 is not a bug and I can close it?

no that observation was between the env imported in the pre script and the one passed as a parameter.
furthermore get_env and imported env seem to be identical on every invocation.
weather or not the behavior of get_env is intended or not, i don't know.

to be perfectly honest, i find scons architecture a little confusing, but i have yet to encounter a build tool that isn't confusing to a degree :D

[ivankravets]

to be perfectly honest, i find scons architecture a little confusing, but i have yet to encounter a build tool that isn't confusing to a degree :D

The project with over 20+years... Let's see how PlatformIO Core will look in the next 20+ years :)

[bkleiner]

Both projects are doing great work, that's for sure.
I am using pio for all my embedded projects. its ease of use and hackability are unparalleled.
Thanks for creating it.

[ivankravets]

Thanks for the PR! Merged and updated docs! https://docs.platformio.org/en/latest/scripting/middlewares.html