Last Comment Bug 526817 - Port bug 526668 - add option to 'unify' to allow files to match if their sorted contents match
: Port bug 526668 - add option to 'unify' to allow files to match if their sort...
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: Thunderbird 3.1a1
Assigned To: Mark Banner (:standard8, limited time in Dec)
:
:
Mentors:
Depends on: 518641 compdir-lockdown 526668
Blocks: 559088 559184
  Show dependency treegraph
 
Reported: 2009-11-05 11:10 PST by Mark Banner (:standard8, limited time in Dec)
Modified: 2010-05-10 04:40 PDT (History)
1 user (show)
standard8: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
.5-fixed


Attachments
The fix [Checkin: Comment 2] (479 bytes, patch)
2009-11-10 11:26 PST, Mark Banner (:standard8, limited time in Dec)
gozer: review+
Details | Diff | Splinter Review
(Bv1) Add missed "all-test-dirs.list", Unify indentation [Checkin: See comment 4+5] (5.69 KB, patch)
2010-03-04 08:59 PST, Serge Gautherie (:sgautherie)
gozer: review+
Details | Diff | Splinter Review
(Ev1-191) Backport to c-1.9.1 too (4.87 KB, patch)
2010-03-24 15:24 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
(Ev1a-191) Backport to c-1.9.1 too [Checkin: See comment 14] (5.42 KB, patch)
2010-03-24 18:32 PDT, Serge Gautherie (:sgautherie)
gozer: review+
standard8: approval‑thunderbird3.0.5+
Details | Diff | Splinter Review

Description Mark Banner (:standard8, limited time in Dec) 2009-11-05 11:10:28 PST
+++ This bug was initially created as a clone of Bug #526668 +++

We have some files that wind up with the same contents in a slightly different order when you run a parallel build. The 'unify' script currently expects files to have identical contents, so these situations break universal builds easily. I have a patch that adds a --unify-with-sort <regex> option to unify, so we can pass in file patterns and have them special-cased. unify will sort the contents of the files and compare that instead.
Comment 1 Mark Banner (:standard8, limited time in Dec) 2009-11-10 11:26:33 PST
Created attachment 411470 [details] [diff] [review]
The fix
[Checkin: Comment 2]

Ports the fix from bug 526668 and from bug 519357.
Comment 2 Mark Banner (:standard8, limited time in Dec) 2009-11-10 12:02:59 PST
Checked in:
http://hg.mozilla.org/comm-central/rev/588b16a4afb2
Comment 3 Serge Gautherie (:sgautherie) 2010-03-04 08:59:09 PST
Created attachment 430325 [details] [diff] [review]
(Bv1) Add missed "all-test-dirs.list", Unify indentation
[Checkin: See comment 4+5]

First patch was incomplete because bug 518641 had not be ported yet...

NB: All 'Bo' SeaMonkey builds are failing now and most probably need this.
Comment 4 Serge Gautherie (:sgautherie) 2010-03-04 09:36:13 PST
Comment on attachment 430325 [details] [diff] [review]
(Bv1) Add missed "all-test-dirs.list", Unify indentation
[Checkin: See comment 4+5]


http://hg.mozilla.org/comm-central/rev/7cb6c4474d90
(Bv1a) Unify indentation
http://hg.mozilla.org/comm-central/rev/b357570131ea
(Cv1) Complete bug 526668 port: add missed "all-test-dirs.list"
Comment 5 Serge Gautherie (:sgautherie) 2010-03-04 09:46:18 PST
Comment on attachment 430325 [details] [diff] [review]
(Bv1) Add missed "all-test-dirs.list", Unify indentation
[Checkin: See comment 4+5]


