I think we can simplify this, when we are looking for things in PATH we are anyway going to look for python.exe or python binary (and a few combinations of those. Why do we even have to get the full list of files on path at all? I don't see a need for fallback. I think what we do in the fallback should be the main thing. We can have a separate thing in the background that looks at path in lot more detail by examining each file in the directory.

This way we will have the fast detection of anything that is common, and eventual detection of anything that is uncommon python executable pattern.

I agree with @karthiknadig, if we're just looking for python.exe and python, using the fallback directly should be the simplest soln for now.
FWIW I think we should look into the old locators, and how they are working just fine when they do the same thing. Something is going wrong with the new setup somewhere.

We can have a separate thing in the background that looks at path in lot more detail by examining each file in the directory.

This practically never finishes, it'll eventually hang the extension. So we cannot have this running even in background until we find the fix.

Removing my review until this is ready to be reviewed again.

Accidently closed it, sorry.

@karthiknadig, @karrtikr, I've updated the PR to deal with the quirks on Windows. Now we do not watch Windows\system32 and we avoid unnecessary lstat() calls. It should be similar in performance to the old code now.

There is no need to watch for files on PATH. We don't do that now, This can be expensive on slower machines. We also don't for other platforms.

If we are just copying over the old behavior then I'm fine with dropping the watcher stuff. However, my understanding was that we were implementing the new discovery code in the proper way, regardless of how we were doing it before.

In the interest of getting this wrapped up sooner rather than later, I'll drop the watcher stuff for now.

Codecov Report

Merging #15177 (1040ca2) into main (95c5f28) will decrease coverage by 0%.
The diff coverage is 56%.

@@          Coverage Diff           @@
##            main   #15177   +/-   ##
======================================
- Coverage     64%      64%   -1%     
======================================
  Files        558      558           
  Lines      26656    26716   +60     
  Branches    3851     3854    +3     
======================================
+ Hits       17315    17353   +38     
- Misses      8634     8649   +15     
- Partials     707      714    +7     

@karthiknadig ETA on a review?

@karrtikr, FYI, I tried my branch on my Windows laptop and it worked fine. However, I recall that this impacted other people more than it did me. Would you mind trying it out, just to double-check?

@ericsnowcurrently Yeah okay I'll try it out.

@ericsnowcurrently Works for me 👍 Although I see that the paths discovered by this locator are lowercased so they appear to be different than the others. ~ is not used to simplify those paths.

@ericsnowcurrently Works for me 👍

Great! Thanks!

Although I see that the paths discovered by this locator are lowercased so they appear to be different than the others. ~ is not used to simplify those paths.

Hmm, I don't see anything in the PR that might have caused either behavior directly (other than maybe the glob code). I'll open an issue to investigate.

Codecov Report

❌ Patch coverage is 53.91304% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 64%. Comparing base (95c5f28) to head (1040ca2).
⚠️ Report is 2569 commits behind head on main.

@@          Coverage Diff           @@
##            main   #15177   +/-   ##
======================================
- Coverage     64%      64%   -1%     
======================================
  Files        558      558           
  Lines      26656    26716   +60     
  Branches    3851     3854    +3     
======================================
+ Hits       17315    17353   +38     
- Misses      8634     8649   +15     
- Partials     707      714    +7