dependencies/detect: correctly get the log_tried()

[dcbaker]

We're currently trying to call the method .log_tried() on a function, which is invalid. Mypy even helpfully points this out, except that we don't yet run mypy on this file :/

We can fix this, but we may not be able to figure out the method anyway, since we could end up with a situation where there is no way to get the method tried, since the dependency errored on creation and the callable isn't an ExternalDependency initializer.

Fixes: #11145

[codecov]

Codecov Report

Merging #11147 (79018af) into master (b249470) will decrease coverage by 0.12%.
The diff coverage is n/a.

❗ Current head 79018af differs from pull request most recent head 69dafc0. Consider uploading reports for the commit 69dafc0 to get more accurate results

@@            Coverage Diff             @@
##           master   #11147      +/-   ##
==========================================
- Coverage   66.07%   65.94%   -0.13%     
==========================================
  Files         414      207     -207     
  Lines       90036    45023   -45013     
  Branches    19290     9324    -9966     
==========================================
- Hits        59488    29692   -29796     
+ Misses      26003    12967   -13036     
+ Partials     4545     2364    -2181     

Impacted Files	Coverage Δ
scripts/clangtidy.py	0.00% <0.00%> (-93.34%)	⬇️
modules/cuda.py	0.00% <0.00%> (-69.82%)	⬇️
templates/cudatemplates.py	37.50% <0.00%> (-62.50%)	⬇️
compilers/cuda.py	19.63% <0.00%> (-45.40%)	⬇️
dependencies/cuda.py	19.23% <0.00%> (-43.75%)	⬇️
modules/icestorm.py	57.14% <0.00%> (-40.00%)	⬇️
compilers/cython.py	43.18% <0.00%> (-38.64%)	⬇️
dependencies/coarrays.py	45.00% <0.00%> (-17.50%)	⬇️
cmake/traceparser.py	71.11% <0.00%> (-9.10%)	⬇️
compilers/mixins/clike.py	70.12% <0.00%> (-6.69%)	⬇️
... and 259 more

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

[eli-schwartz]

Dependency lookup for foobar with method 'system' failed:

Or

Dependency lookup for foobar unable to determine method failed:

The latter looks... kind of weird.

The real question to be asked is, why is this actually able to happen? When is it allowable for the callable to not be ExternalDependency?

I think that this is the wrong diagnosis of the issue. The real issue is that in commit 96df0fc I threw up my hands in exasperation because I couldn't figure out how else to handle cmake. Later on, we added this code that assumes we get an ExternalDependency like we do everywhere else, but sneakily, in one case it's not an ExternalDependency but rather a lambda that pops out an ExternalDependency.

We should probably not do that, somehow.

[dcbaker]

It could be a functools.partial, it could be a lambda, it could be a generic function, the only requirement we enforce is that it has to be a function with the signature def _() -> ExternalDependency

[dcbaker]

I've updated the message to be a little clearer. This should be a really hard error to hit, though clearly we are hitting it, which means we're having an exception in the ExternalDependency initializer. sigh

[eli-schwartz]

I think it's really easy to have an exception in the initializer, because we run the entire class in the initializer and do all detection there. Including raise DependencyException('cannot find Foobar because the stars do not align'). This is an exception class that exists explicitly to be caught and safely handled as "abort the dependency lookup, log a reason, and report it as not found".

Of course we can sed all def __init__ to def run and make lookups occur after initialization. Might take a couple more tweaks, I suppose.

[dcbaker]

I don't know that it really helps. We probably should move the Exception throwing out of the ExternalDependency alltogether and have the helper function here check if required and not_found, but that's a big complicated set of changes.

[eli-schwartz]

That would defeat the purpose of using exceptions to a) break out of the lookup, b) set a custom not-found message.

Using exceptions is idiomatic python for handling exceptional cases, aborting in __init__ is the weird thing.

[dcbaker]

Yeah, it's throwing in the initializer that's strange and makes things hard to handle, though really using exceptions for control flow isn't idiomatic in any language with exceptions, python included. But for the moment this at least gets rid of the traceback, and I'm not particularly interested in taking on the task of trying to clean up dependencies while I'm still neck deep in build.py...

[eli-schwartz]

Can I reiterate that the short-term fix is still probably to fix the cmake openssl dependency, and not simply say "unknown"?

Again, the problem is not that we hit an exception in the initializer. The problem is we explicitly handle that by checking for a staticmethod of the functools wrapped ExternalDependency except for this one case where we aren't even using an ExternalDependency but instead use a lambda. In retrospect, that lambda was a bad idea.

[dcbaker]

The entire point of using a static method is that if the ExternalDependency is invalid, you can still get certain data about it. because of the exception that obviously doesn't work. So, the problem is that we hit an Exception in the initializer

[eli-schwartz]

No, absolutely not at all. The exception has nothing to do with this. I already explained this several times.

Why would a staticmethod care if __init__ throws an exception when per definition it operates on the class name before __init__ is run?

This code already works as is for every single dependency in all of meson that throws an exception in __init__... Except for openssl. And only openssl.

[punytroll]

Thanks for working on this! I couldn't get my head around it myself.

[bruchar1]

This can be closed since the bug was solved by #12226.

[tristan957]

I wonder if this should stay open since it is a more general solution? @dcbaker if you wanna provide input...

[eli-schwartz]

My original objection was that "a more general solution" is literally just "try: code; except Exception: log('it failed but we arent sure why. Continuing....')".

Standard development policy when faced with type errors is that we fix the data model, not raised an "I dunno" error; I don't see why this should be an exception.

[tristan957]

Cool sounds good. Closed it is.