search

platformio / platformio-core

Your Gateway to Embedded Software Development Excellence :alien:
https://platformio.org
Apache License 2.0
7.95k stars 792 forks source link

pio-test: global library extra_script.py is executed with an empty project #3915

Open mcspr opened 3 years ago

mcspr commented 3 years ago

Notes

Fixing this issue breaks "mbed" framework. See https://github.com/platformio/platformio-core/runs/5156160446?check_suite_focus=true

Configuration

Operating system: Fedora 33

PlatformIO Version (platformio --version):

PlatformIO Core             5.2.0a3
Python                      3.9.1-final.0
System Type                 linux_x86_64

Description of problem

Globally installed library extra_script.py is executed when running pio-test:

> pio test -e test
Verbose mode can be enabled via `-v, --verbose` option
Collected 1 items

Processing * in test environment
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Building...
TypeError: join() argument must be str, bytes, or os.PathLike object, not 'NoneType':
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/main.py", line 178:
    env.SConscript("$BUILD_SCRIPT")
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Script/SConscript.py", line 591:
    return _SConscript(self.fs, *files, **subst_kw)
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Script/SConscript.py", line 280:
    exec(compile(scriptdata, scriptname, 'exec'), call_stack[-1].globals)
  File "/home/maxim/.platformio/platforms/native/builder/main.py", line 43:
    target_bin = env.BuildProgram()
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Util.py", line 658:
    return self.method(*nargs, **kwargs)
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/tools/platformio.py", line 62:
    env.ProcessProjectDeps()
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Util.py", line 658:
    return self.method(*nargs, **kwargs)
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/tools/platformio.py", line 140:
    project_lib_builder = env.ConfigureProjectLibBuilder()
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Util.py", line 658:
    return self.method(*nargs, **kwargs)
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/tools/piolib.py", line 1064:
    lib_builders = env.GetLibBuilders()
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Util.py", line 658:
    return self.method(*nargs, **kwargs)
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/tools/piolib.py", line 993:
    lb = LibBuilderFactory.new(env, lib_dir)
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/tools/piolib.py", line 61:
    obj = getattr(sys.modules[__name__], clsname)(env, path, verbose=verbose)
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/tools/piolib.py", line 137:
    self.process_extra_options()
  File "/home/maxim/.platformio/penv/lib/python3.9/site-packages/platformio/builder/tools/piolib.py", line 280:
    self.env.SConscript(
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Script/SConscript.py", line 591:
    return _SConscript(self.fs, *files, **subst_kw)
  File "/home/maxim/.platformio/packages/tool-scons/scons-local-4.1.0/SCons/Script/SConscript.py", line 280:
    exec(compile(scriptdata, scriptname, 'exec'), call_stack[-1].globals)
  File "/home/maxim/.platformio/lib/BSEC Software Library/extra_script.py", line 5:
    LIBPATH=[realpath(join('src', env.get('BOARD_MCU')))],
  File "/usr/lib64/python3.9/posixpath.py", line 90:
    genericpath._check_arg_types('join', a, *p)
  File "/usr/lib64/python3.9/genericpath.py", line 152:
    raise TypeError(f'{funcname}() argument must be str, bytes, or '
======================================================================== [FAILED] Took 0.33 seconds ========================================================================

Test    Environment    Status    Duration
------  -------------  --------  ------------
*       test           FAILED    00:00:00.329
================================================================== 1 failed, 0 succeeded in 00:00:00.329 ==================================================================

Steps to Reproduce

  1. Make sure ~/.platformio/lib is empty before the test
  2. pio lib -g install "https://github.com/BoschSensortec/BSEC-Arduino-library.git#v1.6.1480"
  3. Create a dummy project project in some directory
  4. pio test -e test triggers an exception (see above)

If problems with PlatformIO Build System:

The content of platformio.ini:

[env:test]
platform = native

Additional info

As seen in the extra_script of the lib https://github.com/BoschSensortec/BSEC-Arduino-library/blob/7a9357566c75048331c15b55a4eb20f1de14f36e/extra_script.py

Import('env')
from os.path import join, realpath

env.Append(
    LIBPATH=[realpath(join('src', env.get('BOARD_MCU')))],
    LIBS=['algobsec']
)

As seen in the trace:

File "/home/maxim/.platformio/lib/BSEC Software Library/extra_script.py", line 5:
    LIBPATH=[realpath(join('src', env.get('BOARD_MCU')))],

It tries to use BOARD_MCU, which is not available with the platform. But, is it an expected behaviour with the global libraries that scripts are executed unconditionally?

mcspr commented 3 years ago

Small correction - it also affects normal pio-run and it happens whenever BSEC lib is either in the lib/ of the project or the ~/.platformio/lib (or any other lib using extraScript: ... in the library.json which will happen to break with native platform)

Looking at the docs: https://docs.platformio.org/en/latest/librarymanager/config.html#extrascript

Launch extra script before build process

Shouldn't it wait until the lib is picked by builder to call the script? e.g.

diff --git a/platformio/builder/tools/piolib.py b/platformio/builder/tools/piolib.py
index 3849a91b..8e91cc08 100644
--- a/platformio/builder/tools/piolib.py
+++ b/platformio/builder/tools/piolib.py
@@ -273,15 +273,8 @@ class LibBuilderBase(object):
         return {}

     def process_extra_options(self):
-        with fs.cd(self.path):
-            self.env.ProcessFlags(self.build_flags)
-            if self.extra_script:
-                self.env.SConscriptChdir(1)
-                self.env.SConscript(
-                    os.path.realpath(self.extra_script),
-                    exports={"env": self.env, "pio_lib_builder": self},
-                )
-            self.env.ProcessUnFlags(self.build_unflags)
+        self.env.ProcessFlags(self.build_flags)
+        self.env.ProcessUnFlags(self.build_unflags)

     def process_dependencies(self):
         if not self.dependencies:
@@ -436,6 +429,14 @@ class LibBuilderBase(object):
             self.depend_recursive(lb, lb_search_files)

     def build(self):
+        if self.extra_script:
+            with fs.cd(self.path):
+                self.env.SConscriptChdir(1)
+                self.env.SConscript(
+                    os.path.realpath(self.extra_script),
+                    exports={"env": self.env, "pio_lib_builder": self},
+                )
+
         libs = []
         for lb in self._depbuilders:
             libs.extend(lb.build())

(edit: process flags as it is currently, run extra script in the build())

ivankravets commented 3 years ago

But, is it an expected behaviour with the global libraries that scripts are executed unconditionally?

Yes, when we look for dependencies we analyze all libraries according to the storage list. Extra scripts are called on each init of a library because "extra script" can change a lot of things in runtime, such as CPPPATH, etc which we need BEFORE deciding should this lirbary be included in a build process.

ivankravets commented 3 years ago

I forgot you can use lib_ignore to skip some libs.

mcspr commented 3 years ago

I forgot you can use lib_ignore to skip some libs.

Hm. This does not work for the case above :/ With the modified .ini:

> pio run -v -e test
Processing test (platform: native; lib_ignore: BSEC Software Library)
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
LDF: Library Dependency Finder -> http://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
CALLED
TypeError: join() argument must be str, bytes, or os.PathLike object, not 'NoneType':
...etc...
  File "/home/maxim/.platformio/lib/BSEC Software Library/extra_script.py", line 7:
    LIBPATH=[realpath(join('src', env.get('BOARD_MCU')))],
  File "/usr/lib64/python3.9/posixpath.py", line 90:
    genericpath._check_arg_types('join', a, *p)
  File "/usr/lib64/python3.9/genericpath.py", line 152:
    raise TypeError(f'{funcname}() argument must be str, bytes, or '
ivankravets commented 3 years ago

I see. Need to think about if it is possible to fix it with minimal effort.

mcspr commented 3 years ago

Yes, when we look for dependencies we analyze all libraries according to the storage list. Extra scripts are called on each init of a library because "extra script" can change a lot of things in runtime, such as CPPPATH, etc which we need BEFORE deciding should this lirbary be included in a build process.

Looking at this once again, would this be a valid fix? afaik we can't make the lib compatible through the extra script if platform is not compatible to begin with? (edit: or avoid creating the libbuilder object to begin with, but I got lost in the libbuilder factory)

diff --git a/platformio/builder/tools/piolib.py b/platformio/builder/tools/piolib.py
index 3849a91b..aae76f14 100644
--- a/platformio/builder/tools/piolib.py
+++ b/platformio/builder/tools/piolib.py
@@ -134,7 +134,8 @@ class LibBuilderBase(object):
         self.env["SRC_FILTER"] = ""

         # process extra options and append to build environment
-        self.process_extra_options()
+        if env.IsCompatibleLibBuilder(self):
+            self.process_extra_options()

     def __repr__(self):
         return "%s(%r)" % (self.__class__, self.path)

In case of BSEC library... it seems to need a correct (?) "platforms" entry in the library.json to denote that it only supports MCU platforms and not the native one, as right now it is just a glob: https://github.com/BoschSensortec/BSEC-Arduino-library/blob/7a9357566c75048331c15b55a4eb20f1de14f36e/library.json#L15

mmmspatz commented 1 year ago

I got around this by adding this at the top of my extra_script.py (edit: Exit() -> Return())

Import("env")
Import("pio_lib_builder")

if not env.IsCompatibleLibBuilder(pio_lib_builder):
    Return()

IMO https://github.com/platformio/platformio-core/issues/3915#issuecomment-851050357 is a good solution. The concept of library compatibility should extend to the compatibility of a library's extra_script.py