Re-implemented salt.utils.path.which to properly define executable semantics on both the windows and posix platforms.

[arizvisa]

What does this PR do?

The original salt.utils.path.which() function had a number of issues, one of which resulted in #51784. The original implementation did not appear to properly isolate windows and posix semantics which is why the issue came into existence. Another probably non-important issue is that memoization is attempted to be used, but on a function with no arguments which results in the performance benefit not being able to be taken advantage of. I assume the original author intended to use memoization to improve the peformance of the regular expression.

In essence, this PR is the same as the following code but with the ability to follow links.

executable = lambda(_): os.path.isfile(_) and os.access(_,os.X_OK)
which = lambda _,envvar="PATH",extvar='PATHEXT':_ if executable(_) else iter(filter(executable,itertools.starmap(os.path.join,itertools.product(os.environ.get(envvar,os.defpath).split(os.pathsep),(_+e for e in os.environ.get(extvar,'').split(os.pathsep)))))).next()

What issues does this PR fix or reference?

This fixes issue #51784, and also re-implements some of the optimizations that the original author desired.

Previous Behavior

In some instances (see #51784), the salt.utils.path.which() function would return non-(shell)-executable programs on the Windows platform, and also not benefit from memoization used to reduce the number of times a regular expression is matched (which it appears that the original author considered expensive)

New Behavior

Now it splits up the posix and windows logic so that the windows implementation properly searches the path for executables according to the PATHEXT environment variable, posix version will follow links in order to identify whether the link is executable, and the matching of PATHEXT is done in constant-time without using regexes.

Tests written?

No

Commits signed with GPG?

No

[arizvisa]

Some of these tests need to be punted as they're dying due to a MemoryError in code unrelated to mine.

[arizvisa]

Still something to fix... just a moment here.

[arizvisa]

Force-pushed as the commit specified the wrong module path.

[arizvisa]

Hmm..why is salt.utils.is_running not being loaded in the tests for this? I walked through the backtrace and nothing there is importing my module, and I tested via the following (which is what the backtrace is doing) and it works without a problem?

$ salt --async \* state.apply
Executed command with job ID: 20190225210028096476
$ salt \* saltutil.is_running 'state.*'
888e0dab3011409888f36dbf6153f9cf:
    |_
      ----------
      arg:
      fun:
          state.apply
      jid:
          20190225210028096476
      master_id:
          888e0dab3011409888f36dbf6153f9cf
      pid:
          508
      ret:
          etcd
      tgt:
          *
      tgt_type:
          glob
      user:
          root
bX_nxPEVRyWOcIDBECwW:
    |_
      ----------
      arg:
      fun:
          state.apply
      jid:
          20190225210028096476
      master_id:
          888e0dab3011409888f36dbf6153f9cf
      pid:
          2136
      ret:
          etcd
      tgt:
          *
      tgt_type:
          glob
      user:
          root


-------------------------------------------
Summary
-------------------------------------------
# of minions targeted: 2
# of minions returned: 2
# of minions that did not return: 0
# of minions with errors: 0
-------------------------------------------
$

[arizvisa]

Bah, this one also needs its tests punted as they're having that saltutil.is_running error again...

I patched the unit-tests to include an assignment to os.pathsep since it's ':' on linux, and also changed some of the side-effects of os.access since the original implementation had one more syscall than was necessary.

[arizvisa]

Wow, am I reading this right? All the tests passed, yay.

[arizvisa]

Thx @twangboy