Closed
Bug 485837
Opened 15 years ago
Closed 15 years ago
vc90.pdb files are output in source directory instead of OBJDIR
Categories
(NSS :: Build, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.12.3
People
(Reporter: julien.pierre, Assigned: nelson)
Details
Attachments
(1 file)
829 bytes,
patch
|
julien.pierre
:
review+
|
Details | Diff | Splinter Review |
When building with MSVC 9 (Visual studio 2008), one has to define MOZ_DEBUG_SYMBOLS since /PDB:NONE isn't supported. But then, the compiler generates PDB files in the source directory instead of OBJDIR . Even though I don't need the PDB files, I ran into the following build race condition while simultaneously building 32-bit and 64-bit binaries on Windows : c:/NSS/tip/mozilla/security/nss/lib/freebl/loader.c : fatal error C1033: cannot open program database 'c:\nss\tip\mozilla\security\nss\lib\freebl\vc90.pdb' make[2]: *** [WINNT6.0_64_OPT.OBJ/loader.obj] Error 2 make[2]: Leaving directory `/c/NSS/tip/mozilla/security/nss/lib/freebl' make[1]: *** [libs] Error 2 make[1]: Leaving directory `/c/NSS/tip/mozilla/security/nss/lib' make: *** [libs] Error 2 That's because two concurrent freebl builds were being done, and they each needed access to the vc90.pdb . It means the PDB file is being overwritten by each build. IMO, this should be resolved , even for non-concurrent builds. If some customer asks us for PDB files, it will only be the PDB from the last build. There seems to be other, named, PDB files output in OBJDIR, though. Perhaps we don't need those VC90.PDB files at all, and we need to turn off their generation altogether.
Assignee | ||
Comment 1•15 years ago
|
||
There's a trivial fix to this that puts the pdb files in the objdir directory. It's a one-line change to coreconf. I have it in some of my workareas. Let me find it.
Assignee | ||
Comment 2•15 years ago
|
||
Actually, it should no longer be true that it is necessary to define MOZ_DEBUG_SYMBOLS to prevent PDB:NONE. Wan-Teh committed a change to coreconf to detect the version of MSVC and do the right thing. (Well, Wan-Teh wrote such a patch, and I reviewed it. I assume it was committed.)
Reporter | ||
Comment 3•15 years ago
|
||
Nelson, I believe the autodetection is for nspr only with autoconf. I don't think it works with coreconf. The environment variable is still needed for nss.
Assignee | ||
Comment 4•15 years ago
|
||
Julien, Wan-Teh and I discussed the patch for cl version detection in several bugs. The bug for NSS is Bug 478563. The patch attached to that bug was not committed because it was a "work in progress" and several suggested changes appear in the comments. The patch attached here should fix the problem with the location of the vcx0.pdb files. Please test it in a clobber build.
Attachment #369943 -
Flags: review?(julien.pierre.boogz)
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → nelson
Assignee | ||
Comment 5•15 years ago
|
||
Comment on attachment 369943 [details] [diff] [review] patch v1 I tested this patch in both debug and optimized builds. Looks good to me.
Attachment #369943 -
Attachment description: untested patch v1 → patch v1
Reporter | ||
Comment 6•15 years ago
|
||
Comment on attachment 369943 [details] [diff] [review] patch v1 Nelson, I will test this patch, but I have two questions : 1) is -Fd$(OBJDIR)/ supported in all versions of MSVC (including VC6) ? We don't require VC6 support on the trunk at Sun, but I don't know if anyone else needs it. 2) In the second hunk, I note that the patch was already there, but commented. What was the reason for it ? Also, -Z7 got replaced with -Zi. Will this cause issues with compilers older than VC9 ?
Reporter | ||
Comment 7•15 years ago
|
||
Comment on attachment 369943 [details] [diff] [review] patch v1 I tested this patch and it's fine for VC9 (MSVC 2008). I don't know if it's got any unwanted side effects with previous versions of MSVC which is why I haven't given r+ yet. I may be able to test VC6 in the office tomorrow, but I don't have anything else in between VC6 and VC9 to test. We should clarify whether we need pre-VC9 support on the NSS trunk, and if so, for which versions.
Assignee | ||
Comment 8•15 years ago
|
||
VC8 builds fine with this patch. I don't know about VC6, but don't care much about it, either. Mozilla requires VC8 for FF on Windows. And since Sun uses VC9 now, I see no reason to continue worrying about VC6.
Reporter | ||
Comment 9•15 years ago
|
||
Comment on attachment 369943 [details] [diff] [review] patch v1 I agree we don't need vc6 support on the trunk. Vc7.x shouldn't be needed either.
Attachment #369943 -
Flags: review?(julien.pierre.boogz) → review+
Reporter | ||
Comment 10•15 years ago
|
||
One more thing, please remove the comment before checkin.
Assignee | ||
Comment 11•15 years ago
|
||
Checking in WIN32.mk; new revision: 1.34; previous revision: 1.33
Status: NEW → RESOLVED
Closed: 15 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.12.3
You need to log in
before you can comment on or make changes to this bug.
Description
•