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

IKOS files are not generated in static code analysis #104

Open
xfiderek opened this issue Jan 10, 2024 · 5 comments
Open

IKOS files are not generated in static code analysis #104

xfiderek opened this issue Jan 10, 2024 · 5 comments

Comments

@xfiderek
Copy link
Contributor

xfiderek commented Jan 10, 2024

Summary

+build-testing procedure from spaceros Earthfile invoked in CI should produce summary of tests and static code analysis. There are no ikos.sarif files present in CI artifact. Check the latest artifact saved as part of CI.

To investigate the issue i took a look at logs from robot_state_publisher test run. The logs (after slight modifications) are available in ~/spaceros/build/robot_state_publisher/ament_ikos/ikos.txt file in docker image created by earthly:

-- run_test.py: invoking following command in '/home/spaceros-user/spaceros/build/robot_state_publisher':
 - /home/spaceros-user/spaceros/install/ament_ikos/bin/ament_ikos --xunit-file /home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/ikos.xunit.xml --sarif-file /home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/ikos.sarif /home/spaceros-user/spaceros/build/robot_state_publisher

No marker files found when scanning /home/spaceros-user/spaceros/build/robot_state_publisher

-- run_test.py: return code 0
-- run_test.py: generate result file '/home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/ikos.xunit.xml' with failed test
-- run_test.py: verify result file '/home/spaceros-user/spaceros/build/robot_state_publisher/test_results/robot_state_publisher/ikos.xunit.xml'

I don't have much experience with ikos but from short investigation it seems that ament_ikos is looking for .ikosbin files, which to my understanding should be created during package compilation.

Apart from that I noticed that ikos is not even installed in the docker image. I think we should fix that by adding ikos_vendor package to ament_ikos repo, the same way it is done for cobra in ament_cobra repo.

Additionally ikos is specified in ament_ikos as dependency in package.xml:

https://github.com/ament/ament_ikos/blob/b7663666ef934c51126c5fb496d1eb3099fddbc9/ament_ikos/package.xml#L10

Rosdep is not able to install ikos in this way and it throws error on my machine.

rosdep resolve ikos
ERROR: no rosdep rule for 'ikos'

I believe that is the reason why we are adding ikos to --skip-keys in our Earthfile - otherwise build would simply fail.

RUN rosdep install --from-paths src --ignore-src --rosdistro rolling -y --skip-keys "console_bridge fastcdr fastrtps rti-connext-dds-5.3.1 urdfdom_headers rmw_connextdds ros_testing rmw_connextdds rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp composition demo_nodes_py lifecycle rosidl_typesupport_fastrtps_cpp rosidl_typesupport_fastrtps_c ikos"

@ivanperez-keera
Copy link
Contributor

Also related #99 .

@ivanperez-keera
Copy link
Contributor

Thanks for this investigation. I'm going to continue the discussion on why IKOS is not installed in #99. After we get it in the docker image, we can discuss if the results are being generated.

@xfiderek
Copy link
Contributor Author

Btw the section in README that describes how to run IKOS analysis mentions that executing it on all packages takes "very long time". Maybe we should record this time and decide if we want to run IKOS as part of CI at all / run it on different cadence (e.g. monthly).

@ivanperez-keera
Copy link
Contributor

I propose a different approach.

IKOS can be configured to tune both the speed and the tests (which affect the potential for false positives).

I agree with measuring how long it takes, but, instead of disabling it, lets figure out a set of settings that is fast enough but provides something useful in terms of assurance.

@Bckempa
Copy link
Contributor

Bckempa commented Jan 15, 2024

Maybe we should record this time and decide if we want to run IKOS as part of CI at all / run it on different cadence (e.g. monthly).

Good idea, especially since you've expressed interesting in focusing down static analysis infrastructure for the April release. I've opened a discussion: space-ros/space-ros#128

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

No branches or pull requests

3 participants