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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: abwillis1, Assigned: bryner)
Details
Attachments
(3 files)
475 bytes,
patch
|
Details | Diff | Splinter Review | |
438 bytes,
patch
|
Details | Diff | Splinter Review | |
2.95 KB,
patch
|
mkaply
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•20 years ago
|
||
Reporter | ||
Comment 2•20 years ago
|
||
Reporter | ||
Updated•20 years ago
|
Attachment #167570 -
Attachment description: Corrects const char* to char* conversion error logger.cpp → Corrects const void* to void* conversion error logger.cpp
Reporter | ||
Comment 4•20 years ago
|
||
Adding Mike and Peter to see if there is a better way to do this.
Comment 5•20 years ago
|
||
I don't think there is.
Comment 6•20 years ago
|
||
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.)
Comment 7•20 years ago
|
||
Are you using a later GCC? I get warnings for these but they don't stop my build....
Reporter | ||
Comment 8•20 years ago
|
||
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.
Reporter | ||
Comment 9•20 years ago
|
||
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.
Comment 10•20 years ago
|
||
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
Comment 11•20 years ago
|
||
Knut, Here are some more errors that only show up with the new GCC.
Comment 12•20 years ago
|
||
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
Comment 13•20 years ago
|
||
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
Comment 14•20 years ago
|
||
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).
Comment 15•20 years ago
|
||
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.
Reporter | ||
Comment 16•20 years ago
|
||
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.
Comment 17•20 years ago
|
||
(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...
Comment 18•20 years ago
|
||
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
Comment 19•20 years ago
|
||
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
Comment 20•20 years ago
|
||
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 → ---
Comment 21•20 years ago
|
||
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)
Updated•20 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 22•19 years ago
|
||
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+
Comment 23•19 years ago
|
||
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?
Comment 24•19 years ago
|
||
I'll fix it when I check in. tomorrow. really.
Comment 25•19 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 20 years ago → 19 years ago
Resolution: --- → FIXED
Updated•5 years ago
|
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.
Description
•