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

chore: make firmware check fatal error by default #5749

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

pfeerick
Copy link
Member

@pfeerick pfeerick commented Jan 3, 2025

Summary of changes:

  • now that the version check is not breaking everything other firmware builds... the version check can be a fatal error unless -DUSE_UNSUPPORTED_TOOLCHAIN=Y is specified as a build option to force it to be skipped.

@pfeerick pfeerick added the compilation Related to compiling the firmware and firmware options label Jan 3, 2025
@pfeerick pfeerick requested a review from gagarinlg January 3, 2025 01:28
@pfeerick pfeerick added this to the 2.11 milestone Jan 3, 2025
@pfeerick pfeerick merged commit 424a0f1 into main Jan 13, 2025
49 checks passed
@pfeerick pfeerick deleted the pfeerick/test-compiler-version branch January 13, 2025 00:05
@wimalopaan
Copy link
Contributor

Buildind libsimulator and simulator not does not work anymore outside the docker image. The check for the ARM toolchain is still done and terminates the configure. I think this is useless for a host build.

Using the above option the configure complains about:

CMake Warning:
Manually-specified variables were not used by the project:

USE_UNSUPPORTED_TOOLCHAIN

This looks strange.

@pfeerick
Copy link
Member Author

pfeerick commented Jan 14, 2025

Try clearing your build folder - you are probably having issues with CMake caching. Since the check is running after a non-firmware configure would be terminated (see line 480), it should not be evaluated on a host build. And make sure you define it completely - -DUSE_UNSUPPORTED_TOOLCHAIN=Y - the =Y is not optional, and that message sounds a bit like theD for Define is missing.

@wimalopaan
Copy link
Contributor

I think you have to declare is as an option in the CMakeLists.txt like:

option(USE_UNSUPPORTED_TOOLCHAIN "Unsupported toolchain support" OFF)

@pfeerick
Copy link
Member Author

pfeerick commented Jan 15, 2025

Except it is not an "option". You can simply ignore the warning, as not all automatic warnings are important - better still it looks like I can do something somewhat similar just to make sure it is "used". But in this case, the warning is simply telling you that your configuration settings have caused that variable to not be used - as by configuring for a companion build you are skipping the block of code where it is used.

Keep in mind... the flag is ONLY needed if you are building firmware with an unsupported compiler version... it does not care about any of the other builds.

e.g. on MSYS/Windows - so not the docker container ;)

Peter@B560M-DS3H-AC MINGW64 ~/edgetx/build
$  cmake -G "MSYS Makefiles" -Wno-dev -DCMAKE_PREFIX_PATH=$HOME/qt/5.15.2/mingw81_64 -DSDL2_LIBRARY_PATH=/mingw64/bin/ -DPCB=X7 -DPCBREV=ZORRO ..
-- The C compiler identification is GNU 14.2.0
-- The CXX compiler identification is GNU 14.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/mingw64/bin/cc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/msys64/mingw64/bin/c++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- CMAKE_ARGS: -DCMAKE_PREFIX_PATH=C:/msys64/home/Peter/qt/5.15.2/mingw81_64;-DPCB=X7;-DPCBREV=ZORRO;-DSDL2_LIBRARY_PATH=C:/msys64/mingw64/bin/
-- CMAKE_BUILD_TYPE:
-- Configuring done (1.8s)
-- Generating done (0.1s)
-- Build files have been written to: C:/msys64/home/Peter/edgetx/build

Note the host C++ compiler version is 14.2.0, thus technically "unsupported" ... but as you will see on the configure stage...

