Closed Bug 372878 Opened 17 years ago Closed 17 years ago

reduce the number of AC_TRY_RUN tests in configure.in

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

(Keywords: fixed1.8.1.4, Whiteboard: [patch])

Attachments

(2 files, 1 obsolete file)

We should reduce the number of AC_TRY_RUN tests we have in configure.in, since such tests don't work when cross-compiling.  Missing some of them causes us to miss out on some rather important performance optimizations when cross-compiling.  (And we actually use cross-compilation for Mac universal, among other things.)

Many of the current AC_TRY_RUN tests in configure could be written using AC_TRY_COMPILE.

(I got started on this after discussing bug 257197 comment 84 with andrew, when I pointed out that the AC_TRY_RUN test after --enable-system-png could be an AC_TRY_COMPILE test with #error instead of exit().)

Some tests that could be converted include:

 * tests for sizeof() could be rewritten to depend on array size typedefs requiring positive arguments (or is it nonnegative?  hopefully that's portable.  We could, in fact, check that it's portable before doing it, by rejecting compilers where the test strategy doesn't work)

 * the va_copy tests look like they could just be AC_TRY_COMPILE tests, since they always return 0 (unless they're testing for segfaults)

 * the SYSTEM_JPEG and SYSTEM_PNG version tests could be rewritten using #error rather than exit()

 * the zlib version test could be written using ZLIB_VERNUM instead of ZLIB_VERSION, and thus using #error

That leaves only:
 * the test for MMAP_MISSES_WRITES (already OK, with pessimistic default)
 * the test for dynamic_cast<void*>() (which is for debugging code only)
 * the test for correct temporary object destruction order (already basically OK; we just miss the test)
 * code in a fallback case for libIDL config program not found (already OK)

Benjamin -- I'm not sure how this interacts with your rewrite-in-python plans, but I suspect you're going to want to keep a lot of the existing tests anyway.
(In reply to comment #0)
>  * the va_copy tests look like they could just be AC_TRY_COMPILE tests, since
> they always return 0 (unless they're testing for segfaults)

For what it's worth, these were added in bug 187180.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #257564 - Flags: review?(benjamin)
Attachment #257564 - Attachment description: add macros and convert just the sizeof tests → add static assertion macros and convert just the sizeof tests
Oops, forgot to bump the cache variable names.

Note that I'd also like to get this patch (but not other patches needed to finish fixing this bug) in on the 1.8 branch, since it's what would get the Mac universal build's cross-compiled half back to using the fast version of NS_LITERAL_STRING/NS_NAMED_LITERAL_STRING, which IIRC is a measurable performance difference.
Attachment #257564 - Attachment is obsolete: true
Attachment #257567 - Flags: review?(benjamin)
Attachment #257564 - Flags: review?(benjamin)
Attachment #257567 - Flags: review?(benjamin) → review+
Comment on attachment 257567 [details] [diff] [review]
add static assertion macros and convert just the sizeof tests

I'd like to land this on the 1.8 branch because it prevents a bunch of pretty important performance optimizations (in particular, the fast version of NS_LITERAL_STRING) from being used in the cross-compiled half of Mac universal builds.  I think it's pretty low risk, at least for our own builds, and I wouldn't be surprised if it helps some Linux distros too.
Attachment #257567 - Flags: approval1.8.1.4?
I think we should let this bake a couple weeks before landing on the branch.
And along with that I should bump the libpng version to what we actually use; bug 334110 forgot to do that.
Comment on attachment 257891 [details] [diff] [review]
convert system library version tests

Just two drive-by comments:

+                   [ #if $MOZZLIB > ZLIB_VERNUM
You should probably reverse that comparison to match the others.

Also, can you put the expected minimum version number in the #error message?
Attachment #257891 - Flags: review?(benjamin) → review+
Attachment 257891 [details] [diff], plus improvements from comment 9 (but not comment 8!) checked in to trunk.  That said, it's sort of irrelevant now that we require APNG functions to be present.
Comment on attachment 257567 [details] [diff] [review]
add static assertion macros and convert just the sizeof tests

approved for 1.8.1.4, a=dveditz for release-drivers
Attachment #257567 - Flags: approval1.8.1.4? → approval1.8.1.4+
Attachment 257567 [details] [diff] to MOZILLA_1_8_BRANCH.  Adding fixed1.8.1.4 keyword since that's the only patch I want to land on the branch (at this point, anyway).
Keywords: fixed1.8.1.4
I checked the logs of "MacOSX Darwin 8.7.0 bm-xserve02 Depend Fx-Nightly" on the Mozilla1.8 tinderbox, and landing this patch changed, in the second half of the universal build, this (old):

checking for usable wchar_t (2 bytes, unsigned)... maybe
checking for compiler -fshort-wchar option... maybe

into this (new):

checking for usable wchar_t (2 bytes, unsigned)... no
checking for compiler -fshort-wchar option... yes
And since I'd rather not touch the va_copy tests, since I'm not sure what they're really testing for, I'm going to mark this fixed for the trunk as well.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
This seams to have broken the windows mobile port:

configure: error: Compiler cannot compile macros used in autoconf tests.
*** Fix above errors and then restart with "make -f client.mk build"
make[1]: *** [configure] Error 1

Any information to aide in debugging, please let me know.
The errors in config.log ?
configure:5365: checking that static assertion macros used in autoconf tests work
configure:5386: arm-wince-gcc -c  -TC -nologo  conftest.c 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TC -nologo conftest.c 

conftest.c

configure:5403: arm-wince-gcc -c  -TC -nologo  conftest.c 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TC -nologo conftest.c 

conftest.c

configure(5403) : error C2118: negative subscript

configure:5426: arm-wince-gcc -c  -TP -nologo  conftest.C 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TP -nologo conftest.C 

conftest.C

configure:5443: arm-wince-gcc -c  -TP -nologo  conftest.C 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TP -nologo conftest.C 

conftest.C

configure(5443) : error C2118: negative subscript

configure:5478: checking for 64-bit OS
configure:5487: arm-wince-gcc -c  -TC -nologo  conftest.c 1>&5
c:/Program Files/Microsoft Visual Studio 8/VC/ce/bin/x86_arm/cl.exe /I"c:/Program Files/Microsoft Visual Studio 8/SmartDevices/SDK/PocketPC2003/Include" /I"x:/builds/minimo/mozilla/build/wince/shunt/include/" /FI"mozce_shunt.h" /DMOZCE_STATIC_BUILD /D_WIN32_WCE=420 /DUNDER_CE=420 /DWIN32_PLATFORM_PSPC=400 /DARM /DWINCE /D_ARM_ /Zc:wchar_t- /GS- /GR- -c -TC -nologo conftest.c 

conftest.c

configure(5487) : error C2118: negative subscript



arm-wince-gcc is a wrapper around the arm version of cl.
That output looks fine.  Could you try debugging with echo statements in configure.in (or configure)?  That output shows the compilation failing on the second and fourth tests, as it should (and then again for the 64-bit check, as it should).
That said, I don't think the log in comment 17 matches the output in comment 15, since the log in comment 17 continues after the output in comment 15 fails with an error.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.