Closed Bug 272624 Opened 20 years ago Closed 19 years ago

Build bustage on trunk of Firefox for OS/2

Categories

(Firefox Build System :: General, defect)

x86
OS/2
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: abwillis1, Assigned: bryner)

Details

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.8a6) Gecko/20041127
Build Identifier: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.8a6) Gecko/20041127

Building Firefox from Trunk on OS/2 fails in logger.cpp and loghlp.cpp in
modules\plugin\tools\tester\common
This did not fail day before yesterday and hasn't been changed according to
lxr.mozilla.org since April.  Assuming it is related to checkin from bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=272189
which may have changed what is being built.
Including patch.

Reproducible: Always
Steps to Reproduce:
1.  Pull source from trunk for firefox.
2.  Attempt to build firefox on OS/2.
3.

Actual Results:  
Build failure.
(Affects OS/2 and maybe MingW.)

Expected Results:  
Build completes with no failures.
Setting trunk on Version.
Version: unspecified → Trunk
Attachment #167570 - Attachment description: Corrects const char* to char* conversion error logger.cpp → Corrects const void* to void* conversion error logger.cpp
Adding Mike and Peter to see if there is a better way to do this.
I don't think there is. 
I think you can fill less of the fields in the bug entry form if you  
post enough error messages from the compile failure. I currently have
little time to build Firefox, so I don't know what the error could be.
Too bad, that we don't have a FF tinderbox for OS/2! What you correct
in your patches should normally only be reported as warnings not
errors that result in build failure. Bug 272189 is unlikely to be the
cause as it only changes what was normallly used as build
configuration.

(Btw, if you add patches, it makes more sense to use
   cvs diff -u filename(s) > file.diff
or perhaps the same with -u8. That gives you a diff in the standard   
format for all files, and you only need to attach once.)
Are you using a later GCC? I get warnings for these but they don't stop my build....
Yes, I am using 3.3.5 and not 3.2.2.  Just tried with 3.2.2 and it worked.  Last
time I saw these type errors I tested both and got the error on both so I didn't
realize there might be a difference this time.
For the record:
E:/cvs/work/mozilla/modules/plugin/tools/tester/common/logger.cpp:291: error: in
valid
   conversion from `const void*' to `void*'
E:/cvs/work/mozilla/modules/plugin/tools/tester/common/logger.cpp: In member
   function `void CLogger::dumpLogToTarget()':
E:/cvs/work/mozilla/modules/plugin/tools/tester/common/logger.cpp:317: warning:
unused
   variable `int iLength'
make.exe[1]: *** [logger.o] Error 1
make.exe[1]: Leaving directory `E:/cvs/work/mozilla/ffobj/modules/plugin/tools/t
ester/common'
make.exe: *** [all] Error 2
I don't know why it suddenly started being a problem if it wasn't the bug
specified as the builds I said worked were with 3.3.5 as well.
I brought some of these up with bird previously and he needs to fix them in the
compiler. The new GCC shouldn't be producing more errors than it did before.
Status: UNCONFIRMED → RESOLVED
Closed: 20 years ago
Resolution: --- → INVALID
Knut,

Here are some more errors that only show up with the new GCC.
Arg. I just ignored this as some kind of weird annoyance when I encountered it
last week. But somethings seems messed up and for some really weird reasons - my
attempt on a simple testcase for it didn't work out. I'll take a closer look
later tonight.

Kind Regards,
 knut
I've managed to reduce these two problems to:

unsigned NPN_Write(void* buffer);
void clearTarget(void)
{
    NPN_Write("\n");
}

char * makeLogItemStruct(char *dw3)
{
    char* str = dw3 ? dw3 : "";
    return str;
}

Both the litterals will cause g++ to say:
error: invalid conversion from `const char*' to `char*'

I've verified that the above code will cause errors with the follwoing GCC versions:
  3.2.2 (OS/2)
  3.3.3 (mingw32)
  3.3.3 (OS/2)
  3.3.4 (Linux x86)
  3.3.5 (OS/2)
  3.4.2 (FreeBSD amd64)
  3.4.2 (FreeBSD x86)

OTOH, Visual C++ 7.0, Visual Age for C++ 3.08, Visual Age for C++ 3.6.5 and
Watcom C/C++ v11.0c don't say anything when subjected to the code.

Since so many GCC versions are broken/annoyingly-strict I don't see a big
argument whether this should be patched.


BTW. Some questions naturally rises from this issue. Are the OS/2 guys the only
ones actually compiling the tests regularly? Or do we just happen to be using
odd GCC version?  What GCC versions are used on other platforms?

Kind Regards,
 knut
FYI. On 3.2.2 the -pedantic option will convert the errors to a warning, that's
why I get errors when building it with no options (g++ -c moz-test-char.cpp).
The warnig is correct, because if you tried to modify "" the behaviour is
undefined - it'll crash on unix and I beveli that msvc will put " into a
readonly area too.

