963090 - Compilation error while compiling DEBUG BUILD of Firefox (C-C) rbnf.cpp, ucbuf.c

Status: RESOLVED INCOMPLETE

(MailNews Core :: Build Config, defect)

Description

[ISHIKAWA, Chiaki]

Attachment: Patch to comment out problematic section by |#if 0| (rbnf.cpp, ucbuf.c)

I tried to create local DEBUG BUILD of  Firefox binary by running
|mozilla/mach build|
from C-C tree (this is supposed to work, and indeed it worked after the patch
reported here.)

Somehow during the compilation process, two files produced syntax errors in
#ifdef DEBUG, #endif section each.

File 1:
intl/icu/source/i18n/rbnf.cpp
In a #ifdef DEBUG section, |str| is not found.
(notice that somehow the argumet|str| is commented out in
the function parseError().

File 2:
intl/icu/source/tools/toolutil/ucbuf.c
In a #if DEBUG section, |numRead| is not declared.
The code does not seem to do anything.

In both cases, nothing useful seems to be done.
So I am attaching a patch to change |#ifdef DEBUG|, |#if DEBUG|
to |#if 0|.

I have no idea what local configuration makes this
section being compiled and produce error(s) while M-C tree also has the same issue (I just checked about 30 minutes ago using mxr.mozilla.org.)

Anyway, after this change, I could produce working firefox binary and
am impressed at the ease of invoking various tests using |mach| command.
Unfortunately, thunderbird does not have such a useful single front end, and thus we need to invoke |make mozmill| for mozmill test suite, and
running valgrind is so difficult, I had to create a small wrapper binary that
is invoked as |thunderbird-bin| but actualy invokes thunderbird below valgrind with suitable parameters. I wish |mach| could be improved for TB after C-C is merged into M-C tree.

I wonder if I am using default Japanese locale for the account where
compilation took place has anything to do with the error.
What puzzles me is why compiling debug build of FF using M-C does not cause a compilation error like the ones what I saw(?) 

TIA

Comment 1

[ISHIKAWA, Chiaki]

|I wonder if I am using default Japanese locale for the account where
|compilation took place has anything to do with the error.
Correction: should be clear, but here it goes.
=>
I wonder if my using default Japanese locale for the account where
compilation took place has anything to do with the error.

Comment 2

[Masatoshi Kimura [:emk]]

Comment on attachment 8364349 [details] [diff] [review]
Patch to comment out problematic section by |#if 0| (rbnf.cpp, ucbuf.c)

This is an imported code. Please fix the bug upstream or change the c-c configuration so that it doesn't spew the error.

Comment 3

[ISHIKAWA, Chiaki]

(In reply to Masatoshi Kimura [:emk] from comment #2)
> Comment on attachment 8364349 [details] [diff] [review]
> Patch to comment out problematic section by |#if 0| (rbnf.cpp, ucbuf.c)
> 
> This is an imported code. Please fix the bug upstream or change the c-c
> configuration so that it doesn't spew the error.

Thank you for the info.

I followed the directory tree to "icu" and figured that this is from ICU package.

One problem: mozilla source code has ICU 50 version.

But the original ICU source code has moved to
Version: 52.1, Release Date: 2013-10-09
Please see
http://site.icu-project.org/download

I will check if the latger 52.1 version has already fixed the issue. If so, ICU developers may not
have a motivation to fix the older 50.x.y version.

I will also check WHY specifing --enable-debug in C-C caused the compiler to 
touch the broken code in #if DEBUG, #ifdef DEBUG section while, presumably
building debug build in M-C does not seem to cause the compile error(s).
(I believe DEBUG version of FF is built, isn't it? If not, then I can understand the
broken code remaining in the source code.)

TIA

Comment 4

[ISHIKAWA, Chiaki]

(In reply to ISHIKAWA, Chiaki from comment #3)
> (In reply to Masatoshi Kimura [:emk] from comment #2)
> > Comment on attachment 8364349 [details] [diff] [review]
> > Patch to comment out problematic section by |#if 0| (rbnf.cpp, ucbuf.c)
> > 
> > This is an imported code. Please fix the bug upstream or change the c-c
> > configuration so that it doesn't spew the error.
> 
> Thank you for the info.
> 
> I followed the directory tree to "icu" and figured that this is from ICU
> package.
> 
> One problem: mozilla source code has ICU 50 version.
> 
> But the original ICU source code has moved to
> Version: 52.1, Release Date: 2013-10-09
> Please see
> http://site.icu-project.org/download

The current trunk source of site.icu-project.org has fixed one of the issues, i.e., rbnf.cpp seems to be fixed.

http://source.icu-project.org/repos/icu/icu/trunk/source/i18n/rbnf.cpp

But the problem with  ucbuf.c still remains.
http://source.icu-project.org/repos/icu/icu/trunk/source/tools/toolutil/ucbuf.c

> 
> I will check if the latger 52.1 version has already fixed the issue. If so,
> ICU developers may not
> have a motivation to fix the older 50.x.y version.

Anyway, I filed a bug at ICU bug trac system.
http://bugs.icu-project.org/trac/ticket/10856

Amazing, it did not ask for user registration at all! :-)

> 
> I will also check WHY specifing --enable-debug in C-C caused the compiler to 
> touch the broken code in #if DEBUG, #ifdef DEBUG section while, presumably
> building debug build in M-C does not seem to cause the compile error(s).
> (I believe DEBUG version of FF is built, isn't it? If not, then I can
> understand the
> broken code remaining in the source code.)

It was not the issue of my local ccache, etc.
But basically, for whatever the reason,
comm-central thunderbird compilation does not touch the mentioned two files during build.
(At least with my mozconfig.)

However, inside comm-central, there is a copy of m-c as "./mozilla" under the top-level of
comm-central source.
If I do the following, the build DOES try to compile the two files and thus I saw errors.
 cd mozilla
 ./mach build

(my mozconfig specified with debug and I also specify -DDEBUG on the compilation command line explicitly.
I forgot why I did this, but in the last 4 or 5 years, I think there was a time when -DDEBUG was missing
in a compilation of a certain directories due to build system bugs or something.)

Anyway, I hope this helps us to have a less buggy upstream source files.

In the meanwhile, I guess I would live with apolying a local patch.
(It seems that I am probably the only developer who, for some historical reason,
has -DDEBUG on my compilation command line macro definition before ./configure.)

TIA

Comment 5

[Masatoshi Kimura [:emk]]

OK, please put the local patch under intl/icu-patches/, create a new patch with the local patch included and request the review again.

Comment 6

[ISHIKAWA, Chiaki]

(In reply to Masatoshi Kimura [:emk] from comment #5)
> OK, please put the local patch under intl/icu-patches/, create a new patch
> with the local patch included and request the review again.

Thank you for your comment. Will do shortly.

Comment 7

[ISHIKAWA, Chiaki]

(In reply to ISHIKAWA, Chiaki from comment #6)
> (In reply to Masatoshi Kimura [:emk] from comment #5)
> > OK, please put the local patch under intl/icu-patches/, create a new patch
> > with the local patch included and request the review again.
> 
> Thank you for your comment. Will do shortly.

Sorry, obviously I forgot to create the patch when my machine broke down early last summer and I could not do much development for about two months.

In the meantime, the remaining bug in the source line has been taken care of by the original ICU source file,

http://source.icu-project.org/repos/icu/icu/trunk/source/tools/toolutil/ucbuf.c

in this source, and another one that caused breakage when I defined -DDEBUG=1,
new macros UCBUF_DEBUG (and RBNF_DEBUG) are used instead of the simple DEBUG.

Thus it would be only a matter of incorporating new ICU source files.

TIA