Draft: buildtools:wafsamba: make the dependency include order deterministic

Stian Halsethrequested to merge
shalseth/samba:wafsamba-deterministic-includes into master

build_includes() iterates inc_deps, a set of target names, so the order in which dependencies contribute -I flags is unspecified. When two dependencies provide a header of the same name, which one a target compiles against is therefore luck, and that luck differs per build host.

This is the root cause Andrew Bartlett identified in bug 15080 in 2022: "the set reduction used in samba_deps.py being non-determinately ordered between platforms". It is also what produced the sparc64 build failure in https://bugzilla.samba.org/show_bug.cgi?id=16229, and the duplicate gssapi.h reported on samba-technical in November 2022 is the same shape.

Note that PYTHONHASHSEED=1, which buildtools/wafsamba/wscript already requires, is not sufficient: with that seed set on both machines, hash('HEIMDAL_KRB5') is -5777948353383250625 on amd64 and 7800708043191837418 on a sparc64 T4, so the sets iterate differently.

This change iterates in sorted order, and honours local_include_first for dependencies the way it is already honoured for the target being built, so a dependency's own directory precedes the directories it borrows headers from. It does not by itself make any particular header choice correct, but it makes the choice the same everywhere, so a wrong one fails on every architecture instead of on one porter box.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=15080

Verification

On amd64 against master: of the 1025 targets that have both third_party/heimdal_build and third_party/heimdal/include on their include path, the number searching heimdal/include first ranges from 0 to 891 with the hash seed on master, and is 0 at every seed tried with this change. PYTHONHASHSEED=5, which fails to build master, builds to completion.

Please read before merging

This reorders -I for every target in the tree, not just heimdal's. On the two Debian buildd logs of 2:4.24.6+dfsg-1, counting compiler command lines carrying both -I.../heimdal/lib/gssapi and -I.../heimdal/lib:

amd64   (build succeeded)   310 with lib/gssapi first, 1426 with lib first
sparc64 (build failed)      143 with lib/gssapi first,  610 with lib first

so the current order is not consistent even within a single build, and making it deterministic will change which header a number of objects compile against. I have not run autobuild and cannot. This wants a full CI run before anyone trusts it.

No test suite change, for the same reason as the companion MR: the failure mode is the order in which the build system emits -I flags. It can be regression tested by generating task lists under several PYTHONHASHSEED values and asserting the order is identical between them, which is how the numbers above were produced, and I am happy to turn that into a check under buildtools/.

The companion merge request !4684 fixes the heimdal instance specifically (bug 16229). Either one fixes that build failure on its own; this one is the general case.

Checklist

  • Commits have Signed-off-by: with name/author being identical to the commit author
  • (optional) This MR is just one part towards a larger feature.
  • (optional, if backport required) Bugzilla bug filed and BUG: tag added
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated
  • CI timeout is 3h or higher (see Settings/CICD/General pipelines/ Timeout)

Reviewer's checklist:

  • There is a test suite reasonably covering new functionality or modifications
  • Function naming, parameters, return values, types, etc., are consistent and according to README.Coding.md
  • This feature/change has adequate documentation added
  • No obvious mistakes in the code

Merge request reports