jedisct1 / libsodium

A modern, portable, easy to use crypto library.
https://libsodium.org
Other
12.15k stars 1.73k forks source link

Initial TrustInSoft CI configuration #1003

Closed guillaumemillot closed 3 years ago

guillaumemillot commented 3 years ago

This PR adds an initial TIS CI configuration to libsodium: https://ci.trust-in-soft.com/projects/guillaumemillot/libsodium/1

The configuration includes 56 tests from the /test/default folder and covers the single architecture x86-64 to get started. Note the 5 tests in red which raise an alarm due to address comparisons. We'll soon add an option to ignore it.

I'm asking you a few questions related to this configuration in issue https://github.com/jedisct1/libsodium/issues/995. I would appreciate your feedback!

About the files that are part of this PR:

Pending your review, I expect the PR to be mergeable as is. You are free to merge it on a test branch first or on master directly.

jedisct1 commented 3 years ago

Thanks a lot for this!

Having TIS integrated in our CI will be very valuable.

However, that configuration makes me freak out. It will have to be updated every time a file is added, moved, renamed, or deleted. Or even when something is modified and it doesn't match the expected TIS environment.

It's very likely that updating that configuration will constantly be forgotten.

It also requires a (modified?) copy of an automatically generated file.

These introduces additional burden, especially compared to other static analyzers that don't require such a configuration. The more of these burden we add, the more discouraging it is to work on the project.

We currently have a script (regen-msvc/regen-msvc.py) that creates the Visual Studio solutions according to the current files structure.

Could that script (or another one, with both that could be run with a single command) be used to generate the TIS configuration simultaneously?

Could the predefined macros be produced by a ./configure output (probably with a specific target) instead of being hardcoded?

jakub-zwolakowski commented 3 years ago

Hi again @jedisct1 !

Thanks for your answer. I definitely understand your concerns... This kind of feedback is actually very valuable for us, as we are figuring the question of making robust TIS CI configurations at this moment (until now we mostly deployed TIS CI on projects where very little change was expected).

So I thought about the issues you have mentioned and tried to improve the TIS CI configuration. I have adjusted the config files a bit (the analyses take a little longer now, but the configuration is definitely more robust) and I have written a python script which:

I hope this script will be good enough for you as a (partial) solution of these issues.

Unfortunately I don't have a full automatic solution for all the possible cases "when something is modified and it doesn't match the expected TIS environment" yet. What I can propose is that if any time soon your TIS CI config is broken, and running the regen-trustinsoft.py is not enough to repair it, we (i.e. TIS CI team) are going to step in and handle it. This will be a perfect opportunity for us to understand better what kind of changes we have to take into account in order to make the TIS CI configurations robust and / or what tools we need to develop in order to automatically adapt these configurations.

Is that approach OK for you?

I'll make a PR with the new (more robust) config and the regen-trustinsoft.py script shortly.