Improve error when checksum dict has no entry for a file

[Flamefire]

When the dict didn't contain the filename EB will crash with

Invalid checksum spec 'None', should be a string (MD5) or 2-tuple (type, value).

This will now raise a more descriptive error.

I'm not sure how this should be handled. IMO an error is the right solution especially as the recursive call will consider it also as a found valid checksum even when all others failed.

But not sure if this is correct but it is at least backwards compatible: It just improves the error message to e.g.: "Missing checksum for test.txt in {'wrong-name': '1c49562c4b404f3120a3fa0926c8d09c99ef80e470f7de03ffdfa14047960ea5'}"

The alternatives all have strange edge cases

See also easybuilders/easybuild-docs#104

Fixes #4254

[Flamefire]

Rebased again although I'd like to point to easybuilders/easybuild-docs#104 again where I put up a proposed documentation for how checksums should be handled (and partially(!) are right now) which should be discussed and decided on as otherwise it is hard to "fix" things when you don't know what the correct behavior should be.

E.g. here I assume that each possible filename must be listed in the dict, i.e. a missing filename is an error and NOT equivalent to a filename key with None value.
Note that None as the value is still not handled (see #4142) as that requires defining how it should be handled (e.g. via recursive call to get to https://github.com/easybuilders/easybuild-framework/pull/4150/files#diff-0ca2799267ffd6a801a47e695fbe66c047a44775b616b26148100ec11e6ce5e5R1256) especially related to checksum alternatives: Currently an alternative of None would trigger an exception when checksums are enforced even if another (later) alternative would match.

[Flamefire]

Can this small improvement be merged until the whole checksum format is decided which likely will take a while if it even makes it into 4.x?

[boegel]

How about this:

• an error is raised only if a checksum is missing and --enforce-checksums is enabled;
• if a checksum is missing and --enforce-checksums is not enabled, the skipping of the checksum is simply skipped here, by using continue:

try:
    checksum = checksum[filename]
except KeyError:
    if build_option('enforce_checksums'):
        raise EasyBuildError("Missing checksum for %s in dictionary: %s", filename, checksum)
    else:
        # no checksum available, skipping verification since checksums are not enforced
        _log.info("meaningful log message goes here")
        continue

[Flamefire]

I wanted to keep the current behavior that this situation is an error and just provide a meaningful error message.

I drafted easybuilders/easybuild-docs#104 where there are some open question on how checksums should be handled which is likely a breaking change (or a handful of breaking changes)

IMO a dict with a missing filename/key is an error: Likely the dict was meant for arch-specific files&checksums but a new arch is used which wasn't tested yet. Or a spelling error happened making it always fail to verify checksums even if it should.
That's what I suggested in the draft mentioned above: A dict, if present, needs to list each possible filename even without enforce-checksums

[Flamefire]

BTW: With a recent issue I noticed that enforce-checksums might be to greedy: We need a None value if the archive (e.g. Git checkout) isn't checksummable. But we want to check that all other checksums are present.

Now we don't (usually) have lists anymore but lists of dicts of filename-to-checksum. Now if enforce-checksums triggers for every explicit None but a missing filename in a dict doesn't trigger an error if it isn't set we have no way to check e.g. the (old) PyTorch ECs reliably.

Hence I think the approach I suggested makes sense.

[boegel]

Thanks for clarifying, I agree that this is at least a good step forward to avoid totally meaningless errors.

If you noticed any other problems, please make sure there's an issue open for it (or if not, open one)