Not 100% sure if it should be an error, mind you.
In a roundabout way the bug I referenced was the cause.  I synced my .mozconfig
files and didn't put --disable-tests back in on my first run.  After fixing the
error and finishing the build I found all the test stuff and put it back in but
hadn't linked the 2 in my mind so normally even the OS/2 guys aren't building
the tests I don't think.  
(In reply to comment #13)
> I've verified that the above code will cause errors with the follwoing GCC
versions:
>   3.2.2 (OS/2)
>   3.3.3 (mingw32)
>   3.3.3 (OS/2)
>   3.3.4 (Linux x86)
>   3.3.5 (OS/2)
[...]
> Since so many GCC versions are broken/annoyingly-strict I don't see a big
> argument whether this should be patched.

As you noticed later yourself, gcc is called with several arguments for
compiling loglph.cpp, on my build these are

g++ -o loghlp.o -c -DOSTYPE=\"OS22\" -DOSARCH=\"OS2\"
-Im:/moztrunk/mozilla/modules/plugin/tools/tes

ter/common/../include  -I../../../../../dist/include/java
-I../../../../../dist/include/plugin -I../

../../../../dist/include/plugin -I../../../../../dist/include
-Im:/moztrunk/obj_noMN/dist/include/ns

pr    -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align
-Woverloaded-virtual

 -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pedantic
-Zomf -pipe  -DNDEBUG

 -DTRIMMED -O2 -s   -DMOZILLA_CLIENT -include ../../../../../mozilla-config.h
-Uunix -U__unix -U__un

ix__ -Wp,-MD,.deps/loghlp.pp
m:/moztrunk/mozilla/modules/plugin/tools/tester/common/loghlp.cpp

And there I do see a difference between GCC 3.2.2 and GCC 3.3.5 beta on OS/2,
where the latter gives an error

loghlp.cpp:517: error: invalid conversion from `const char*' to `char*'

while the former just gives a warning for the same code. Unless this reflects a
difference in the GCC development (which I doubt becaue many people build
Mozilla without --disable-tests and with GCC 3.3.x and 3.4.x) this should be
changed in the OS/2 version.

Btw, I do build the tests from time to time (so does the tinderbox on the
Seamonkey-Ports page) but as this just gives a warning with the "standard" GCC
3.2.2 it was never noticed...
Which tinderbox are you talking about, the OS/2 one? The OS/2 one is of little
interest now. The question is if anyone else is using GCC 3.3.1+ and building tests.

And, of course, I did test with all the flags, and found that only the -pedantic
one + 3.2.2 made any difference, as I've already reported. Btw. the -pedantic
error => warning problem apparently was fixed in GCC 3.3.1 and GCC 3.4.0:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10032

Any chance of that a linux developer/buildguy could verify that the tests are
broken on GCC 3.3.1+? There must be someone who tries to build the tests once in
a while...

Kind Regards,
 knut
I was talking about the OS/2 tinderbox, but the same is true for the other
tinderboxes, e.g. "brad" on
http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey uses gcc 3.3.5 on
Linux and its .mozconfig does not seem to include --disable-tests. (I would
guess that all tinderboxes do build the tests because they need the tests to
check that the compile completed successfully.) Additionally, the box "balsa" on
the SeaMonkey-Ports page uses gcc 3.4.

Wait, the logs of these builds never contain anything about plugin/tools/tester.
Ah, that dir ever only gets used on OS/2, modules/plugin/Makefile.in contains:

 76 ifeq ($(OS_ARCH),OS2)
 77 DIRS            += samples/default/os2
 78 ifdef ENABLE_TESTS
 79 DIRS            += tools/tester/common tools/tester/os2
 80 endif
 81 endif
I didn't think about that.

You're right, tester is a Windows OS/2 thing. I'll get this patch in.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Mike, AFAICS you haven't checked anything in yet. I created this patch from
Andy's two patches and added fixes for three more warnings that also appear
with gcc322 when compiling modules/plugin/tools/tester/common/. I still get 

m:/trunk/mozilla/modules/plugin/tools/tester/include/log.h: In
   destructor `LogArgumentStruct::~LogArgumentStruct()':
m:/trunk/mozilla/modules/plugin/tools/tester/include/log.h:58: warning:
deleting
   `void*' is undefined

everytime but this would require more changes througout (although I this
warning seems to make sense). While I hope that the extras do not break
anything on Windows (if it is really still used there), feel free to just use
Andy's patches and ignore the extra stuff...
Attachment #170294 - Flags: review?(mkaply)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 170294 [details] [diff] [review]
Fix all gcc335 errors + a few warnings

>       // strlen not protected from 0!
>-      char* str = dw3 ? (LPSTR)dw3 : "";
>+      char* str = dw3 ? (LPSTR)dw3 : (char*)"";
>       plis->arg3.iLength = strlen(str) + 1;

For perfection sake, that LPSTR cast should probably be a char* cast

otherwise, r=mkaply
Attachment #170294 - Flags: review?(mkaply) → review+
Yes, you are right. Do I need someone's sr for this (it seems previous checkins
were not reviewed at all)? Otherwise can you make that small change when
checking in or do you want me to upload a perfected patch?
I'll fix it when I check in. tomorrow. really.
Fix checked in.
Status: NEW → RESOLVED
Closed: 20 years ago19 years ago
Resolution: --- → FIXED
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: