Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Description • 14 years ago

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.

	Wan-Teh Chang
	Comment 1 • 14 years ago

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

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 2 • 14 years ago

Ah, good catch.  Comments addressed.

	Wan-Teh Chang
	Comment 3 • 14 years ago

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?

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 4 • 14 years ago

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.

	(not currently active) Ted Mielczarek
	Comment 5 • 14 years ago

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

	Wan-Teh Chang
	Comment 6 • 14 years ago

(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.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 7 • 14 years ago

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.

	neil@parkwaycc.co.uk
	Comment 8 • 14 years ago

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.

	Wan-Teh Chang
	Comment 9 • 14 years ago

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

	Wan-Teh Chang
	Comment 10 • 14 years ago

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

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 11 • 14 years ago

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

	Wan-Teh Chang
	Comment 12 • 14 years ago

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.

	Kyle Huey (Exited; not receiving bugmail, old account, do not use) Assignee
	Comment 13 • 14 years ago

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

	Wan-Teh Chang
	Comment 14 • 14 years ago

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.

	neil@parkwaycc.co.uk
	Comment 15 • 14 years ago

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

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

	Wan-Teh Chang
	Comment 16 • 14 years ago

neil: could you contribute your patch?  Thanks.

	Wan-Teh Chang
	Comment 17 • 14 years ago

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