Bugfix: devtools/symbol-check: Check PE libraries case-insensitively #18490

For some reason, one of my gitian builds failed because "DLL" was uppercase where symbol-check.py expected it lowercase.

Since the PE platform is case insensitive, check it case-insensitively here too.

For some reason, one of my gitian builds failed because "DLL" was uppercase

Which version of Core/descriptors were you building? Which DLL was different?

Checking case insensitively was bought up in #18395, but we didn't see a reason to do it, given that in gitian, everything is set. If DLLs are being randomly named during gitian builds, that not only sounds like a reproducibility issue, but something that should be reported upstream. So it would be good to get details.

Checking insensitively would also seem to introduce the scenario where we could be linked against multiple DLLs, and the script wouldn't fail. I'd rather we check everything exactly, and get notified if the underlying DLLs change.

Yes, this is a determinism issue. So NACK on this. Where does the DLL name come from? Must be either\

1. The name directly specified on linking
2. Encoded in the .LIB import library
3. It uses the name of the DLL that happens to be on disk

(3) would be really bad (in this case we should report/fix the upstream as @fanquake suggests), in the other two cases, it shouldn't diverge on deterministic input.

Which version of Core/descriptors were you building?

A rebased #14137, with QtWinExtras split to a separate depends package.

Which DLL was different?

IMM32 and WINMM

Checking insensitively would also seem to introduce the scenario where we could be linked against multiple DLLs, and the script wouldn't fail.

What do you mean?

NACK

A rebased #14137, with QtWinExtras split to a separate depends package.

So nothing in master is broken, and the security checks are working as expected. If you're building code that isn't in this repository (that also modifies dependencies), I don't think there's an expectation that our symbol/security checks should pass.

Does this mean you're planning on reopening #17213 (the rebased version of #14137)? That was closed due to lack of interest, and I still don't think it's something worthwhile merging. Regardless, if changes need to be made to scripts to accommodate that, they should be part of that PR.

What do you mean?

I mean there's no point being lenient in these scripts if we don't need to be. As mentioned by @laanwj, given we control the input, and outputs are deterministic (otherwise there's an actual bug), I'm happy for us to keep checking for the exact libraries we expect. Even though in reality it may not be possible, there's no need to allow the output to contain a a.dll and/or a.DLL, and I'd rather the build fail if the output from gitian suddenly changes.

Going to close this. There are two NACKs, and I haven't seen an argument to change the current behaviour.