(In reply to comment #4)

+
http://hg.mozilla.org/comm-central/rev/364f310598e3
(Dv1) Add missing '\'
Comment 6 Serge Gautherie (:sgautherie) 2010-03-24 15:24:28 PDT
Created attachment 434699 [details] [diff] [review]
(Ev1-191) Backport to c-1.9.1 too
Comment 7 Serge Gautherie (:sgautherie) 2010-03-24 18:32:51 PDT
Created attachment 434759 [details] [diff] [review]
(Ev1a-191) Backport to c-1.9.1 too
[Checkin: See comment 14]

Ev1-191, with Makefile.in part too.
Comment 8 Mark Banner (:standard8, limited time in Dec) 2010-04-06 02:26:30 PDT
Comment on attachment 434759 [details] [diff] [review]
(Ev1a-191) Backport to c-1.9.1 too
[Checkin: See comment 14]

>diff --git a/Makefile.in b/Makefile.in
...
>+else
>+package-tests::
>+	$(MAKE) -C mozilla $@

This change doesn't seem related to this patch/bug and there's no explanation of why it is here.

Therefore a=Standard8 only without that change included in this patch.
Comment 9 Serge Gautherie (:sgautherie) 2010-04-06 09:56:51 PDT
Comment on attachment 434759 [details] [diff] [review]
(Ev1a-191) Backport to c-1.9.1 too
[Checkin: See comment 14]


(In reply to comment #8)
> This change doesn't seem related to this patch/bug and there's no explanation
> of why it is here.

It is. The explanation is in comment 3: see patch Ev1-CC in that bug.
Comment 10 Mark Banner (:standard8, limited time in Dec) 2010-04-13 01:34:56 PDT
(In reply to comment #9)
> (From update of attachment 434759 [details] [diff] [review])
> 
> (In reply to comment #8)
> > This change doesn't seem related to this patch/bug and there's no explanation
> > of why it is here.
> 
> It is. The explanation is in comment 3: see patch Ev1-CC in that bug.

Comment 3 is not an explanation for Ev1-CC. Try looking at it from my perspective: Bv1 is a patch against trunk, it does not include any diffs like the package-tests one. Ev1-CC doesn't include any additional references.

Indeed I notice now its only in comment 3 that you added the dep for bug 518641 which wasn't an original focus of this bug and isn't part of the title/original intent of this bug. So the only connection is the link between 526668 and 518641 for the bit of your patch that doesn't seem to be required as no-one is broken.

In any case, I don't care about package-tests bit for comm-1.9.1. If SM really cares (and I'd be very surprised given SM2.0 tree is green), then please file a separate bug/patch on that.
Comment 11 Serge Gautherie (:sgautherie) 2010-04-13 10:07:37 PDT
Comment on attachment 434759 [details] [diff] [review]
(Ev1a-191) Backport to c-1.9.1 too
[Checkin: See comment 14]


http://hg.mozilla.org/releases/comm-1.9.1/rev/f4cd10d68090
Ev1a-191, with comment 10 suggestion(s).
Comment 12 Serge Gautherie (:sgautherie) 2010-04-13 10:20:40 PDT
(In reply to comment #10)

> In any case, I don't care about package-tests bit for comm-1.9.1. If SM really

I disagree with your explanation, but anyway:
c-1.9.1 is currently out of sync' with the package-tests part(s) that went in m-1.9.1 (in the meantime).

> cares (and I'd be very surprised given SM2.0 tree is green), then please file a

SM 2.0 is not green but random-orange.
(Though I've stopped closely monitoring it for some time now.)

> separate bug/patch on that.

I filed bug 559088.
Comment 13 Mark Banner (:standard8, limited time in Dec) 2010-04-14 03:02:30 PDT
(In reply to comment #11)
> (From update of attachment 434759 [details] [diff] [review])
> 
> http://hg.mozilla.org/releases/comm-1.9.1/rev/f4cd10d68090
> Ev1a-191, with comment 10 suggestion(s).

Backed out due to Thunderbird build bustage.

http://hg.mozilla.org/releases/comm-1.9.1/rev/f4cd10d68090

Next time, please comment on the bug and/or ping me direct if we get a similar issue. Starring the build and leaving the tree busted is not the right thing to do especially as 3.0 is less actively monitored these days as its not a development head.
Comment 14 Mark Banner (:standard8, limited time in Dec) 2010-05-06 11:03:37 PDT
Checked in: http://hg.mozilla.org/releases/comm-1.9.1/rev/8d8c0bff4fce
Comment 15 Mark Banner (:standard8, limited time in Dec) 2010-05-10 04:40:12 PDT
Build-config only patch: as the build process is still working fine, verifying fixed for TB 3.0.5

Note You need to log in before you can comment on or make changes to this bug.