Do not use deprecated find_executable #1000
Conversation
| # Try to find the full compiler path from the name | ||
| if param != "": | ||
| newp = find_executable(param) | ||
| newp = shutil.which(param) |
There was a problem hiding this comment.
This might be slightly different between windows v linux v unix.
Also does shutil reference any aliases? Is that necessary?
There was a problem hiding this comment.
(not blocking just curious)
There was a problem hiding this comment.
No idea tbh, I only followed the deprecation recommendation
There was a problem hiding this comment.
I'll have to check how it handles aliases, but shutil.which is supposed to handle symlinks besides regular files, and it should also be able to work with executable extensions. The difference between path handling between windows and unix systems doesn't look to be too bad.
ricardoV94 force-pushed the deprecated_find_executable branch from 950a77a to 6bb9b59 Compare 
| # Try to find the full compiler path from the name | ||
| if param != "": | ||
| newp = find_executable(param) | ||
| newp = shutil.which(param) |
There was a problem hiding this comment.
I'll have to check how it handles aliases, but shutil.which is supposed to handle symlinks besides regular files, and it should also be able to work with executable extensions. The difference between path handling between windows and unix systems doesn't look to be too bad.
| Oops, I hadn't looked at the failed tests. I'll have to dig a bit deeper into the difference between |
| Someone mentioned this failure in another PR so it might be unrelated actually |
Yeah, it looks like it started at some point in the last three days and it's hitting multiple PRs. It must be some environment change in the github actions container. |
Any idea how to go about fixing it? |
It’s hard without a local env that fails. It could be possible to open a dummy PR and try to debug via the CI workflows. These failures in the blas configuration are just a mess. I don’t know why it was originally programmed in a way that yields an invalid configuration by default when things fail during the blas checks |
What do you mean? On the CI we just want to make sure we are running against BLAS |
ricardoV94 force-pushed the deprecated_find_executable branch 3 times, most recently from 9bbd19c to c5859d9 Compare 
maresb left a comment
maresb left a comment There was a problem hiding this comment.
Not sure about links but LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
@@ Coverage Diff @@ ## main #1000 +/- ## ======================================= Coverage 81.74% 81.74% ======================================= Files 183 183 Lines 47724 47733 +9 Branches 11616 11616 ======================================= + Hits 39011 39020 +9 - Misses 6518 6520 +2 + Partials 2195 2193 -2
|
ricardoV94 force-pushed the deprecated_find_executable branch 2 times, most recently from 1256fee to 7ae68ed Compare ricardoV94 force-pushed the deprecated_find_executable branch from 3532922 to a9ed0f9 Compare 4 participants
Closes #997