divyang4481 / firebreath

Automatically exported from code.google.com/p/firebreath
0 stars 0 forks source link

MinSizeRel and RelWithDebugInfo configurations on MSVC can't be statically linked #82

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?

1. Generate MSVC project files on WIN32 using fbgen, normal build process, etc.
2. Open generated solution for the plugin, and check Project properties -> 
Configuration properties -> C/C++ -> Code Generation -> Runtime library

You'd expect it to be consistent, but in fact you see it have the DLL version 
for the MinSizeRel and RelWithDebugInfo configurations but a non-DLL version 
for the Debug and Release versions.

Applies to everything up to 1.2.1, Windows Vista, any browser

One way of fixing this requires changes similar to the following, in 
cmake/buildconfig.cmake:

@@ -20,10 +20,14 @@
         set(CMAKE_CPP_FLAGS ${CMAKE_CPP_FLAGS} /D BOOST_ALL_NO_LIB=1)
     endif()
    set(CMAKE_CXX_FLAGS                          "/DWIN32 /DXP_WIN=1 /W3 /wd4996 /nologo /EHsc /wd4290 /D UNICODE /D _UNICODE /D _WINDOWS")
-   set(CMAKE_C_FLAGS_RELEASE                    "/MT /O1 /DNDEBUG")
-   set(CMAKE_CXX_FLAGS_RELEASE                  "/MT /O1 /DNDEBUG")
+   set(CMAKE_C_FLAGS_RELEASE                    "/MT /Ox /DNDEBUG")
+   set(CMAKE_CXX_FLAGS_RELEASE                  "/MT /Ox /DNDEBUG")
    set(CMAKE_C_FLAGS_DEBUG                      "/MTd /Od /DDEBUG /D_DEBUG /ZI /RTC1 /Gm")
    set(CMAKE_CXX_FLAGS_DEBUG                    "/MTd /Od /DDEBUG /D_DEBUG /ZI /RTC1 /Gm")
+   set(CMAKE_C_FLAGS_MINSIZEREL                 "/MT /O1 /DNDEBUG")
+   set(CMAKE_CXX_FLAGS_MINSIZEREL               "/MT /O1 /DNDEBUG")
+   set(CMAKE_C_FLAGS_RELWITHDEBINFO             "/MTd /Ox /DDEBUG /D_DEBUG /ZI 
/RTC1 /Gm")
+   set(CMAKE_CXX_FLAGS_RELWITHDEBINFO           "/MTd /Ox /DDEBUG /D_DEBUG /ZI 
/RTC1 /Gm")
    set(CMAKE_EXE_LINKER_FLAGS_DEBUG
        "${CMAKE_EXE_LINKER_FLAGS_DEBUG}         ")
    set(CMAKE_EXE_LINKER_FLAGS_RELEASE

NB: this patch also changes the release build optimisation setting from 
"Optimize Size" to "Full Optimization" so someone might want to switch that 
back.

Original issue reported on code.google.com by kylo...@gmail.com on 29 Sep 2010 at 9:34

GoogleCodeExporter commented 8 years ago
Turns out that patch won't work - the glorious inpenetrability of CMake meant 
that somehow this fixed the static/dynamic issue as expected but magically 
ignored some of the other flags, which are apparently incompatible. But this 
was only noticed when I deleted the cmake cache.

Oh, and also I think the debug flags are wrong for the last configuration.

Try these flags instead. These appear to work.

    set(CMAKE_CXX_FLAGS                          "/DWIN32 /DXP_WIN=1 /W3 /wd4996 /nologo /EHsc /wd4290 /D UNICODE /D _UNICODE /D _WINDOWS")
    set(CMAKE_C_FLAGS_RELEASE                    "/MT /Ox /DNDEBUG")
    set(CMAKE_CXX_FLAGS_RELEASE                  "/MT /Ox /DNDEBUG")
    set(CMAKE_C_FLAGS_DEBUG                      "/MTd /Od /DDEBUG /D_DEBUG /ZI /RTC1 /Gm")
    set(CMAKE_CXX_FLAGS_DEBUG                    "/MTd /Od /DDEBUG /D_DEBUG /ZI /RTC1 /Gm")
    set(CMAKE_C_FLAGS_MINSIZEREL                 "/MT /O1 /DNDEBUG")
    set(CMAKE_CXX_FLAGS_MINSIZEREL               "/MT /O1 /DNDEBUG")
    set(CMAKE_C_FLAGS_RELWITHDEBINFO             "/MTd /Ox /DNDEBUG /Zi")
    set(CMAKE_CXX_FLAGS_RELWITHDEBINFO           "/MTd /Ox /DNDEBUG /Zi")

Original comment by kylo...@gmail.com on 29 Sep 2010 at 2:53

GoogleCodeExporter commented 8 years ago
So the reason that we had the default set to minimum size is because generally 
with plugins the main concern is size; still, you're probably right that it 
would be better to do full optimization.  I will make it so =]  This change 
will go into 1.3, which should be released probably a week from tomorrow (on 
Oct 7).  That date may slip a bit, but should be pretty close.

Original comment by taxilian on 29 Sep 2010 at 3:34

GoogleCodeExporter commented 8 years ago

Original comment by taxilian on 29 Sep 2010 at 3:34

GoogleCodeExporter commented 8 years ago
Yeah, I figured that if you have a working 'min-size' configuration, it makes 
sense to have that be the 'small' build and for the other to be the 'fast' one, 
but really that's up to you guys.

Original comment by kylo...@gmail.com on 29 Sep 2010 at 3:40

GoogleCodeExporter commented 8 years ago
yeah, you're right; that does make sense =]  I just hadn't previously really 
looked at those two configurations (which in hindsight was rather 
shortsighted).  The changes have been applied as you suggest and are fixed in 
dev.

Original comment by taxilian on 29 Sep 2010 at 3:43

GoogleCodeExporter commented 8 years ago

Original comment by taxilian on 29 Sep 2010 at 3:43