Re: [ros-dev] [ros-diffs] [jgardou] 53481: [WIDL] - reenable -Oif flag, it's too good to be disabled With this commit, advapi32:service winetest suffers only 2 failures.

[Jérôme Gardou]

Feel free to disable it again if probems occur.

"Probable bugs" are hard to test alone, so I'm waiting for test bot to 
confirm this fixes things without breaking others.

Regards.
Jérôme.

Le 28/08/2011 17:37, [e-mail] a écrit :
> Author: jgardou
> Date: Sun Aug 28 15:37:01 2011
> New Revision: 53481
>
> URL: http://svn.reactos.org/svn/reactos?rev=53481&view=rev
> Log:
> [WIDL]
> reenable -Oif flag, it's too good to be disabled
>
> With this commit, advapi32:service winetest suffers only 2 failures.
>
> Modified:
>      trunk/reactos/cmake/idl-support.cmake
>      trunk/reactos/tools/rbuild/backend/mingw/rules.mak
>
> Modified: trunk/reactos/cmake/idl-support.cmake
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/cmake/idl-support.cmake?rev...
> ==============================================================================
> --- trunk/reactos/cmake/idl-support.cmake [iso-8859-1] (original)
> +++ trunk/reactos/cmake/idl-support.cmake [iso-8859-1] Sun Aug 28 15:37:01 2011
> @@ -22,8 +22,8 @@
>       set(IDL_HEADER_ARG -h -o) #.h
>       set(IDL_HEADER_ARG2 -h -H) #.h
>       set(IDL_TYPELIB_ARG -t -o) #.tlb
> set(IDL_SERVER_ARG -s -o) #.c for server library
> set(IDL_CLIENT_ARG -c -o) #.c for stub client library
> set(IDL_SERVER_ARG -Oif -s -o) #.c for server library
> set(IDL_CLIENT_ARG -Oif -c -o) #.c for stub client library
>  set(IDL_PROXY_ARG -p -o)
>  set(IDL_INTERFACE_ARG -u -o)
>  if(ARCH MATCHES i386)
>
> @@ -169,6 +169,6 @@
>           list(APPEND IID_SOURCES ${NAME}_i.c)
>       endforeach()
>       add_library(${TARGET} ${IID_SOURCES})
> add_dependencies(${TARGET} psdk)
> add_dependencies(${TARGET} psdk)
>  set_target_properties(${TARGET} PROPERTIES EXCLUDE_FROM_ALL TRUE)
> endfunction()
>
> Modified: trunk/reactos/tools/rbuild/backend/mingw/rules.mak
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/tools/rbuild/backend/mingw/...
> ==============================================================================
> --- trunk/reactos/tools/rbuild/backend/mingw/rules.mak [iso-8859-1] (original)
> +++ trunk/reactos/tools/rbuild/backend/mingw/rules.mak [iso-8859-1] Sun Aug 28 15:37:01 2011
> @@ -264,7 +264,7 @@
>
> ${call RBUILD_intermediate_path_noext,$(2)}_c: $(2) $(3) $$(widl_TARGET) | ${call RBUILD_intermediate_dir,$(2)}
>    $$(ECHO_WIDL)
> $$(Q)$$(widl_TARGET) ${call RBUILD_midlflags,$(1),$(4),-I${call RBUILD_dir,$(2)}} -h -H ${call RBUILD_intermediate_path_noext,$(2)}_c.h -c -C ${call RBUILD_intermediate_path_noext,$(2)}_c.c $(2)
>
> $$(Q)$$(widl_TARGET) ${call RBUILD_midlflags,$(1),$(4),-I${call RBUILD_dir,$(2)}} -Oif -h -H ${call RBUILD_intermediate_path_noext,$(2)}_c.h -c -C ${call RBUILD_intermediate_path_noext,$(2)}_c.c $(2)
> $${checkpoint} $$@>$(NUL)
>
> ${call RBUILD_CC,$(1),${call RBUILD_intermediate_path_noext,$(2)}_c.c,,,${call RBUILD_intermediate_path_noext,$(2)}_c.o}
>
> @@ -280,7 +280,7 @@
>
> ${call RBUILD_intermediate_path_noext,$(2)}_s: $(2) $(3) $$(widl_TARGET) | ${call RBUILD_intermediate_dir,$(2)}
>    $$(ECHO_WIDL)
> $$(Q)$$(widl_TARGET) ${call RBUILD_midlflags,$(1),$(4),-I${call RBUILD_dir,$(2)}} -h -H ${call RBUILD_intermediate_path_noext,$(2)}_s.h -s -S ${call RBUILD_intermediate_path_noext,$(2)}_s.c $(2)
>
> $$(Q)$$(widl_TARGET) ${call RBUILD_midlflags,$(1),$(4),-I${call RBUILD_dir,$(2)}} -Oif -h -H ${call RBUILD_intermediate_path_noext,$(2)}_s.h -s -S ${call RBUILD_intermediate_path_noext,$(2)}_s.c $(2)
> $${checkpoint} $$@>$(NUL)
>
> ${call RBUILD_CC,$(1),${call RBUILD_intermediate_path_noext,$(2)}_s.c,,,${call RBUILD_intermediate_path_noext,$(2)}_s.o}

[Eric Kohl]

Hello Jérôme,

your patch looks very good! I hope Alexandre accepts it without a length 
discussion. ;-)

I just ran another set of "advapi32_winetest service" tests on my 
machine. Both tests, with and without -Oif option return the same 
results: 2 failures! Great!

And as far as I can see our test machines return bogus results again. I 
guess we really need to reboot the test machines for each test, 
otherwise the results are useless. :-/

Regards
Eric
> Feel free to disable it again if probems occur.
>
> "Probable bugs" are hard to test alone, so I'm waiting for test bot to
> confirm this fixes things without breaking others.
>
> Regards.
> Jérôme.

Yes! Lets hide the bugs, so we can assume ReactOS is fixed!

Please excuse my sarcasm. I couldn't really stop myself.

In my opinion, useless results do happen when you reboot VM that many times until it returns no failures. Such results are then, useless. If there is an error, it should be fixed. It doesnt have to be advapi32 error, but still.

Best regards

On Sun, 28 Aug 2011 20:43 +0200, "Eric Kohl" [e-mail] wrote:
> Hello Jérôme,
>
> your patch looks very good! I hope Alexandre accepts it without a length 
> discussion. ;-)
>
> I just ran another set of "advapi32_winetest service" tests on my 
> machine. Both tests, with and without -Oif option return the same 
> results: 2 failures! Great!
>
> And as far as I can see our test machines return bogus results again. I 
> guess we really need to reboot the test machines for each test, 
> otherwise the results are useless. :-/

With best regards
Caemyr