526817 - Port bug 526668 - add option to 'unify' to allow files to match if their sorted contents match

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Mark Banner (:standard8)]

+++ 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)]

Attachment: The fix [Checkin: Comment 2]

Ports the fix from bug 526668 and from bug 519357.

Comment 2

[Mark Banner (:standard8)]

Checked in:
http://hg.mozilla.org/comm-central/rev/588b16a4afb2

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (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)]

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)]

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)]

Attachment: (Ev1-191) Backport to c-1.9.1 too

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (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)]

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)]

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)]

(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)]

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)]

(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)]

(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)]

Checked in: http://hg.mozilla.org/releases/comm-1.9.1/rev/8d8c0bff4fce

Comment 15

[Mark Banner (:standard8)]

Build-config only patch: as the build process is still working fine, verifying fixed for TB 3.0.5