Closed Bug 948787 Opened 10 years ago Closed 10 years ago

Print diffs during config.status

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Depends on: 948272
Attached patch Print diffs during config.status (obsolete) — Splinter Review
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: nobody → gps
Status: NEW → ASSIGNED
This should fix the test failure.
Attachment #8345741 - Flags: review?(mh+mozilla)
Attachment #8345681 - Attachment is obsolete: true
Attachment #8345681 - Flags: review?(mh+mozilla)
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+
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-
https://hg.mozilla.org/mozilla-central/rev/d12b0a6c7641
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
This doesn't work for added and removed files :-/
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: