Closed
Bug 550571
Opened 14 years ago
Closed 14 years ago
NSPR configure uses host lib tool for windows ce and doesn't produce usable debug symbols
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: blassey, Assigned: blassey)
References
Details
Attachments
(2 files, 1 obsolete file)
3.16 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
14.41 KB,
patch
|
wtc
:
review+
|
Details | Diff | Splinter Review |
you can see here that its hard coding AR to lib, which is the x86 version: http://mxr.mozilla.org/nspr/source/nsprpub/configure.in#1653 This causes a link error when any arm specific options are passed in (and possibly hidden errors we don't know about). Also we only set the appropriate debug linker flags for when MOZ_DEBUG_SYMBOLS is set and we only set the appropriate debug compiler flags when MOZ_DEBUG is set. Having one without the other produces nothing of use.
Attachment #430716 -
Flags: review?(wtc)
Updated•14 years ago
|
Summary: NSPR configure uses host lib tool for windows ce and doesn't produce usible debug symbols → NSPR configure uses host lib tool for windows ce and doesn't produce usable debug symbols
Comment 1•14 years ago
|
||
Comment on attachment 430716 [details] [diff] [review] patch >- if test -n "$MOZ_DEBUG_SYMBOLS"; then >+ if test -n "$MOZ_DEBUG_SYMBOLS" || test -n "$MOZ_DEBUG"; then > OS_LDFLAGS='-DEBUG -DEBUGTYPE:CV' > OS_DLLFLAGS='-DEBUG -DEBUGTYPE:CV' > DSO_LDOPTS='-DEBUG -DEBUGTYPE:CV' >+ _DEBUG_FLAGS=-Zi >+ if test -z "$MOZ_DEBUG"; then >+ CFLAGS="$CFLAGS -Zi" >+ fi > fi Brad, why do you add -Zi to both _DEBUG_FLAGS and CFLAGS? Later in mozilla/nsprpub/configure.in, we have 2729 if test -n "$MOZ_DEBUG"; then 2730 CFLAGS="$CFLAGS $_DEBUG_FLAGS" 2731 CXXFLAGS="$CXXFLAGS $_DEBUG_FLAGS" 2732 fi Perhaps we should change that code to the following? 2729 if test -n "$MOZ_DEBUG_SYMBOLS" || test -n "$MOZ_DEBUG"; then 2730 CFLAGS="$CFLAGS $_DEBUG_FLAGS" 2731 CXXFLAGS="$CXXFLAGS $_DEBUG_FLAGS" 2732 fi You should have Ted Mielczarek (:luser) review the MOZ_DEBUG portions of this patch because they're closely related to bug 338224 (see especially bug 338224 comment 53). Do you want me to check in the AR_FLAGS portion of this patch first?
Comment 2•14 years ago
|
||
Ted: this bug reports two independent issues. I believe the second issue is a duplicate of bug 338224. Could you take a look?
Assignee | ||
Comment 3•14 years ago
|
||
Your proposed change would work just as well and is probably cleaner. I'll attach a patch to do that and get Ted's review. In the mean time, please do land the AR/AR_FLAGS portion of this patch as it will unblock bug 550319.
Blocks: 550319
Assignee | ||
Comment 4•14 years ago
|
||
Attachment #430716 -
Attachment is obsolete: true
Attachment #431188 -
Flags: review?(ted.mielczarek)
Attachment #430716 -
Flags: review?(wtc)
Assignee | ||
Updated•14 years ago
|
Attachment #430716 -
Attachment is obsolete: false
Assignee | ||
Comment 5•14 years ago
|
||
Attachment #430716 -
Attachment is obsolete: true
Attachment #431190 -
Flags: review?(wtc)
Assignee | ||
Updated•14 years ago
|
Attachment #431188 -
Attachment is patch: true
Attachment #431188 -
Attachment mime type: application/octet-stream → text/plain
Comment 6•14 years ago
|
||
Comment on attachment 431190 [details] [diff] [review] patch, AR/AR_FLAGS (checked in) r=wtc. I checked in the patch on the NSPR trunk (NSPR 4.8.5). Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.269; previous revision: 1.268 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.273; previous revision: 1.272 done
Attachment #431190 -
Attachment description: patch, AR/AR_FLAGS → patch, AR/AR_FLAGS (checked in)
Attachment #431190 -
Flags: review?(wtc) → review+
Assignee | ||
Comment 7•14 years ago
|
||
Wan-Teh, (same question as always) are you planning on doing a RTM tag in the near term?
Comment 8•14 years ago
|
||
Brad: No. I just released NSPR_4_8_4_RTM. If you need this change for the Mozilla trunk only, I can push a non-RTM tag to mozilla-central this week.
Assignee | ||
Comment 9•14 years ago
|
||
trunk only is fine by me, if we can get the debug symbols patch included as well that would be great.
Comment 10•14 years ago
|
||
Good. Ted or I will push a new tag to mozilla-central that includes both patches.
Updated•14 years ago
|
Assignee: wtc → bugmail
Updated•14 years ago
|
Attachment #431188 -
Flags: review?(ted.mielczarek) → review+
Comment 11•14 years ago
|
||
Comment on attachment 431188 [details] [diff] [review] patch, debug symbols Checking in configure; /cvsroot/mozilla/nsprpub/configure,v <-- configure new revision: 1.271; previous revision: 1.270 done Checking in configure.in; /cvsroot/mozilla/nsprpub/configure.in,v <-- configure.in new revision: 1.275; previous revision: 1.274 done
Comment 12•14 years ago
|
||
I'll tag and land to mozilla-central.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•14 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/rev/9f7d41c42cf4
You need to log in
before you can comment on or make changes to this bug.
Description
•