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

Not included dependency for unit tests #1886

Closed
nuschpl opened this issue Feb 27, 2024 · 7 comments · Fixed by #1892
Closed

Not included dependency for unit tests #1886

nuschpl opened this issue Feb 27, 2024 · 7 comments · Fixed by #1892
Assignees

Comments

@nuschpl
Copy link

nuschpl commented Feb 27, 2024

The IronOS referes to usb-pd which in turn refers to CppUTest, neither the git submodule or Makefiles provide way to find those files resulting in an error when compiling on MacOS:

 %  make firmware-EN model=TS100
make  -C source/  firmware-EN
make[1]: Entering directory '/Users/User/src/Iron/IronOS/source'
Building for Miniware
Building file: ./Core/BSP/Miniware//Startup/startup_stm32f103t8ux.S
Core/Drivers//usb-pd/tests/test_pd_policy_engine.cpp:1:10: fatal error: CppUTest/TestHarness.h: No such file or directory
    1 | #include "CppUTest/TestHarness.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:534: Objects/TS100/./Core/Drivers//usb-pd/tests/test_pd_policy_engine.o] Error 1
make[1]: Leaving directory '/Users/USER/src/Iron/IronOS/source'
make: *** [Makefile:228: firmware-EN] Error 2
@Ralim
Copy link
Owner

Ralim commented Feb 27, 2024

The test files are not intended to be compiled as part of IronOS and normally excluded. They are tested and used in the usb-pd repository.

Nominal build process for TS100 in English would be as follows, this is what is tested in every CI run.

make docker-shell
cd source
make model=TS100

@nuschpl
Copy link
Author

nuschpl commented Feb 27, 2024

I'm doing the same but without docker- just issuing:
make firmware-EN model=TS100 as per guide printed by make itself.
Being enforced to install battery hungry proprietary Docker software when the aim is to compile other software from source(iron FW itself) seems an overkill for me.
Here is the patch which made it compiling fine:

% git diff
diff --git a/source/Makefile b/source/Makefile
index f0b0f9f1..94027bcd 100644
--- a/source/Makefile
+++ b/source/Makefile
@@ -66,7 +66,7 @@ BRIEFLZ_DIR=./Core/brieflz/
 DRIVERS_DIR=./Core/Drivers/
 PD_DRIVER_DIR=./Core/Drivers/usb-pd
 # Exclude USB-PD tests
-PD_DRIVER_TESTS_DIR=./Core/Drivers/usb-pd/tests
+PD_DRIVER_TESTS_DIR=./Core/Drivers//usb-pd/tests

The slash char seems to be the reson -the path which was built contains double slash which causes it no longer match the exclusion rule in source/Makefile

The proper solution would be probably to globaly resolve the paths to reduce extre '/' or things like '././' . Eg in python this can be done with os.path.join() instead of manual concatenation of strings.

I would propose something specific but propably it would be easier for someone already familiar with current built system or IronOS.

@Ralim
Copy link
Owner

Ralim commented Feb 27, 2024

Docker/podman (both can be used) is the only supported build setup at this time to ensure that the same tooling and software versions are used by all.

For example some tools act subtly different on Apple or Windows, and as I dont have access to those to test, I cant support them. But if everyone uses docker/podman then it levels the playing field to ensure everyone is working in the same setup.

@ia
Copy link
Collaborator

ia commented Mar 15, 2024

Hello.

TL; DR: after reading report, provided diff, related Makefile and recalling that OSX (like any other BSD from userland point of view) uses BSD versions of userland tools not GNU ones, it may be BSD find behavior leading to this issue. Probably it has something to do with adding/keeping/removing trailing /'s for find.

Now since I'm nerd-sniped by this, I will try to figure this out in a little bit.

Just a little rant first:

Being enforced to install battery hungry proprietary Docker software when the aim is to compile other software from source(iron FW itself) seems an overkill for me.

I fully understand your point (and even frustration). However IronOS quite unique project having a lot of moving parts and depends on various tools so it can be built. Therefore, like @Ralim underscored, to eliminate the factor of too old/too fresh/too buggy version of Bash/Python/GCC C++/GCC RISCV/etc. and to let contributors to concentrate on their contributions, docker is used as the main reference devkit. But in general in fact I agree with you that as long as project & its required toolchain with tools is set according to provided documentation, it should be compile-able at least (I have one GNULinux box myself configured to build IronOS without docker using host tools only). Just keep in mind that your results may drastically vary. Just as example: if you use OSX with GCC 4.2 (as far as I know this is the last GCC they ship if they even still do that instead of full transition to clang) keep in mind that this is pretty old version which may have poor compile optimization specifically from the point of the firmware final size. So in cases like this you're on your own. And once you have any bug - better try to reproduce it using firmware compiled with "officially supported way" first before placing a bug report to eliminate "custom toolchain" factor. Thanks for understanding.

@ia
Copy link
Collaborator

ia commented Mar 15, 2024

So, I had to dust one BSD host nearby off:

  • BSD find with trailing / in argument:
