Adding version-conditional context string support

[baentsch]

This also acts as a test for the new liboqs version define feature by conditionally enabling context string support during signature operations for plain PQ algorithms supporting that feature and in openssl versions also supporting that feature.

[SWilson4]

LGTM! Thanks, Michael. I left a couple of picky style-related comments; feel free to ignore them if you wish.

[baentsch]

@SWilson4 Did you provide the tool "format_code.sh" running the docker image with the same clang-format version that CI uses? It now seems to fail: I just re-ran it and it confirms "no change" while CI does detect a change. Ideas what's wrong here?

[SWilson4]

@SWilson4 Did you provide the tool "format_code.sh" running the docker image with the same clang-format version that CI uses? It now seems to fail: I just re-ran it and it confirms "no change" while CI does detect a change. Ideas what's wrong here?

It looks to me like the coding style tests are passing in the Ubuntu 24 image (which is what that script uses). However, the failing tests also seem to be running clang-format in other images (Ubuntu 22 / Alpine) :(

[baentsch]

However, the failing tests also seem to be running clang-format in other images (Ubuntu 22 / Alpine) :(

Hmpf; OK, just saw

oqs-provider/.github/workflows/linux.yml

Lines 46 to 52 in 98ec7fc

	- name: alpine
	container: openquantumsafe/ci-alpine-amd64:latest
	# focal test done on CircleCI - save the compute cycles here until CCI is dropped
	# - name: focal
	# container: openquantumsafe/ci-ubuntu-focal-x86_64:latest
	- name: jammy
	container: openquantumsafe/ci-ubuntu-jammy:latest

That's of course an invitation to fail -- interesting that it worked until today :-/. Objections to removing these style checks from these platforms?

[SWilson4]

That's of course an invitation to fail -- interesting that it worked until today :-/. Objections to removing these style checks from these platforms?

None from me—especially since those tests only run if the style checks pass on Ubuntu 24.

[baentsch]

OK -- then what about putting these "nothing (in terms of style) changes on regenerate" tests also into the coding style check script? The re-generate and run tests on different platforms do make sense to retain (just no coding style check then there).

[SWilson4]

OK -- then what about putting these "nothing (in terms of style) changes on regenerate" tests also into the coding style check script? The re-generate and run tests on different platforms do make sense to retain (just no coding style check then there).

The purpose is to check idempotence under the operation of "regenerate and reformat", correct? (Similar to the copy_from_upstream check in liboqs.) If so, makes sense to me, and I agree that it makes sense to retain the regenerate -> run tests flow on different platforms.

I do wonder if there's value in also running the tests without regenerating on different platforms. Previously we had assurance that no code differences arose as part of the regeneration process, so it didn't matter. I'm thinking of the following scenario:

1. There's a bug specific to Alpine (say) present in the code as available in GitHub.
2. The bug is silently fixed by regenerating the code on Alpine. CI doesn't catch the change since it now ignores code changes after regeneration.
3. Since we only run tests after regeneration, the bug is never detected.

Does this seem far-fetched to you, or is it something we need to worry about?

[baentsch]

Does this seem far-fetched to you, or is it something we need to worry about?

No, it doesn't. My approach would have been to just remove the git diff test from where it is right now and add a "regenerate-and-test-diff" test to the style check job using the ubuntu-latest image.

[baentsch]

No, it doesn't. My approach would have been to just remove the git diff test from where it is right now and add a "regenerate-and-test-diff" test to the style check job using the ubuntu-latest image.

So now implemented -- addressing all your concerns @SWilson4 ?

[SWilson4]

So now implemented -- addressing all your concerns @SWilson4 ?

Yes, I'm happy. Thanks @baentsch!