Last Comment Bug 850389 - Move XPIDL variables to moz.build files
: Move XPIDL variables to moz.build files
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 22.0
Assigned To: Gregory Szorc [:gps]
:
Mentors:
Depends on: 818246
Blocks: nomakefiles
  Show dependency treegraph
 
Reported: 2013-03-12 13:42 PDT by Gregory Szorc [:gps]
Modified: 2013-03-13 08:27 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Part 2: Move IDLSRCS into moz.build (auto), v1 (45.56 KB, patch)
2013-03-12 13:49 PDT, Gregory Szorc [:gps]
Pidgeot18: review+
Details | Diff | Review
Part 1: rules.mk update, v1 (1.50 KB, patch)
2013-03-12 20:12 PDT, Gregory Szorc [:gps]
bugspam.Callek: review+
Pidgeot18: feedback+
Details | Diff | Review
Part 2b: Move XPIDL_MODULE and XPIDL_FLAGS, v1 (23.41 KB, patch)
2013-03-12 20:13 PDT, Gregory Szorc [:gps]
Pidgeot18: review+
Details | Diff | Review
Part 3: Manual conversion, v1 (3.28 KB, patch)
2013-03-12 20:14 PDT, Gregory Szorc [:gps]
Pidgeot18: review+
Details | Diff | Review

Description Gregory Szorc [:gps] 2013-03-12 13:42:00 PDT
comm-central needs work performed in bug 818246 applied to it.
Comment 1 Gregory Szorc [:gps] 2013-03-12 13:49:46 PDT
Created attachment 724122 [details] [diff] [review]
Part 2: Move IDLSRCS into moz.build (auto), v1

Conversion was performed automatically using our tried and true mach command. Review should largely be a rubber stamp.
Comment 2 Joshua Cranmer [:jcranmer] 2013-03-12 13:57:38 PDT
Comment on attachment 724122 [details] [diff] [review]
Part 2: Move IDLSRCS into moz.build (auto), v1

Consider this an rs+. All I did was eyeball most of the directories to see that roughly the same number of files were transferred; since it's automatic, I presume that all the spellings, etc. are safe. It also looks like you've picked up every directory in comm-central.

There are few manual things that need transferring as well though.
Comment 3 Gregory Szorc [:gps] 2013-03-12 20:12:29 PDT
Created attachment 724249 [details] [diff] [review]
Part 1: rules.mk update, v1

Pretty much copied lines from m-c's rules.mk.
Comment 4 Gregory Szorc [:gps] 2013-03-12 20:13:13 PDT
Created attachment 724250 [details] [diff] [review]
Part 2b: Move XPIDL_MODULE and XPIDL_FLAGS, v1

Conversion was done automatically. Should be a rubber stamp.
Comment 5 Gregory Szorc [:gps] 2013-03-12 20:14:31 PDT
Created attachment 724251 [details] [diff] [review]
Part 3: Manual conversion, v1

Manual conversion of the remaining XPIDL* variables to moz.build (things in conditionals). You should give this a thorough review.

I haven't tested any of the patches in this bug. I'm really that lazy.
Comment 6 Joshua Cranmer [:jcranmer] 2013-03-12 20:22:40 PDT
Comment on attachment 724249 [details] [diff] [review]
Part 1: rules.mk update, v1

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

I'm not a c-c build config peer, so I don't feel comfortable r+'ing this. Redirecting review to other people who can hopefully r+ this before the m-c patch lands.
Comment 7 Joshua Cranmer [:jcranmer] 2013-03-12 20:27:38 PDT
Comment on attachment 724251 [details] [diff] [review]
Part 3: Manual conversion, v1

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

::: ldap/xpcom/public/moz.build
@@ -17,5 @@
>      'nsILDAPService.idl',
>      'nsILDAPURL.idl',
>  ]
>  
> -XPIDL_MODULE = 'mozldap'

You dropped this line in the file.

Also, as mentioned over IRC, you forgot to remove the stuff from ldap/xpcom/public/Makefile.in.
Comment 8 Joshua Cranmer [:jcranmer] 2013-03-12 20:32:28 PDT
Comment on attachment 724250 [details] [diff] [review]
Part 2b: Move XPIDL_MODULE and XPIDL_FLAGS, v1

I'll admit that I didn't try to compile with these patches, but I don't see anything untoward here. Although it is unusual to see such inherent Makefile syntax as -I$(topsrcdir)/ in the moz.build files, but as mozilla-central is using that syntax for its makefiles...
Comment 9 Justin Wood (:Callek) 2013-03-12 20:51:00 PDT
(In reply to Joshua Cranmer [:jcranmer] from comment #6)
> I'm not a c-c build config peer, so I don't feel comfortable r+'ing this.
> Redirecting review to other people who can hopefully r+ this before the m-c
> patch lands.

To be extra clear, all other parts on this bug I feel are also explicitly build-system reviews, not just this one. However I feel comfortable with your review on all these patches, so carry-on.

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