Eliminate distutils.util.strtobool

[bartoldeman]

It's only used once so I open-coded it. A stricter but not backwards compatible check would be to only allow 'True' and 'False'.

Addresses part of #3963

[branfosj]

From https://docs.python.org/3.9/distutils/apiref.html#distutils.util.strtobool

True values are y, yes, t, true, on and 1; false values are n, no, f, false, off and 0.

[Flamefire]

I'd not use EasyBuildError here, we use it way to much IMO. Raising this will log an "Easybuild crashed" message which I'm not sure we usually want. See #4301
I don't see the purpose of this error type at all. Python has enough standard error types (like the previous ValueError) and we could just catch any error in the main function and log the "EasyBuild crashed" there with the stacktrace of the error.

However we use EasyBuildError below so this is at least consistent. Just wanted to bring this topic up that we shouldn't keep replacing specific errors with the generic EasyBuildError

[boegel]

You're right, we should improve our error reporting quite a bit.

My hope was to improve that while developing EasyBuild v5.0, but the focus is on different currently (and I don't want to keep postponing the release of EasyBuild v5.0 either, so this may need to be an iterative effort)

[Flamefire]

True. Point here was my last sentence: We shouldn't make it worse going forward and check for introducing "wrong" EasyBuildErrors in PRs in the future.

[bartoldeman]

My reasoning was indeed consistency with the EasyBuildError below. In this case it catches a syntax error in the easyconfig, which I see as semantically different from a ValueError which (if not handled) I'd consider a bug in EasyBuild itself (this is just my perception, it may not be correct!).

[Flamefire]

From what EasyBuildError does it should be used when EasyBuild ran into a state where the build must be aborted (it currently logs "EasyBuild crashed")
In this case ValueError is correct: The function "Convert a comma-separated string or 2/3-element list of strings to a dictionary", i.e. a value to a toolchain dict. If it can't do that, the value is wrong. Same as int("abc") would fail as the value is not an integer.

Note that this function is not parsing an EasyConfig. It is used when parsing an EasyConfig. And if that EasyConfig isn't a valid EasyConfig due to wrong values then again this is a ValueError. But it might be used in other places. The function that parses the EasyConfig may catch this ValueError and handle it. One way of handling would be to throw an EasyBuildError from this ValueError. However the function could as well just ignore the EasyConfig and try another one. And especially as the EasyConfig might not be from EasyBuild but from a user I wouldn't consider it "a bug in EasyBuild itself"

Using exceptions/errors is actually very common in Python which has the philosophy "Ask for forgiveness not for permission". IIRC the dict.get method is implemented like this:

def get(self, key, default):
  try:
    return self[key]
  catch KeyError:
    return default

So using this errors and handling them on a higher level is actually good practice in Python (where exceptions are supposedly very cheap)

I hope that makes sense and helps making the error handling better/more consistent.