source $ find ./Core/Drivers/ -type f -name '*.cpp' -not -path "./Core/Drivers/usb-pd/tests/*"
./Core/Drivers//BMA223.cpp
./Core/Drivers//BootLogo.cpp
./Core/Drivers//Buttons.cpp
./Core/Drivers//FS2711.cpp
./Core/Drivers//HUB238.cpp
./Core/Drivers//I2CBB1.cpp
./Core/Drivers//I2CBB2.cpp
./Core/Drivers//LIS2DH12.cpp
./Core/Drivers//MMA8652FC.cpp
./Core/Drivers//MSA301.cpp
./Core/Drivers//OLED.cpp
./Core/Drivers//SC7A20.cpp
./Core/Drivers//Si7210.cpp
./Core/Drivers//TipThermoModel.cpp
./Core/Drivers//USBPD.cpp
./Core/Drivers//Utils.cpp
./Core/Drivers//usb-pd/src/fusb302b.cpp
./Core/Drivers//usb-pd/src/policy_engine.cpp
./Core/Drivers//usb-pd/src/policy_engine_states.cpp
./Core/Drivers//usb-pd/tests/main.cpp
./Core/Drivers//usb-pd/tests/mock_fusb302.cpp
./Core/Drivers//usb-pd/tests/test_fusb302.cpp
./Core/Drivers//usb-pd/tests/test_pd_policy_engine.cpp
./Core/Drivers//usb-pd/tests/test_ringbuffer.cpp
./Core/Drivers//usb-pd/tests/user_functions.cpp
  • BSD find without trailing / in argument:
source $ find ./Core/Drivers -type f -name '*.cpp' -not -path "./Core/Drivers/usb-pd/tests/*"
./Core/Drivers/BMA223.cpp
./Core/Drivers/BootLogo.cpp
./Core/Drivers/Buttons.cpp
./Core/Drivers/FS2711.cpp
./Core/Drivers/HUB238.cpp
./Core/Drivers/I2CBB1.cpp
./Core/Drivers/I2CBB2.cpp
./Core/Drivers/LIS2DH12.cpp
./Core/Drivers/MMA8652FC.cpp
./Core/Drivers/MSA301.cpp
./Core/Drivers/OLED.cpp
./Core/Drivers/SC7A20.cpp
./Core/Drivers/Si7210.cpp
./Core/Drivers/TipThermoModel.cpp
./Core/Drivers/USBPD.cpp
./Core/Drivers/Utils.cpp
./Core/Drivers/usb-pd/src/fusb302b.cpp
./Core/Drivers/usb-pd/src/policy_engine.cpp
./Core/Drivers/usb-pd/src/policy_engine_states.cpp

And here is

  • GNU find with trailing / in argument:
source$ find ./Core/Drivers/ -type f -name '*.cpp' -not -path "./Core/Drivers/usb-pd/tests/*"
./Core/Drivers/TipThermoModel.cpp
./Core/Drivers/USBPD.cpp
./Core/Drivers/LIS2DH12.cpp
./Core/Drivers/MMA8652FC.cpp
./Core/Drivers/I2CBB2.cpp
./Core/Drivers/Utils.cpp
./Core/Drivers/FS2711.cpp
./Core/Drivers/Buttons.cpp
./Core/Drivers/BMA223.cpp
./Core/Drivers/BootLogo.cpp
./Core/Drivers/Si7210.cpp
./Core/Drivers/HUB238.cpp
./Core/Drivers/I2CBB1.cpp
./Core/Drivers/usb-pd/src/fusb302b.cpp
./Core/Drivers/usb-pd/src/policy_engine_states.cpp
./Core/Drivers/usb-pd/src/policy_engine.cpp
./Core/Drivers/SC7A20.cpp
./Core/Drivers/OLED.cpp
./Core/Drivers/MSA301.cpp
  • GNU find without trailing / in argument:
source$ find ./Core/Drivers -type f -name '*.cpp' -not -path "./Core/Drivers/usb-pd/tests/*"
./Core/Drivers/TipThermoModel.cpp
./Core/Drivers/USBPD.cpp
./Core/Drivers/LIS2DH12.cpp
./Core/Drivers/MMA8652FC.cpp
./Core/Drivers/I2CBB2.cpp
./Core/Drivers/Utils.cpp
./Core/Drivers/FS2711.cpp
./Core/Drivers/Buttons.cpp
./Core/Drivers/BMA223.cpp
./Core/Drivers/BootLogo.cpp
./Core/Drivers/Si7210.cpp
./Core/Drivers/HUB238.cpp
./Core/Drivers/I2CBB1.cpp
./Core/Drivers/usb-pd/src/fusb302b.cpp
./Core/Drivers/usb-pd/src/policy_engine_states.cpp
./Core/Drivers/usb-pd/src/policy_engine.cpp
./Core/Drivers/SC7A20.cpp
./Core/Drivers/OLED.cpp
./Core/Drivers/MSA301.cpp

ia added a commit to ia/IronOSf that referenced this issue Mar 15, 2024
ia added a commit to ia/IronOSf that referenced this issue Mar 15, 2024
@ia
Copy link
Collaborator

ia commented Mar 15, 2024

So, could you, @nuschpl, please, [when you have time] to pull out these changes [#1892] locally to test & verify clean fresh build to make sure that now it works as it should? Thanks. I'm just really curious to find out myself if it works (but I don't have OSX with IronOS env up & running to test myself). At least it doesn't break gnulinux builds.

Ralim pushed a commit that referenced this issue Mar 16, 2024
* source/Makefile compatibility with BSD find [#1886]

* Align formatting

* source/Makefile: remove trailing /s from DIR vars to fix build using BSD find/OSX env [#1886]
@nuschpl
Copy link
Author

nuschpl commented Mar 16, 2024

@ia Thank you for investigation, it works now out of the box with current HEAD I assume its because of above patch.
The gcc points to clang FYI(MacOs Sonomoa 14.3.1, Apple Silicon M2) :

 gcc -v
Apple clang version 15.0.0 (clang-1500.3.9.4)
Target: arm64-apple-darwin23.3.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants