Closed Bug 1484503 Opened 6 years ago Closed 6 years ago

Random failure in check_spidermonkey_style.py

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: kats, Assigned: kats)

Details

Attachments

(1 file, 1 obsolete file)

The last indexing run had a failure that resulted from the expected output showing up in the wrong order.
Specifically the output for BadIncludes.h at [1] showed up after the output for BadIncludesOrder-inl.h. Seems like a pre-existing bug with nondeterministic file processing order.

[1] https://searchfox.org/mozilla-central/rev/ef8b3886cb173d5534b954b6fb7eb2d94a9473d0/config/check_spidermonkey_style.py#136-150
This fixes it by making file order deterministic
Assignee: nobody → kats
Attachment #9002264 - Flags: review?(continuation)
Attachment #9002264 - Attachment is obsolete: true
Attachment #9002264 - Flags: review?(continuation)
Attachment #9002265 - Flags: review?(continuation)
Component: Searchfox → JavaScript Engine
Product: Webtools → Core
Comment on attachment 9002265 [details] [diff] [review]
Ensure output is determistically ordered

This should be reviewed by a Spidermonkey peer.
Attachment #9002265 - Flags: review?(continuation) → review?(jdemooij)
Comment on attachment 9002265 [details] [diff] [review]
Ensure output is determistically ordered

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

Great find. A bit surprising we didn't run into this sooner on at least one of our platforms.

::: config/check_spidermonkey_style.py
@@ +304,5 @@
>      for inclname in non_js_inclnames:
>          edges[inclname] = set()
>      # Process all the JS files.
> +    for filename in sorted(js_names.keys()):
> +	print("Checking {}".format(filename))

I'm pretty sure this will spam the console when building Firefox or the JS shell, so I'd just remove it.
Attachment #9002265 - Flags: review?(jdemooij) → review+
Ah whoops I didn't mean to leave that print there; was using it for debugging.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a34cfe5e196
Ensure spidermonkey style check output is deterministic. r=jandem
https://hg.mozilla.org/mozilla-central/rev/7a34cfe5e196
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: