Closed
Bug 1257542
Opened 8 years ago
Closed 8 years ago
check_prog template should sanity check that `progs` is not a string
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox48 fixed)
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: ted, Assigned: ted)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
I was writing a patch that used `check_prog`, but I wrote: ``` yasm = check_prog('YASM', ('yasm'), allow_missing=True) ``` This didn't fail (because of allow_missing=True) but it didn't do what I expected because `('yasm') == 'yasm'`, so check_prog helpfully checked for ['y', 'a', 's', 'm'] in $PATH. I have a patch to fix that, but it requires exposing 'isinstance' and 'tuple' in the ConfigureSandbox builtins. I've also added a `configure_error` template to use in other templates for situations like this where we want to report a programming error in the moz.configure files themselves.
Assignee | ||
Comment 1•8 years ago
|
||
In Python parens around an expression without a trailing comma is not a tuple, so ('foo') == 'foo'. This is really easy to screw up with check_progs, which coerces progs to a list and would give you ['f','o','o'] in this case. This patch enforces that the progs argument is a tuple or list and errors if it is not. Review commit: https://reviewboard.mozilla.org/r/40791/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/40791/
Attachment #8731714 -
Flags: review?(mh+mozilla)
Comment 2•8 years ago
|
||
Comment on attachment 8731714 [details] MozReview Request: bug 1257542 - check_prog template should sanity check that progs is not a string. r?glandium https://reviewboard.mozilla.org/r/40791/#review37481 ::: build/moz.configure/util.configure:26 (Diff revision 1) > sys.exit(1) > > > @template > @advanced > +def configure_error(message): Please add a comment that this function is meant mainly for use in templates to throw errors about bad uses of them, not about "normal" configure errors. ::: python/mozbuild/mozbuild/configure/__init__.py:96 (Diff revision 1) > > # The default set of builtins. > BUILTINS = ReadOnlyDict({ > b: __builtins__[b] > for b in ('None', 'False', 'True', 'int', 'bool', 'any', 'all', 'len', > - 'list', 'set', 'dict') > + 'list', 'set', 'dict', 'isinstance', 'tuple') put tuple next to the other iterables.
Attachment #8731714 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 3•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5bd947546623f30a25113b0bb996d781ff938f2 bug 1257542 - check_prog template should sanity check that progs is not a string. r=glandium
Comment 4•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a5bd94754662
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•