modernize ipdlsrcs.mk writing

RESOLVED FIXED in mozilla27

Status

RESOLVED FIXED
5 years ago
9 months ago

People

(Reporter: froydnj, Unassigned)

Tracking

unspecified
mozilla27

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
I started refactoring ipdlsrcs.mk writing so that I could build ipc/ipdl in
unified mode per bug 907789.  I ran into some problems, which need to be
solved elsewhere, b ut I thought I would at least throw the patches up for
review now, since they're all finished.
(Reporter)

Comment 1

5 years ago
Created attachment 804627 [details] [diff] [review]
part 1 - convert ipdlsrcs.mk writing to use mozbuild.makeutil

We have better ways to do things nowadays.  Trivial.
Attachment #804627 - Flags: review?(gps)
(Reporter)

Comment 2

5 years ago
Created attachment 804628 [details] [diff] [review]
part 2 - write CPPSRCS from each ipdl file in one go

This patch is probably misnamed; it's mostly here to separate the logic for
determining what files we're generating from a single ipdl file from the logic
for actually writing the necessary definitions.
Attachment #804628 - Flags: review?(gps)
(Reporter)

Comment 3

5 years ago
Created attachment 804629 [details] [diff] [review]
part 3 - write ALL_IPDLSRCS and CPPSRCS in one go

Maybe not strictly necessary, but if pymake is happier with a single statement
rather than a bunch of variable appends, a small efficiency improvement.
Attachment #804629 - Flags: review?(gps)

Updated

5 years ago
Attachment #804627 - Flags: review?(gps) → review+

Updated

5 years ago
Attachment #804628 - Flags: review?(gps) → review+

Comment 4

5 years ago
Comment on attachment 804629 [details] [diff] [review]
part 3 - write ALL_IPDLSRCS and CPPSRCS in one go

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

Huzzah!
Attachment #804629 - Flags: review?(gps) → review+
Sorry, I backed this out on inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbb0d3946a92

because of this build error:
NameError: global name 'mozmakeutil' is not defined
https://tbpl.mozilla.org/php/getParsedLog.php?id=27853917&tree=Mozilla-Inbound
(Reporter)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8da12b9e73f
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9a59bc50ffb
https://hg.mozilla.org/integration/mozilla-inbound/rev/e068176c4814

Actually |import|'d all the things I was supposed to this time, rather than relying on previous patches in my queue to provide them.
(Reporter)

Comment 9

5 years ago
Created attachment 808595 [details] [diff] [review]
part 2 - separate out files-from-this-ipdl logic from writing CPPSRCS

Ah, the tests, the tests.  Must fix the tests.

I think the blank lines technically from from the mozbuild.makeutil conversion,
but I am being lazy and not splitting the test changes up amongst all three
patches...
Attachment #808595 - Flags: review?(gps)
(Reporter)

Comment 10

5 years ago
Created attachment 808596 [details] [diff] [review]
part 3 - write ALL_IPDLSRCS and CPPSRCS in one go

More test changes.  Wasn't quite sure what to do about the overly long lines; please
yell if the chosen path is not acceptable.
Attachment #804629 - Attachment is obsolete: true
Attachment #808596 - Flags: review?(gps)
(Reporter)

Updated

5 years ago
Attachment #804628 - Attachment is obsolete: true

Updated

5 years ago
Attachment #808595 - Flags: review?(gps) → review+
Comment on attachment 808596 [details] [diff] [review]
part 3 - write ALL_IPDLSRCS and CPPSRCS in one go

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

I am slightly scared by the frequency you use itertools. The force is strong with you.
Attachment #808596 - Flags: review?(gps) → review+
(Reporter)

Comment 13

5 years ago
(In reply to Gregory Szorc [:gps] from comment #11)
> I am slightly scared by the frequency you use itertools. The force is strong
> with you.

\o/

Updated

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