1461774 - gyp_reader.py does not order flags specified in .gyp files optimally

Status: RESOLVED INVALID

(Firefox Build System :: General, defect)

Description

[Tom Ritter [:tjr] (one week until extended leave)]

Right now; nothing needs this patch - the thing I was doing that needed this is going to happen another way.

But what I discovered was that if you had an included .gyp file at the top that set flags (like a --foo cflag) and then later in your gyp file you tried to override one of those flags (with -no-foo) - the --no-foo would come first and then the --foo would come later.

So what you wound up with was flags specified later were being specified first. The attached patch reverses that ordering which, I think anyway, is more sane.

Comment 1

[Tom Ritter [:tjr] (one week until extended leave)]

Attachment: Bug 1461774 Have gyp_reder specify flags in the order they appear in .gyp files instead of the reverse

Review commit: https://reviewboard.mozilla.org/r/244110/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244110/

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 8975905 [details]
Bug 1461774 Have gyp_reder specify flags in the order they appear in .gyp files instead of the reverse

https://reviewboard.mozilla.org/r/244110/#review251500

::: python/mozbuild/mozbuild/frontend/gyp_reader.py:308
(Diff revision 1)
>                          f = expand_variables(f, config.substs).split()
>                          if not f:
>                              continue
>                          # the result may be a string or a list.
>                          if isinstance(f, types.StringTypes):
> -                            context[var].append(f)
> +                            context[var].insert(0, f)

This could stand a one-line comment explaining that flags come out of the gyp reader in reverse order.

Comment 3

[Tom Ritter [:tjr] (one week until extended leave)]

Comment on attachment 8975905 [details]
Bug 1461774 Have gyp_reder specify flags in the order they appear in .gyp files instead of the reverse

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/244110/diff/1-2/

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8975905 [details]
Bug 1461774 Have gyp_reder specify flags in the order they appear in .gyp files instead of the reverse

https://reviewboard.mozilla.org/r/244110/#review251928

Comment 5

[Tom Ritter [:tjr] (one week until extended leave)]

Actually, while this did fix the problem I was trying to solve, it broke a bunch of other stuff. And since my problem isn't a problem anymore it's not worth trying to resolve. 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66a261d72f50d6fa35a4dcf7d786b96c0a641fad&selectedJob=179694772