518136 - Consider using cl's showIncludes option instead of mkdepend.exe to generate dependencies

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Rain [:rain1] (inactive)]

We could possibly use pymake to transform the showIncludes output into something that GNU make is happy with.

One example of how to do this is at <http://www.conifersystems.com/2008/10/09/dependencies-from-showincludes/>.

Comment 1

[Rain [:rain1] (inactive)]

> GNU make 

er, pymake itself, that is.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: WIP

Wrapping cl in this cl.py (derived from the example) gives me the following numbers (from executing "make -f client.mk build")

Before
real    102m46.086s
user    5m21.533s
sys     12m3.653s

After
real    88m8.517s
user    4m3.316s
sys     9m3.182s

I'll take a 14% win!  Pymake should benefit even more since it won't have to fire up the interpreter every time it calls cl.

Comment 3

[Justin Wood (:Callek)]

kyle, can you please be sure to coordinate with c-c before this lands (either in a "can someone work on a patch" or you requesting review for a c-c patch from me).

This at least as long as you are breaking the old way for windows.

If you don't have time to write the patch, I can promise at most a 48 hour turnaround once this bug gets review for me writing a c-c ver.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

This does the following:

1. Completely removes the existing mkdepend code from config/ and js/src/config.  I'm assuming that we use gcc with -MD on all non-windows platforms, but if we need this for something obviously we can keep it instead.
2. Creates a python wrapper script that wraps cl and generates the .pp files.
3. Changes configure.in to use this python script as the compiler.
4. Changes rules.mk to not run mkdepend.exe.
5. Teaches mddepend.pl to deal with dependencies with spaces (need to make sure this doesn't break gcc).  This is necessary because showincludes is going to give us all of the windows headers which are included while are going to be somewhere like C:\Program Files\Microsoft SDKs\...  Alternatively we could filter those out and only write out headers that live somewhere inside $(topsrcdir)

Callek: I think the only changes c-c will need to work in this brave new world are 3 and 4.

I've timed this several times and it seems to be consistently a 13-15% win on my machine.

I didn't touch security/coreconf because that looked more involved.

Comment 5

[[e-mail]]

Comment on attachment 451054 [details] [diff] [review]
Patch

>-config/mkdepend/Makefile
Aren't you assuming that your configuration is the only one using mkdepend?

>+  # Figure out what the target is
>+  for arg in cmdline:
>+    if arg.startswith("-Fo"):
>+      target = arg[3:]
>+
>+  # The deps target lives here
>+  depstarget = os.path.basename(target) + ".pp"
Why not set _DEPEND_CFLAGS to something that your script can extract?

>+# When we're using MSVC, we make our dependencies through cl.py
>+MAKE_DEPS_AUTO=
Or set COMPILER_DEPEND in the first place, perhaps?

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #5)
> (From update of attachment 451054 [details] [diff] [review])
> >-config/mkdepend/Makefile
> Aren't you assuming that your configuration is the only one using mkdepend?

Yes.  Is there anything else out there using it.

> >+  # Figure out what the target is
> >+  for arg in cmdline:
> >+    if arg.startswith("-Fo"):
> >+      target = arg[3:]
> >+
> >+  # The deps target lives here
> >+  depstarget = os.path.basename(target) + ".pp"
> Why not set _DEPEND_CFLAGS to something that your script can extract?
>
> >+# When we're using MSVC, we make our dependencies through cl.py
> >+MAKE_DEPS_AUTO=
> Or set COMPILER_DEPEND in the first place, perhaps?

Yeah, that's probably a better way to go.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 451054 [details] [diff] [review]
Patch

Unfortunately the mddepend.pl changes here will blow up gcc.

Comment 8

[[e-mail]]

Comment on attachment 451054 [details] [diff] [review]
Patch

>-  my @deps = split /\s+/, $rest;
>-  push @{$alldeps{$obj}}, @deps;
>+  push @{$alldeps{$obj}}, $rest;
>   if (DEBUG >= 2) {
>-    foreach my $dep (@deps) { print STDERR "add $obj $dep\n"; }
>+    print STDERR "add $obj $rest\n";
Oh, I meant to ask what this change does, but I overlooked it because the patch was too noisy :-(

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #8)
> (From update of attachment 451054 [details] [diff] [review])
> >-  my @deps = split /\s+/, $rest;
> >-  push @{$alldeps{$obj}}, @deps;
> >+  push @{$alldeps{$obj}}, $rest;
> >   if (DEBUG >= 2) {
> >-    foreach my $dep (@deps) { print STDERR "add $obj $dep\n"; }
> >+    print STDERR "add $obj $rest\n";
> Oh, I meant to ask what this change does, but I overlooked it because the patch
> was too noisy :-(

The reason for this change is that when you feed MSVC -showIncludes it's going to hand us back a list that includes the system headers which are generally in C:\Program Files\... (a path that has spaces).  If we split the dependencies along space boundaries then you'll end up stat-ing bizarre things and triggering a rebuild every time.

The problem is that gcc -MD puts all of the dependencies on one line so you have to split on spaces for gcc to work.  There are various ways around this, not sure which is the best yet.

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

After thinking about this some more I think the best way forward is to have mddepend do different things on msvc and gcc.  If/when we get to removing mddepend.pl we can teach cl.py to escape spaces.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

A pymake build went from 92 min to 84.5 min, an 8% speedup.

Comment 12

[[e-mail]]

(In reply to comment #9)
> The reason for this change is that when you feed MSVC -showIncludes it's going
> to hand us back a list that includes the system headers which are generally in
> C:\Program Files\... (a path that has spaces).
Why bother with system includes? mkdepend.exe doesn't.

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #12)
> (In reply to comment #9)
> > The reason for this change is that when you feed MSVC -showIncludes it's going
> > to hand us back a list that includes the system headers which are generally in
> > C:\Program Files\... (a path that has spaces).
> Why bother with system includes? mkdepend.exe doesn't.

-showIncludes gives us a list including them.  I can't think of any simple way to filter them out (trying not to slow down the gains here unnecessarily).

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

This leaves mkdepend in the tree and simply uses different mddepend.pl scripts for MSVC and GCC.

Comment 15

[Mitchell Field [:Mitch]]

(In reply to comment #14)
> Created an attachment (id=452618) [details]
> Patch
> 
> This leaves mkdepend in the tree and simply uses different mddepend.pl scripts
> for MSVC and GCC.

I got build times of ~34m29s without patch, versus ~32m34s with it, both using pymake -j2.

Comment 16

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

With some fun pymake hacks to avoid firing off another interpreter process, pymake -j2 drops from 67 min to 55m30s for me (18% win).

Comment 17

[(not currently active) Ted Mielczarek]

Comment on attachment 452618 [details] [diff] [review]
Patch

>diff -r db5fa3adde6e build/cl.py
>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/build/cl.py	Sun Jun 20 15:32:37 2010 -0700
>+from sets import Set

Python 2.4 or newer has "set" as a built-in type.

>+def InvokeClWithDependencyGeneration(cmdline):
>+  target=""
>+  # Figure out what the target is
>+  for arg in cmdline:
>+    if arg.startswith("-Fo"):
>+      target = arg[3:]

break

It probably makes sense to error after the loop if target isn't set. You can just print >>sys.stderr, "No target set" and sys.exit(1).

>+
>+  # The deps target lives here
>+  depstarget = os.path.basename(target) + ".pp"
>+
>+  cmd = ['cl', '-showIncludes'] + cmdline

Feels weird to hardcode 'cl' here, maybe you could set (in configure): CC="cl.py $(CC)" so that you get the original value of the compiler as the first arg?

>+  cl = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)

Mixing stdout and stderr here sucks. If someone does make 2>/dev/null, then they'll still get the stderr from the compiler. I don't know of an easy way to loop over all the output without using .communicate() and buffering it all, which is also crappy. I guess file this as a followup?

>+
>+  deps = Set()
set()

>+  for line in cl.stdout:
>+    if line.startswith("Note: including file:"):
>+      dep = line[21:].strip()

I'm not a fan of magic numbers, even if the string is right there.


>+  depsdir = os.path.normpath(os.path.join(os.path.dirname(target),".deps"))

Is "target" actually an absolute path? I thought we usually just passed -Fofoo.obj. If that's true, then you could just use os.path.abspath(".deps") here. (I think that will work anyway.)

>+  depstarget = os.path.join(depsdir, depstarget)
>+  if not os.path.exists(depsdir):

os.path.isdir might be more correct.

>+    try:
>+      os.makedirs(depsdir)
>+    except OSError:
>+      pass # This suppresses the error we get when the dir exists, at the cost
>+           # of masking failure to create the directory.  We'll just die on the
>+           # next line though, so it's not that much of a loss.
>+
>+  f = open(depstarget, "wt")

Is "t" actually a valid mode flag? (text mode is the default, I don't think there's a way to force it on.)

Some minor general nits:
I prefer to have spaces around operators, like "foo = 1", and spaces after commas in argument lists.

We also had a discussion on the auto-tools list about the amount of indentation to use in Python scripts, since it's all over the place in the tree currently. I'm leaning towards consistency with PEP 8, which says 4-space indents, even though I prefer 2-space. I find that I never remember to reset my emacs settings on new machines, and wind up writing 4-space indent anyway.

http://www.python.org/dev/peps/pep-0008/

>diff -r db5fa3adde6e build/win32/mddepend.pl
>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/build/win32/mddepend.pl	Sun Jun 20 15:32:37 2010 -0700

This is crappy, let's not fork this file. Either fix it the right way or do away with it somehow. (Also, just FYI, if you absolutely need to fork a file, use "hg cp" to copy it, then make your changes, and the diff will only show your changes.)

>diff -r db5fa3adde6e configure.in
>--- a/configure.in	Sat Jun 19 20:46:23 2010 -0700
>+++ b/configure.in	Sun Jun 20 15:32:37 2010 -0700
>@@ -188,8 +188,9 @@
> if test -z "$CROSS_COMPILE"; then
> case "$target" in
> *-cygwin*|*-mingw*|*-msvc*|*-mks*)
>-    if test -z "$CC"; then CC=cl; fi
>-    if test -z "$CXX"; then CXX=cl; fi
>+    _topsrcdirwin=`cd \`dirname $0\`; pwd -W`
>+    if test -z "$CC"; then CC="python -O $_topsrcdirwin/build/cl.py"; fi
>+    if test -z "$CXX"; then CXX="python -O $_topsrcdirwin/build/cl.py"; fi

You could just make this 'python -O $(topsrcdir)/build/cl.py' (note the single quotes), which will cause make to subst the $(topsrcdir) value in there. (The single quotes protect it from shell expansion.)

r- for the mddepend thing, but otherwise looks good.

Comment 18

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #17)
> It probably makes sense to error after the loop if target isn't set. You can
> just print >>sys.stderr, "No target set" and sys.exit(1).

I did this in an early draft but it breaks configure.

> >+
> >+  # The deps target lives here
> >+  depstarget = os.path.basename(target) + ".pp"
> >+
> >+  cmd = ['cl', '-showIncludes'] + cmdline
> 
> Feels weird to hardcode 'cl' here, maybe you could set (in configure):
> CC="cl.py $(CC)" so that you get the original value of the compiler as the
> first arg?

I don't really agree that it's weird to hardcode cl here since this is a wrapper over cl and is specific to its output format.  Note that there is no original value of the compiler because we replaced the hardcoded cl in configure.in with python <blah>/cl.py.
 
> >+  cl = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
> 
> Mixing stdout and stderr here sucks. If someone does make 2>/dev/null, then
> they'll still get the stderr from the compiler. I don't know of an easy way to
> loop over all the output without using .communicate() and buffering it all,
> which is also crappy. I guess file this as a followup?

I think this might be avoidable.

> >+  for line in cl.stdout:
> >+    if line.startswith("Note: including file:"):
> >+      dep = line[21:].strip()
> 
> I'm not a fan of magic numbers, even if the string is right there.

Do you have an alternative suggestion?  I don't see anyway to avoid this (we have to use the magic string somewhere).  I could comment what it is.
 
> >+  depsdir = os.path.normpath(os.path.join(os.path.dirname(target),".deps"))
> 
> Is "target" actually an absolute path? I thought we usually just passed
> -Fofoo.obj. If that's true, then you could just use os.path.abspath(".deps")
> here. (I think that will work anyway.)

I'm not sure why this is here.  Will take a look.
 
> >+  depstarget = os.path.join(depsdir, depstarget)
> >+  if not os.path.exists(depsdir):
> 
> os.path.isdir might be more correct.

Right.
 
> >+    try:
> >+      os.makedirs(depsdir)
> >+    except OSError:
> >+      pass # This suppresses the error we get when the dir exists, at the cost
> >+           # of masking failure to create the directory.  We'll just die on the
> >+           # next line though, so it's not that much of a loss.
> >+
> >+  f = open(depstarget, "wt")
> 
> Is "t" actually a valid mode flag? (text mode is the default, I don't think
> there's a way to force it on.)

Apparently not.

> Some minor general nits:
> I prefer to have spaces around operators, like "foo = 1", and spaces after
> commas in argument lists.
> 
> We also had a discussion on the auto-tools list about the amount of indentation
> to use in Python scripts, since it's all over the place in the tree currently.
> I'm leaning towards consistency with PEP 8, which says 4-space indents, even
> though I prefer 2-space. I find that I never remember to reset my emacs
> settings on new machines, and wind up writing 4-space indent anyway.
>
> http://www.python.org/dev/peps/pep-0008/

I agree with your preference :-) but I will change it so we are consistent.

> >diff -r db5fa3adde6e build/win32/mddepend.pl
> >--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> >+++ b/build/win32/mddepend.pl	Sun Jun 20 15:32:37 2010 -0700
> 
> This is crappy, let's not fork this file. Either fix it the right way or do
> away with it somehow. (Also, just FYI, if you absolutely need to fork a file,
> use "hg cp" to copy it, then make your changes, and the diff will only show
> your changes.)

We discussed on IRC using the heuristic of simply dropping files with spaces in the path, since they aren't going to be mozilla headers.  I will move to that.
 
> >diff -r db5fa3adde6e configure.in
> >--- a/configure.in	Sat Jun 19 20:46:23 2010 -0700
> >+++ b/configure.in	Sun Jun 20 15:32:37 2010 -0700
> >@@ -188,8 +188,9 @@
> > if test -z "$CROSS_COMPILE"; then
> > case "$target" in
> > *-cygwin*|*-mingw*|*-msvc*|*-mks*)
> >-    if test -z "$CC"; then CC=cl; fi
> >-    if test -z "$CXX"; then CXX=cl; fi
> >+    _topsrcdirwin=`cd \`dirname $0\`; pwd -W`
> >+    if test -z "$CC"; then CC="python -O $_topsrcdirwin/build/cl.py"; fi
> >+    if test -z "$CXX"; then CXX="python -O $_topsrcdirwin/build/cl.py"; fi
> 
> You could just make this 'python -O $(topsrcdir)/build/cl.py' (note the single
> quotes), which will cause make to subst the $(topsrcdir) value in there. (The
> single quotes protect it from shell expansion.)

The substitution needs to happen in configure or compiler checks will break.

I'll have a patch addressing everything except the points mentioned after my flight.

Comment 19

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Review comments mostly addressed.  Will followup on the target absolute path thing after landing.

Comment 20

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #19)
> Review comments mostly addressed.  Will followup on the target absolute path
> thing after landing.

After landing meant after my plane landed :-).  The target foo is necessary because of NSS.  Outside of NSS we always do -FonsFooService.obj but inside NSS we do:

make[3]: Entering directory `/c/dev/mozilla-central/security/nss/lib/util'
cl -Foc:/dev/mozilla-central/obj-i686-pc-mingw32/nss/nssutil/quickder.obj -c -Zi -Fdc:/dev/mozilla-central/obj-i686-pc-mingw32/nss/nssutil/ -Od -W3 -nologo -D_C
RT_SECURE_NO_WARNINGS -MDd -we4002 -we4003 -we4004 -we4006 -we4009 -we4013 -we4015 -we4028 -we4033 -we4035 -we4045 -we4047 -we4053 -we4054 -we4063 -we4064 -we40
78 -we4087 -we4098 -we4390 -we4551 -we4553 -we4715 -DXP_PC -DDEBUG -D_DEBUG -UNDEBUG -DDEBUG_Kyle_Huey -DWIN32 -D_X86_ -D_WINDOWS -DWIN95 -DNSS_ENABLE_ECC -DUSE
_UTIL_DIRECTLY -Ic:/dev/mozilla-central/obj-i686-pc-mingw32/dist/include/nspr -Ic:/dev/mozilla-central/obj-i686-pc-mingw32/dist/include -Ic:/dev/mozilla-central
/obj-i686-pc-mingw32/dist/public/nss -Ic:/dev/mozilla-central/obj-i686-pc-mingw32/dist/private/nss -Ic:/dev/mozilla-central/obj-i686-pc-mingw32/dist/include   "
/c/dev/mozilla-central/security/nss/lib/util/quickder.c"

If we don't deal with absolute paths we'll place the deps in the source tree in NSS which we don't want to do.

Comment 21

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Bleh, I should know better than to trust MSDN.  Even though all the docs say -showIncludes dumps to stderr, it really dumps to stdout.

Comment 22

[Mitchell Field [:Mitch]]

Comment on attachment 456149 [details] [diff] [review]
Patch

Drive-by feedback:
>+            if -1 != dep.find(' '):
Weird operand order?

>+    if ret != 0:
>+        sys.exit(ret)
>+
>+    if target == "":
>+        sys.exit(0)

if (ret != 0) || (target == ""):
    sys.exit(ret)

Comment 23

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #22)
> (From update of attachment 456149 [details] [diff] [review])
> Drive-by feedback:
> >+            if -1 != dep.find(' '):
> Weird operand order?

No it should be == instead of !=.

> >+    if ret != 0:
> >+        sys.exit(ret)
> >+
> >+    if target == "":
> >+        sys.exit(0)
> 
> if (ret != 0) || (target == ""):
>     sys.exit(ret)

Sure.

Comment 24

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #23)
> (In reply to comment #22)
> > (From update of attachment 456149 [details] [diff] [review] [details])
> > Drive-by feedback:
> > >+            if -1 != dep.find(' '):
> > Weird operand order?

Ah I see what you're saying.  I'm used to writing C++ which won't yell at you if you write if (a = 1) so I try to always write if (1 == a) because if (1 = a) doesn't compile.  That said, that's not necessary in python so I can use the normal version.

Comment 25

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 26

[(not currently active) Ted Mielczarek]

Comment on attachment 456688 [details] [diff] [review]
Patch

As discussed on IRC, I'd like this to not set CC/CXX in configure, but instead maybe set a CC/CXX_WRAPPER variable that gets used in config.mk or rules.mk, so we don't have to bother launching Python for the compiler tests we do in configure. (configure on Windows is slow enough as-is.) With that change you should be able to exit with an error from this script if "target" isn't set, as I previously mentioned.

>diff -r 053bf6cd63e9 build/cl.py
>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/build/cl.py	Sat Jul 10 11:49:17 2010 -0700
>@@ -0,0 +1,91 @@
>+# ***** BEGIN LICENSE BLOCK *****
>+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
>+#
>+# The contents of this file are subject to the Mozilla Public License Version
>+# 1.1 (the "License"); you may not use this file except in compliance with
>+# the License. You may obtain a copy of the License at
>+# http://www.mozilla.org/MPL/
>+#
>+# Software distributed under the License is distributed on an "AS IS" basis,
>+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
>+# for the specific language governing rights and limitations under the
>+# License.
>+#
>+# The Original Code is cl.py: a python wrapper for cl to automatically generate
>+# dependency information.
>+#
>+# The Initial Developer of the Original Code is
>+#   Mozilla Foundation.
>+# Portions created by the Initial Developer are Copyright (C) 2010
>+# the Initial Developer. All Rights Reserved.
>+#
>+# Contributor(s):
>+#   Kyle Huey <[e-mail]>
>+#
>+# Alternatively, the contents of this file may be used under the terms of
>+# either the GNU General Public License Version 2 or later (the "GPL"), or
>+# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
>+# in which case the provisions of the GPL or the LGPL are applicable instead
>+# of those above. If you wish to allow use of your version of this file only
>+# under the terms of either the GPL or the LGPL, and not to allow others to
>+# use your version of this file under the terms of the MPL, indicate your
>+# decision by deleting the provisions above and replace them with the notice
>+# and other provisions required by the GPL or the LGPL. If you do not delete
>+# the provisions above, a recipient may use your version of this file under
>+# the terms of any one of the MPL, the GPL or the LGPL.
>+#
>+# ***** END LICENSE BLOCK *****
>+
>+import os, os.path
>+import subprocess
>+import sys
>+
>+def InvokeClWithDependencyGeneration(cmdline):
>+    target=""

nit: spaces around the =. Also, I tend to use None as the default value of things, since it's pretty explicitly not what you want.

>+    # Figure out what the target is
>+    for arg in cmdline:
>+        if arg.startswith("-Fo"):
>+            target = arg[3:]
>+            break
>+
>+    # The deps target lives here
>+    depstarget = os.path.basename(target) + ".pp"
>+
>+    cmd = cmdline + ['-showIncludes']
>+    cl = subprocess.Popen(cmd, stdout=subprocess.PIPE)

nit: you don't really need the intermediate variable "cmd" here.

>+    ret = cl.wait()
>+    if (ret != 0) or (target == ""):

You don't need the parens here since there's no confusion of operator precedence.

>+    f = open(depstarget, "w")
>+    for dep in sorted(deps):
>+        print >>f, "%s: %s" %(target, dep)

nit: space after the % operator

Otherwise this looks fine. r=me with those nits fixed (and the moving-out-of-configure addressed)

Comment 27

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch w/nits addressed

Carrying forward r+

Asking for approval2.0+.  Risk is minimal, build perf wins are good.

Comment 28

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #26)
> Comment on attachment 456688 [details] [diff] [review]
> Patch
> 
> As discussed on IRC, I'd like this to not set CC/CXX in configure, but instead
> maybe set a CC/CXX_WRAPPER variable that gets used in config.mk or rules.mk, so
> we don't have to bother launching Python for the compiler tests we do in
> configure. (configure on Windows is slow enough as-is.) With that change you
> should be able to exit with an error from this script if "target" isn't set, as
> I previously mentioned.

NSS thwarts us here because it also invokes cl to generate an EXE.  I can either file a followup on NSS to change that and enable this, or I can set CC_WRAPPER and CXX_WRAPPER to nothing before we include config.mk into the Makefile that kicks off NSS.  Thoughts?

Comment 29

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Went with the [CC|CXX]_WRAPPER hackery.

http://hg.mozilla.org/mozilla-central/rev/6001758d1f47

Comment 30

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: C-c port

Completely untested

Comment 31

[Rain [:rain1] (inactive)]

(In reply to comment #16)
> With some fun pymake hacks to avoid firing off another interpreter process

Is this available as a patch somewhere?

Comment 32

[(not currently active) Ted Mielczarek]

He wrote those hacks before I informed him of my Pymake work (since merged to mozilla-central). It should be trivial to rewrite it as a pymake native command instead. I filed bug 585011 on that.

Comment 33

[Justin Wood (:Callek)]

Comment on attachment 462999 [details] [diff] [review]
C-c port

>+++ b/build/cl.py

Nit, lets not copy this file to c-c, and just use the m-c one.

>+    dnl cl.py provides dependency generation for MSVC
>+    CC_WRAPPER="$PYTHON -O $_topsrcdirwin/build/cl.py"
>+    CXX_WRAPPER="$PYTHON -O $_topsrcdirwin/build/cl.py"

Which we can accomplish by just making this $_topsrcdirwin/mozilla/bio;d/cl.py

Comment 34

[Justin Wood (:Callek)]

(In reply to comment #33)
> Which we can accomplish by just making this $_topsrcdirwin/mozilla/bio;d/cl.py

(without the typo ;-) )

Comment 35

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 462999 [details] [diff] [review]
C-c port

Going to check in a combined version of this and the pymake native command implementation at a later date.

Comment 36

[Justin Wood (:Callek)]

Pushed as: http://hg.mozilla.org/comm-central/rev/168507b57522 and http://hg.mozilla.org/comm-central/rev/c6fcb582b885 Though still added the cl.py to c-c. (and with that followup to address my nit, its unused)

Comment 37

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #36)
> Pushed as: http://hg.mozilla.org/comm-central/rev/168507b57522 and
> http://hg.mozilla.org/comm-central/rev/c6fcb582b885 Though still added the
> cl.py to c-c. (and with that followup to address my nit, its unused)

http://hg.mozilla.org/comm-central/rev/a38d5248ae05 to remove that.