make build-clang.py use json instead of simplejson

RESOLVED FIXED in mozilla23

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: froydnj, Unassigned)

Tracking

unspecified
mozilla23

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

6 years ago
No description provided.
Reporter

Comment 1

6 years ago
The OS X machines don't appear to have simplejson installed, and Python comes
with decent json support nowadays anyway.
Attachment #736424 - Flags: review?(mh+mozilla)
Comment on attachment 736424 [details] [diff] [review]
use json instead of simplejson in build-clang.py

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

We only used simplejson as a stop-gap until modern Python was deployed everywhere. stdlib json is effectively equivalent to simplejson in modern Pythons (at least in terms of perf).
Attachment #736424 - Flags: review?(mh+mozilla) → review+
Reporter

Comment 4

6 years ago
It turns out the simple s/simplejson/json/ didn't work because
build_tooltool_manifest used item_sort_key to ensure that the objects
read in were written out in the same order.  This scheme was brittle,
since it only understand a couple of keys from the manifest file, and it
doesn't work with the stdlib json.

Instead, use object_pairs_hook=collections.OrderedDict with json.load,
which ensures that current objects get written out in the same order
they were read in *and* that any future keys will be handled
automagically.

I fail for not actually running the script all the way through.  gps
fails for not catching it and so, as punishment, he gets to review the
fix. ;) I have verified that I can build clang packages with these
changes.
Attachment #736477 - Flags: review?(gps)
Comment on attachment 736477 [details] [diff] [review]
followup: fix json.dump call and be more explicit about object ordering

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

I kinda assumed you tested this before commit :)

Yay for Python 2.7!
Attachment #736477 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/71bf6bcba445
https://hg.mozilla.org/mozilla-central/rev/ebcfc07b6315
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.