[step_runner] add logic to resolve absolute path of argv[0] on windows.

[step_runner] add logic to resolve absolute path of argv[0] on windows.

CreateProcess will automagically append '.exe' if the application
supplied doesn't have one, which explains why "cipd" works on bots
(cipd.exe is in PATH), but not for devs (cipd.bat is in PATH, but no
cipd.exe).

This CL will hunt through PATHEXT for each folder on PATH, which is what
cmd.exe does when you type e.g. 'cipd'. If the supplied command already has
an extension or is an absolute path, behavior will not change.

This hunt is done before printing the step to stdout, so we should be
able to see when this logic had an effect.

R=vadimsh@chromium.org,dnj@chromium.org
BUG=697327
Review-Url: https://codereview.chromium.org/2727553005
Committed: https://github.com/luci/recipes-py/commit/ea723457b7a1d098594e68ad82f98964dc3d6cfa

[Vadim Sh., 2017-03-01 19:40:45]

please try it for real before committing

https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.py
File recipe_engine/step_runner.py (right):

https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.p...
recipe_engine/step_runner.py:651: if sys.platform == "win32":
nit: the rest of the file seems to be using ' for strings

https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.p...
recipe_engine/step_runner.py:676: full_pathext = step_env.get("PATHEXT",
".EXE;.BAT").split(os.path.pathsep)
nit: let's lower them, or we'll have ugly "cipd.EXE" in step logs

https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.p...
recipe_engine/step_runner.py:681: for path, ext in itertools.product(path, ext):
er.. what?.. Should it be itertools.product(full_path, full_pathext)?

[iannucci, 2017-03-01 19:53:41]

On 2017/03/01 19:40:45, Vadim Sh. wrote:
> please try it for real before committing

obviously!! :D

> 
> https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.py
> File recipe_engine/step_runner.py (right):
> 
>
https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.p...
> recipe_engine/step_runner.py:651: if sys.platform == "win32":
> nit: the rest of the file seems to be using ' for strings
> 
>
https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.p...
> recipe_engine/step_runner.py:676: full_pathext = step_env.get("PATHEXT",
> ".EXE;.BAT").split(os.path.pathsep)
> nit: let's lower them, or we'll have ugly "cipd.EXE" in step logs
> 
>
https://codereview.chromium.org/2727553005/diff/1/recipe_engine/step_runner.p...
> recipe_engine/step_runner.py:681: for path, ext in itertools.product(path,
ext):
> er.. what?.. Should it be itertools.product(full_path, full_pathext)?

(fixed all of this in following patchsets)

[Vadim Sh., 2017-03-01 20:01:48]

lgtm

[iannucci, 2017-03-01 20:44:09]

On 2017/03/01 20:01:48, Vadim Sh. wrote:
> lgtm

Actually, what do you think if this is explicitly restricted to ('.exe',
'.bat')? We clearly didn't do this before, and I can't imagine any use cases for
the other extensions (in the sort of environments where recipes should run).

[Vadim Sh., 2017-03-01 20:45:00]

On 2017/03/01 20:44:09, iannucci wrote:
> On 2017/03/01 20:01:48, Vadim Sh. wrote:
> > lgtm
> 
> Actually, what do you think if this is explicitly restricted to ('.exe',
> '.bat')? We clearly didn't do this before, and I can't imagine any use cases
for
> the other extensions (in the sort of environments where recipes should run).

sgtm

[iannucci, 2017-03-01 20:46:37]

On 2017/03/01 20:45:00, Vadim Sh. wrote:
> On 2017/03/01 20:44:09, iannucci wrote:
> > On 2017/03/01 20:01:48, Vadim Sh. wrote:
> > > lgtm
> > 
> > Actually, what do you think if this is explicitly restricted to ('.exe',
> > '.bat')? We clearly didn't do this before, and I can't imagine any use cases
> for
> > the other extensions (in the sort of environments where recipes should run).
> 
> sgtm

for the record, the PATHEXT we see on bots looks like:
.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC

[iannucci, 2017-03-01 20:55:28]

The CQ bit was checked by iannucci@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 20:55:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 20:58:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-01 20:58:43]

Dry run: This issue passed the CQ dry run.

[iannucci, 2017-03-01 22:52:54]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2017-03-01 22:52:55]

The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org
Link to the patchset: https://codereview.chromium.org/2727553005/#ps140001
(title: "fix comment")

[commit-bot: I haz the power, 2017-03-01 22:53:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-01 22:56:35]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1488408774739480,
"parent_rev": "91d331a98de5bbb709ddda15684059a3e1255088", "commit_rev":
"ea723457b7a1d098594e68ad82f98964dc3d6cfa"}

[commit-bot: I haz the power, 2017-03-01 22:56:40]

Description was changed from

==========
[step_runner] add logic to resolve absolute path of argv[0] on windows.

CreateProcess will automagically append '.exe' if the application
supplied doesn't have one, which explains why "cipd" works on bots
(cipd.exe is in PATH), but not for devs (cipd.bat is in PATH, but no
cipd.exe).

This CL will hunt through PATHEXT for each folder on PATH, which is what
cmd.exe does when you type e.g. 'cipd'. If the supplied command already has
an extension or is an absolute path, behavior will not change.

This hunt is done before printing the step to stdout, so we should be
able to see when this logic had an effect.

R=vadimsh@chromium.org,dnj@chromium.org
BUG=697327
==========

to

==========
[step_runner] add logic to resolve absolute path of argv[0] on windows.

CreateProcess will automagically append '.exe' if the application
supplied doesn't have one, which explains why "cipd" works on bots
(cipd.exe is in PATH), but not for devs (cipd.bat is in PATH, but no
cipd.exe).

This CL will hunt through PATHEXT for each folder on PATH, which is what
cmd.exe does when you type e.g. 'cipd'. If the supplied command already has
an extension or is an absolute path, behavior will not change.

This hunt is done before printing the step to stdout, so we should be
able to see when this logic had an effect.

R=vadimsh@chromium.org,dnj@chromium.org
BUG=697327

Review-Url: https://codereview.chromium.org/2727553005
Committed:
https://github.com/luci/recipes-py/commit/ea723457b7a1d098594e68ad82f98964dc3...
==========

[commit-bot: I haz the power, 2017-03-01 22:56:41]

Committed patchset #8 (id:140001) as
https://github.com/luci/recipes-py/commit/ea723457b7a1d098594e68ad82f98964dc3...