Peter@B560M-DS3H-AC MINGW64 ~/edgetx/build
$ make configure
[ 10%] Creating directories for 'arm-none-eabi'
[ 20%] No download step for 'arm-none-eabi'
[ 30%] No update step for 'arm-none-eabi'
[ 40%] No patch step for 'arm-none-eabi'
[ 50%] Performing configure step for 'arm-none-eabi'
loading initial cache file C:/msys64/home/Peter/edgetx/build/arm-none-eabi-prefix/tmp/arm-none-eabi-cache-.cmake
-- The C compiler identification is GNU 14.2.1
-- The CXX compiler identification is GNU 14.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/opt/gcc-arm-none-eabi/bin/arm-none-eabi-gcc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/msys64/opt/gcc-arm-none-eabi/bin/arm-none-eabi-g++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- The ASM compiler identification is GNU
-- Found assembler: C:/msys64/opt/gcc-arm-none-eabi/bin/arm-none-eabi-gcc.exe
-- Found Git: C:/msys64/usr/bin/git.exe (found version "2.47.1")
-- Found PythonInterp: C:/msys64/mingw64/bin/python3.exe (found suitable version "3.12.7", minimum required is "3")
-- Python found, version: 3.12.7
-- EdgeTX 2.11.0-selfbuild @ 424a0f18
CPU_TYPE_FULL = STM32F407xE
-- Adding support for USB serial
-- Adding support for MULTI as internal module
-- Adding support for CRSF as internal module
-- Use LTO to compile bootloader
TARGET bootloader: cpp compiler C:/msys64/opt/gcc-arm-none-eabi/bin/arm-none-eabi-g++.exe v14.2.1
-- Bootloader contained in firmware binary
TARGET firmware: cpp compiler C:/msys64/opt/gcc-arm-none-eabi/bin/arm-none-eabi-g++.exe v14.2.1
-- Configuring done (1.7s)
-- Generating done (0.4s)
-- Build files have been written to: C:/msys64/home/Peter/edgetx/build/arm-none-eabi
[ 50%] Built target arm-none-eabi-configure
[ 60%] Creating directories for 'native'
[ 70%] No download step for 'native'
[ 80%] No update step for 'native'
[ 90%] No patch step for 'native'
[100%] Performing configure step for 'native'
loading initial cache file C:/msys64/home/Peter/edgetx/build/native-prefix/tmp/native-cache-.cmake
-- The C compiler identification is GNU 14.2.0
-- The CXX compiler identification is GNU 14.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/msys64/mingw64/bin/cc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/msys64/mingw64/bin/c++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- The ASM compiler identification is GNU
-- Found assembler: C:/msys64/mingw64/bin/cc.exe
-- Found Git: C:/msys64/usr/bin/git.exe (found version "2.47.1")
-- Found PythonInterp: C:/msys64/mingw64/bin/python3.exe (found suitable version "3.12.7", minimum required is "3")
-- Python found, version: 3.12.7
-- EdgeTX 2.11.0-selfbuild @ 424a0f18
Libfox not found, simu target will not be available
-- Qt Version: 5.15.2
-- SDL2 Lib: C:/msys64/mingw64/bin/SDL2.dll Libs: SDL2::SDL2main;SDL2::SDL2; Headers: C:/msys64/mingw64/include;C:/msys64/mingw64/include/SDL2
-- Found LIBUSB1: C:/msys64/mingw64/lib
-- Found DFU_UTIL: C:/msys64/mingw64/bin/dfu-util.exe
-- Found OpenSSL: C:/msys64/mingw64/lib/libcrypto.dll.a (found version "3.4.0")
-- Found Python3: C:/Users/Peter/AppData/Local/Programs/Python/Python312/python.exe (found version "3.12.7") found components: Interpreter
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
Fetched miniz source code from Github: C:/msys64/home/Peter/edgetx/build/native/_deps/miniz-src
Fetched yaml-cpp source code from Github: C:/msys64/home/Peter/edgetx/build/native/_deps/yaml-cpp-src
-- Qt Version: 5.15.2
-- Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE)
Fetched maxLibQt source code from Github: C:/msys64/home/Peter/edgetx/build/native/_deps/maxlibqt-src/src
TARGET companion: cpp compiler C:/msys64/mingw64/bin/c++.exe v14.2.0
-- Added optional gtests-companion target
-- Install to C:/msys64/home/Peter/edgetx/build/native/Release
-- windeployqt command: C:/msys64/home/Peter/qt/5.15.2/mingw81_64/bin/windeployqt.exe --no-translations --no-opengl-sw --no-system-d3d-compiler --no-angle --debug -dir "C:/msys64/home/Peter/edgetx/build/native/Release" C:/msys64/home/Peter/edgetx/build/native/companion.exe C:/msys64/home/Peter/edgetx/build/native/simulator.exe
CPU_TYPE_FULL = STM32F407xE
-- Adding support for USB serial
-- Adding support for MULTI as internal module
-- Adding support for CRSF as internal module
TARGET simu/libsimulator: cpp compiler C:/msys64/mingw64/bin/c++.exe v14.2.0
-- Added optional gtests target
-- firmware target disabled
-- Configuring done (28.3s)
-- Generating done (1.8s)
-- Build files have been written to: C:/msys64/home/Peter/edgetx/build/native
[100%] Built target native-configure
[100%] Built target configure

... the version used for the firmware (arm-none-eabi) is 14.2.1 (supported) ... and again the version that will be used by native is 14.2.0 ... so I didn't need to pass the USE_UNSUPPORTED_TOOLCHAIN override flag as I am using a supported version of the firmware compiler. If I did, I would correctly get the warning that the variable isn't used...

pfeerick added a commit that referenced this pull request Jan 15, 2025
@wimalopaan
Copy link
Contributor

wimalopaan commented Jan 15, 2025

My point was that the configure step is aborted even if you don't build the firmware, only native. I think the check against the ARM toolchain should be done later in the make step, not in the configure step.

@pfeerick
Copy link
Member Author

pfeerick commented Jan 15, 2025

Clearly there is something more to this, as this is not what I am seeing on either Linux or Windows/MSYS (with valid firmware compiler version and invalid host compiler version), not does it make any sense given it is after when the native_build check should abort the check.

Have you started from a new/empty build folder (eliminating cached configuration settings)? What is your build environment, toolchain, what exact steps to reproduce? i.e. some logs please! 😅😂

