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)

ARM
Windows Mobile 6 Professional
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: blassey, Assigned: blassey)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached patch patch (obsolete) — 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)
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 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?
Ted: this bug reports two independent issues.  I believe the second issue
is a duplicate of bug 338224.  Could you take a look?
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
Attachment #430716 - Attachment is obsolete: true
Attachment #431188 - Flags: review?(ted.mielczarek)
Attachment #430716 - Flags: review?(wtc)
Attachment #430716 - Attachment is obsolete: false
Attachment #430716 - Attachment is obsolete: true
Attachment #431190 - Flags: review?(wtc)
Attachment #431188 - Attachment is patch: true
Attachment #431188 - Attachment mime type: application/octet-stream → text/plain
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+
Wan-Teh, (same question as always) are you planning on doing a RTM tag in the near term?
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.
trunk only is fine by me, if we can get the debug symbols patch included as well that would be great.
Good.  Ted or I will push a new tag to mozilla-central that includes
both patches.
Assignee: wtc → bugmail
Attachment #431188 - Flags: review?(ted.mielczarek) → review+
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
I'll tag and land to mozilla-central.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: