Closed
Bug 948787
Opened 10 years ago
Closed 10 years ago
Print diffs during config.status
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla29
People
(Reporter: gps, Assigned: gps)
References
Details
Attachments
(1 file, 1 obsolete file)
15.46 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
glandium and I think it would be useful if you could do config.status --diff to get it to print differences. Intended use case if to facilitate build system hacking.
Assignee | ||
Comment 1•10 years ago
|
||
Build system developers commonly need to see what changes have on the generated build files. We often put our objdir under version control and diff commits before and after running config.status. This patch adds a --diff option to config.status that will print diffs of changes made during config.status. This functionality is implemented on top of FileAvoidWrite, using Python's built-in diffing library. While display of diffs is opt-in, diffs are always being captured when config.status runs. There could be an unwanted performance regression from this. Because diffs are only computed if files change and most files don't change during most config.status runs, this greatly reduces the surface area of the concern. The area for largest concern is clobber builds. On my machine, I measured an increase of 0.2 to 0.3s from 2.0s. While this is 10-15%, the total time is so small that I don't feel snaking a "capture diff" flag through the build system is worth the effort. This would make a decent followup bug if this turns out to be a problem in the future.
Attachment #8345681 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → gps
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
This should fix the test failure.
Attachment #8345741 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8345681 -
Attachment is obsolete: true
Attachment #8345681 -
Flags: review?(mh+mozilla)
Comment 3•10 years ago
|
||
Comment on attachment 8345741 [details] [diff] [review] Print diffs during config.status Review of attachment 8345741 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozbuild/mozbuild/util.py @@ +175,5 @@ > + # diffing isn't a critical feature, we just ignore the failure. > + # This can go away once FileAvoidWrite uses io.BytesIO and > + # io.StringIO. But that will require a lot of work. > + except (UnicodeDecodeError, UnicodeEncodeError): > + pass self.diff = 'Binary file %s changed' % self.name
Attachment #8345741 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d12b0a6c7641 Good idea assigning to self.diff with a stub message! I also snuck in a change to make all-tests.json print on multiple lines since a single line 10+MB JSON file being diffed was... not pleasant. This was reviewed IRL.
Flags: in-testsuite-
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d12b0a6c7641
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Comment 6•10 years ago
|
||
This doesn't work for added and removed files :-/
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
•