@wimalopaan
Copy link
Contributor

I do not want to build the firmware, only native simulator. I go thru the usual steps incl. make configure (which is wrong, see below):

bash-5.2$ pwd
/home/lmeier/Projekte/edgetx/edgetx.wmgit/build_tx16s
bash-5.2$ rm -rf
bash-5.2$ ls -la
insgesamt 8
drwxr-xr-x  2 lmeier users 4096 15. Jan 11:40 .
drwxr-xr-x 10 lmeier users 4096 15. Jan 06:30 ..
bash-5.2$ arm-none-eabi-gcc --version
arm-none-eabi-gcc (Arch Repository) 14.2.0
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

bash-5.2$ gcc --version
gcc (GCC) 15.0.0 20240430 (experimental)
Copyright (C) 2024 Free Software Foundation, Inc.
Dies ist freie Software; die Kopierbedingungen stehen in den Quellen. Es
gibt KEINE Garantie; auch nicht für MARKTGÄNGIGKEIT oder FÜR SPEZIELLE ZWECKE.

bash-5.2$ cmake -DDEBUG=YES -DTRANSLATIONS=DE -DPCB=X10 -DPCBREV=TX16S -DLUA=YES -DLUA_MIXER=YES -DGVARS=YES ..
-- The C compiler identification is GNU 14.2.1
-- The CXX compiler identification is GNU 14.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- CMAKE_ARGS: -DDEBUG=YES;-DGVARS=YES;-DLUA=YES;-DLUA_MIXER=YES;-DPCB=X10;-DPCBREV=TX16S;-DTRANSLATIONS=DE
-- CMAKE_BUILD_TYPE:
-- Configuring done (0.5s)
-- Generating done (0.0s)
-- Build files have been written to: /home/lmeier/Projekte/edgetx/edgetx.wmgit/build_tx16s
bash-5.2$ make configure
[ 10%] Creating directories for 'arm-none-eabi'
[ 20%] No download step for 'arm-none-eabi'
[ 30%] No update step for 'arm-none-eabi'
[ 40%] No patch step for 'arm-none-eabi'
[ 50%] Performing configure step for 'arm-none-eabi'
loading initial cache file /home/lmeier/Projekte/edgetx/edgetx.wmgit/build_tx16s/arm-none-eabi-prefix/tmp/arm-none-eabi-cache-.cmake
-- The C compiler identification is GNU 14.2.0
-- The CXX compiler identification is GNU 14.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/arm-none-eabi-gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/arm-none-eabi-g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/arm-none-eabi-gcc
-- Found Git: /usr/bin/git (found version "2.48.1")
-- Found PythonInterp: /usr/bin/python3 (found suitable version "3.13.1", minimum required is "3")
-- Python found, version: 3.13.1
-- EdgeTX 2.11.0-selfbuild @ 8812c365
-- Internal GPS enabled
-- Adding support for USB serial
-- Adding support for PXX1 as internal module
-- Adding support for PXX2 as internal module
-- Adding support for MULTI as internal module
-- Adding support for CRSF as internal module
CPP Compiler: /usr/bin/arm-none-eabi-g++, version 14.2.0
CMake Error at radio/src/CMakeLists.txt:485 (message):
Only Arm GNU Toolchain version 14.2.rel1 is supported!!


-- Configuring incomplete, errors occurred!
make[3]: *** [CMakeFiles/arm-none-eabi-configure.dir/build.make:75: arm-none-eabi-prefix/src/arm-none-eabi-stamp/arm-none-eabi-configure] Fehler 1
make[2]: *** [CMakeFiles/Makefile2:196: CMakeFiles/arm-none-eabi-configure.dir/all] Fehler 2
make[1]: *** [CMakeFiles/Makefile2:236: CMakeFiles/configure.dir/rule] Fehler 2
make: *** [Makefile:176: configure] Fehler 2
bash-5.2$``

I shouldn't do a configure , instead I should directly make libsimulator simulator, as this steps does only a configure for native. Makes sense ...

Please excuse me for the misunderstanding ... (I should have read the logs more carefully).
Thanks!

@pfeerick
Copy link
Member Author

pfeerick commented Jan 15, 2025

Ok, I see what is going on now... thanks for that.

So when you have the wrong firmware compiler version, but aren't actually interested in compiling the firmware, rather than running make configure, which effectively configures for all toolchains... i.e. both arm-none-eabi and native toolchains... you instead want

make native-configure

so that you configure 'only' for the native toolchain. Otherwise, you would need the -DUSE_UNSUPPORTED_TOOLCHAIN=Y flag to compensate for the fact you're telling it to configure for both firmware and native builds, at which point the check will be run.

There are also scoped target commands you can run for building firmware, simulator, but in practise I've not needed to run them... c.f. #1736

e.g.

make -C native -j`nproc` libsimulator simulator companion

@wimalopaan
Copy link
Contributor

Thanks for pointing me to #1736 : that was the info I was looking for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compilation Related to compiling the firmware and firmware options
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants