604263 - NSPR parallel building chokes on debug windows

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Description

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

Attachment: Patch

PDB files don't have an explicit target in the NSPR makefiles.  Thus, when doing a parallel debug build on windows make will error complaining that there is no target to build the PDB.  This is not an issue with -j1 because the PDB dependency is after the SHARED_LIB dependency, so by the time the shared library has finished the PDB has been generated as a byproduct.

The solution is to make a PDB target that depends explicitly on the shared library.

I missed this because | make clean | in mozilla's tree calls make clean in NSPR which doesn't remove the generated binaries/pdbs/etc.  I filed Bug 604260 to fix that.

Comment 1

[Wan-Teh Chang]

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

Thanks for the patch.

>+# PDBs need to depend on the shared lib to order dependencies properly
>+$(SHARED_LIB_PDB):: $(SHARED_LIB)

BUG: SHARED_LIB is a typo.  It should be SHARED_LIBRARY.

Use the single colon (:) rule here.

I suggest you add the rule here, next to the similar rule for $(IMPORT_LIBRARY):
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/config/rules.mk&rev=3.77&mark=309#308

Comment 2

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

Attachment: Patch

Ah, good catch.  Comments addressed.

Comment 3

[Wan-Teh Chang]

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

r=wtc.  Thanks.

Do you have a good way to test this?  How did the
previous incorrect patch pass your tests?

Comment 4

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

See the last bit of comment 0.  I missed this previously because I wasn't building in a completely clean object dir.  If you have a clean objdir and build with a reasonable -jN (I'm using -j4) you'll hit it every time.

Comment 5

[(not currently active) Ted Mielczarek]

Checking in config/rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.78; previous revision: 3.77
done

Comment 6

[Wan-Teh Chang]

(In reply to comment #4)
I know.  I'm just wondering why your previous fix with a typo:
    >+$(SHARED_LIB_PDB):: $(SHARED_LIB)
passed your testing.

Comment 7

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

Because the actual target is irrelevant, all that matters is that we have a target for make to find.  It won't try to install anything until all the dependencies have finished, at which point the PDB has been generated.

Comment 8

[[e-mail]]

Note that this still breaks with VC7.1 debug or earlier, since it can't open the PDB file in parallel, and nspr's rules.mk doesn't have the .NOTPARALLEL ifdef.

Comment 9

[Wan-Teh Chang]

khuey: should we turn off NSPR parallel builds for VC7.1
or earlier?

Comment 10

[Wan-Teh Chang]

khuey: can you fix the problem reported by neil in comment 8?
Thanks.

Comment 11

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

Ah, sorry, that bugmail slipped through the cracks somehow.  I'll get to it soon.

Comment 12

[Wan-Teh Chang]

khuey: ping.

neil: do you know how to fix the problem you reported in
comment 8?  It seems that we just need to add
.NOTPARALLEL.

Comment 13

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

Attachment: Patch (add .NOTPARALLEL)

This should be all that's needed, but I don't have an old enough compiler to test.

Comment 14

[Wan-Teh Chang]

Comment on attachment 498134 [details] [diff] [review]
Patch (add .NOTPARALLEL)

r=wtc.

neil: could you please test this patch?

>+# Disallow parallel builds with MSVC < 8

khuey: The comment should explain why.  You can just
quote what neil said in comment 8:
  Parallel builds break with VC7.1 debug or earlier,
  since it can't open the PDB file in parallel.

Comment 15

[[e-mail]]

Comment on attachment 498134 [details] [diff] [review]
Patch (add .NOTPARALLEL)

Yes, this works (although I actually use a slightly enhanced version myself).

Comment 16

[Wan-Teh Chang]

neil: could you contribute your patch?  Thanks.

Comment 17

[Wan-Teh Chang]

Attachment: 498134: Patch (add .NOTPARALLEL) v2, by Kyle Huey

I edited khuey's a bit and checked it in on the NSPR trunk
(NSPR 4.8.7).

Checking in rules.mk;
/cvsroot/mozilla/nsprpub/config/rules.mk,v  <--  rules.mk
new revision: 3.79; previous revision: 3.78
done