fab icon indicating copy to clipboard operation
fab copied to clipboard

Investigate use of ``shutil.which`` to discover tool availability

Open MatthewHambley opened this issue 9 months ago • 3 comments

Currently a "null" operation for each tool is needed to determine if it is available. This causes a lot of mess in the code and seems unnecessary as the which command is provided for precisely this job.

It can't cope with the situation where the tool exist but is broken (for example fails to link with a missing library) but maybe we don't need to worry about this as long as tool execution can gracefully report broken tooling.

Some tools (PSyclone is an example) behave different depending on their version but is this something we need to take into account in the availability check?

MatthewHambley avatar Mar 04 '25 13:03 MatthewHambley

Suggested this as a good issue for @cameronbateman-mo

t00sa avatar Aug 05 '25 13:08 t00sa

I don't agree that this create a lot of mess. It's integrated in the base class (typically it is running a simple --help or --version command) once (and with log file improvements the chatter in the log files can be supressed), the result is cached, and I don't think it's noticeable more expensive than running a which. It's like 8 lines of code.

I believe I have written this elsewhere: on our Research HPC we have a broken Python environment. Using which reports that PSyclone (or other tools) is available, but if you try to run it, it just aborts. Yes, the sysadmins should fix this, but it's not exactly their priority (older python version, I just sometimes need to switch to an older version for some other tests, and then forget to unload this :) ). I have seen similar problems in incorrect conda environments. In all cases, not psyclone's or fab's fault, but I think we should make the job for a user as easy as possible.

For the user it's much easier to find the problem when psyclone --version aborts with a strange error, compared to a full psyclone command line (where the user might think that the errors is in the parameters, depending on the error message).

And, I would even agree if we wouldn't have the ability to test if a tool actually run, that it might not be worth adding, but given that we have the better solution, spending effort to removing this capability seems a waste of our time.

hiker avatar Aug 05 '25 13:08 hiker

I've no issues with the current implementation and I don't think there is a need to remove this. If we must detect missing libraries (?) for ELF files (should not pose a threat for statically linked applications), potentially this can be extended?

yaswant avatar Aug 06 '25 08:08 yaswant