botan icon indicating copy to clipboard operation
botan copied to clipboard

Build examples from documentation in CI

Open lieser opened this issue 3 years ago • 4 comments

Extract the examples in the documentation into separate files and add a build target for them. This allows us to build them in the CI and gives us at least a minimal insurance that they actually work.

A limitation of the current approach is that all examples have to be standalone executable with a main.

TODOs:

  • [x] Add new code structure to documentation.
  • [ ] Extract examples in doc/api_ref/pkcs11.rst.
    • Examples would need some changes, as the current ones to not have any includes or main.

Open Questions:

  • How to properly model the requirements of the examples? Currently the examples target would simply fail if Botan is not configured with all required modules and boost enabled.
    • Solution 1:
      • Do the same as in tests with using #if defined(BOTAN_HAS_NUMBERTHEORY) and similar things
      • We could use start-after / end-before in the literalinclude directive to avoid it being shown in the documentation
      • Would still look ugly if looking directly at the source file
      • We would create "empty" example executable
    • Are there better but still simple solutions?
    • Is this even a concern for us? E.g. for the CI this could be a desired behavior, as no example is silently excluded from it
  • Also run examples? Would of course be nice, but I would leave that for later.

lieser avatar Aug 02 '22 15:08 lieser

Codecov Report

Merging #3025 (c55cd1b) into master (db42866) will decrease coverage by 0.00%. The diff coverage is n/a.

:exclamation: Current head c55cd1b differs from pull request most recent head 27cd03e. Consider uploading reports for the commit 27cd03e to get more accurate results

@@            Coverage Diff             @@
##           master    #3025      +/-   ##
==========================================
- Coverage   92.60%   92.59%   -0.01%     
==========================================
  Files         596      596              
  Lines       69791    69791              
  Branches     6617     6609       -8     
==========================================
- Hits        64627    64622       -5     
- Misses       5131     5136       +5     
  Partials       33       33              
Impacted Files Coverage Δ
src/lib/utils/cpuid/cpuid_x86.cpp 76.62% <0.00%> (-11.69%) :arrow_down:
src/lib/pk_pad/emsa_raw/emsa_raw.cpp 71.42% <0.00%> (-2.86%) :arrow_down:
src/lib/asn1/asn1_obj.cpp 94.73% <0.00%> (-1.76%) :arrow_down:
src/lib/tls/tls12/tls_channel_impl_12.cpp 89.08% <0.00%> (+0.28%) :arrow_up:
src/lib/pubkey/mce/mceliece_key.cpp 84.29% <0.00%> (+1.04%) :arrow_up:
src/lib/asn1/der_enc.cpp 86.33% <0.00%> (+2.48%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Aug 02 '22 18:08 codecov-commenter

Are there better but still simple solutions?

I'm not sure about promoting those example-builds as a full-blown build target for two reasons:

  1. Users and distributers would need to be educated that this build target is somewhat internal and will not result in any meaningful build artefact
  2. The "examples" build should as much as possible mimick an actual library user. Which might not be the case when they are built via Botan's main build system.

Instead, let's place a simple Makefile next to the example source files and invoke that exclusively in a CI job from ci_build.py. The CI can then make sure that the library is configured sufficiently and we avoid the confusion of the new first-class build target.

Otherwise, that's really cool! Little is more frustrating for new users than broken examples in the documentation.

reneme avatar Aug 04 '22 15:08 reneme

This is great! Being able to test documentation examples is a feature I really like about rustdoc and have missed in C++. Also agree with @reneme that this would be confusing as a toplevel target that is visible to end users - building these examples for test purposes should only ever happen in our CI.

randombit avatar Aug 04 '22 19:08 randombit

Replaced the added target with a separate Makefile in the examples directory.

lieser avatar Aug 09 '22 16:08 lieser

I think this is now ready for a review. Note that I tried to keep the code move in separate commits. So looking at the commits separately should make it easier to see what I changed in the examples.

lieser avatar Aug 11 '22 15:08 lieser