1482676 - configure should prefer .mozbuild copy of "node" if "nodeJS" is later in the PATH

Status: RESOLVED FIXED

(Firefox Build System :: Bootstrap Configuration, enhancement, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

From bug 1481693 comment 24:

> You mentioned on IRC that there were issues if both `node` and `nodejs` exist: due to the way traversal works, we apparently search all paths for executable variant A before variant B. That means we could pick up a `nodejs` executable on a later path even if a sooner path had `node` available.

> I don't think this is a deal breaker. But we may want to make check_prog() invert the traversal order by default or by request as a follow-up.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: add pathsHavePriority option to check_prog for use with Node

MozReview-Commit-ID: 5Nj49cjD7cq

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8999489 [details] [diff] [review]
add pathsHavePriority option to check_prog for use with Node

We do need this before we ask people to start testing, since otherwise they will be testing the wrong code and getting bogus results on Linux distros.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=90ce01c07c4290f65876f5fa57aea1c197003523

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Hmm, I've canceled that try job because there's infrastructure lossage going on, causing lots of false build failures.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

OK, word on the street is that things are getting better, so here goes:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=67fae45898dd851964f13fca38924254efe34ee8

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8999489 [details] [diff] [review]
add pathsHavePriority option to check_prog for use with Node

Review of attachment 8999489 [details] [diff] [review]:
-----------------------------------------------------------------

r+ conditional on using the proper naming convention.

::: build/moz.configure/checks.configure
@@ +102,5 @@
>  # use that value instead.
>  @template
>  @imports(_from='mozbuild.shellutil', _import='quote')
>  def check_prog(var, progs, what=None, input=None, allow_missing=False,
> +               paths=None, pathsHavePriority=False, when=None):

Names in Python use underscores. So this should be paths_have_priority.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4da6d38fa33
configure should prefer .mozbuild copy of node

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: configure should prefer .mozbuild copy of node, v2

MozReview-Commit-ID: 91F4Z8GljkV

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8999660 [details] [diff] [review]
configure should prefer .mozbuild copy of node, v2

Naming convention fixed.

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/e4da6d38fa33