[vpython] Implement smart probing.

[vpython] Implement smart probing.

Use the "prober" package to enable "vpython" to probe for its
wrapped Python instance without risk of hitting another "vpython"
instance. Previously, identification was just done by name.

This allows "vpython" to be named "python" without infinitely calling
itself.

BUG=chromium:730649
TEST=local

Review-Url: https://codereview.chromium.org/2925723004
Committed: https://github.com/luci/luci-go/commit/5cf5e3db67fbf52c83ae0875d70d58ece1b73b86

[dnj, 2017-06-07 16:45:45]

Description was changed from

==========
[vpython] Implement smart probing.

Use the "prober" package to enable "vpython" to probe for its
wrapped Python instance without risk of hitting another "vpython"
instance. Previously, identification was just done by name.

This allows "vpython" to be named "python" without infinitely calling
itself.

BUG=None
TEST=local
==========

to

==========
[vpython] Implement smart probing.

Use the "prober" package to enable "vpython" to probe for its
wrapped Python instance without risk of hitting another "vpython"
instance. Previously, identification was just done by name.

This allows "vpython" to be named "python" without infinitely calling
itself.

BUG=chromium:730649
TEST=local
==========

[dnj (Google), 2017-06-07 17:10:21]

move to system

[dnj, 2017-06-07 17:27:59]

dnj@chromium.org changed reviewers:
+ dnj@chromium.org

[dnj, 2017-06-07 17:28:00]

(ready now)

[dnj, 2017-06-07 17:28:11]

dnj@chromium.org changed reviewers:
+ iannucci@chromium.org

[dnj, 2017-06-07 17:28:12]

+reviewer

[iannucci, 2017-06-07 17:40:26]

lgtm

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/app...
File vpython/application/application.go (right):

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/app...
vpython/application/application.go:183: lp := lookPath{
doesn't git implement this too? can some of this move to a common code area?

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/pro...
File vpython/application/probe.go (right):

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/pro...
vpython/application/probe.go:99: // since no Python interpreter would return
non-zero for "--version".
it might if it failed to load properly though, right? Should we print something
like "IMAWRAPPER" to stdout instead?

[dnj, 2017-06-07 17:43:26]

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/app...
File vpython/application/application.go (right):

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/app...
vpython/application/application.go:183: lp := lookPath{
On 2017/06/07 17:40:26, iannucci wrote:
> doesn't git implement this too? can some of this move to a common code area?

Git does. The common code part is pretty boring - an exec.Cmd and "return 1" -
so I'd rather not dedupe it. Also worth noting is that this specific
implementation also captures STDOUT, which is not needed by the Git wrapper. I
think this is the right balance - let each app chose its exposure / detection
scheme.

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/pro...
File vpython/application/probe.go (right):

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/pro...
vpython/application/probe.go:99: // since no Python interpreter would return
non-zero for "--version".
On 2017/06/07 17:40:26, iannucci wrote:
> it might if it failed to load properly though, right? Should we print
something
> like "IMAWRAPPER" to stdout instead?

If it fails to load properly, we should not use it as a candidate anyway. The
net effect of "isWrapper" is to skip that candidate, and I think we would want
to do that with a Python binary that can't run.

Perhaps I should invert this? If I make this check "is candidate" instead of "is
wrapper", then this can prune any non-candidate without being confusing. I think
I'll do that.

[dnj, 2017-06-07 17:45:51]

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/pro...
File vpython/application/probe.go (right):

https://codereview.chromium.org/2925723004/diff/20001/vpython/application/pro...
vpython/application/probe.go:99: // since no Python interpreter would return
non-zero for "--version".
On 2017/06/07 17:43:26, dnj wrote:
> On 2017/06/07 17:40:26, iannucci wrote:
> > it might if it failed to load properly though, right? Should we print
> something
> > like "IMAWRAPPER" to stdout instead?
> 
> If it fails to load properly, we should not use it as a candidate anyway. The
> net effect of "isWrapper" is to skip that candidate, and I think we would want
> to do that with a Python binary that can't run.
> 
> Perhaps I should invert this? If I make this check "is candidate" instead of
"is
> wrapper", then this can prune any non-candidate without being confusing. I
think
> I'll do that.

OTOH if Python is broken, I don't want "vpython" to gloss over it with some
hard-to-track behavior. I'll go with your idea and have it print out a sentinel
string.

[dnj (Google), 2017-06-07 18:03:12]

sentinel text

[iannucci, 2017-06-07 18:04:31]

lgtm

[dnj, 2017-06-07 18:05:17]

The CQ bit was checked by dnj@chromium.org

[commit-bot: I haz the power, 2017-06-07 18:05:23]

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-06-07 18:15:12]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496858717408150,
"parent_rev": "85885811d2157568a42bb39001ab39345724555c", "commit_rev":
"5cf5e3db67fbf52c83ae0875d70d58ece1b73b86"}

[commit-bot: I haz the power, 2017-06-07 18:15:15]

Description was changed from

==========
[vpython] Implement smart probing.

Use the "prober" package to enable "vpython" to probe for its
wrapped Python instance without risk of hitting another "vpython"
instance. Previously, identification was just done by name.

This allows "vpython" to be named "python" without infinitely calling
itself.

BUG=chromium:730649
TEST=local
==========

to

==========
[vpython] Implement smart probing.

Use the "prober" package to enable "vpython" to probe for its
wrapped Python instance without risk of hitting another "vpython"
instance. Previously, identification was just done by name.

This allows "vpython" to be named "python" without infinitely calling
itself.

BUG=chromium:730649
TEST=local

Review-Url: https://codereview.chromium.org/2925723004
Committed:
https://github.com/luci/luci-go/commit/5cf5e3db67fbf52c83ae0875d70d58ece1b73b86
==========

[commit-bot: I haz the power, 2017-06-07 18:15:17]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-go/commit/5cf5e3db67fbf52c83ae0875d70d58ece1b73b86