Re: [E-devel] efl merge and win32

[Gustavo S. B. <bar...@pr...>]

On Mon, Jan 14, 2013 at 12:03 PM, Cedric BAIL <ced...@fr...> wrote:
> On Mon, Jan 14, 2013 at 7:51 PM, Gustavo Sverzut Barbieri
> <bar...@pr...> wrote:
>> On Mon, Jan 14, 2013 at 4:53 AM, Nicolas Aguirre
>> <agu...@gm...> wrote:
>>> 2013/1/11 Gustavo Sverzut Barbieri <bar...@pr...>
>>>> On Fri, Jan 11, 2013 at 4:43 PM, Nicolas Aguirre
>>>> <agu...@gm...> wrote:
>>>> modules/ecore_evas/engines/win32/modules_ecore_evas_engines_win32_module_la-ecore_evas_win32.lo
>>>> > In file included from
>>>> > modules/ecore_evas/engines/win32/ecore_evas_win32.c:19:0: error:
>>>> > unterminated #ifdef
>>>> > make[4]: ***
>>>> >
>>>> [modules/ecore_evas/engines/win32/modules_ecore_evas_engines_win32_module_la-ecore_evas_win32.lo]
>>>> > Error 1
>>>>
>>>> Flavio split ecore-evas engines out of ecore_evas itself, but seems he
>>>> couldn't test with win32. It should be a matter of basing the fixes on
>>>> other modules. Very likely these are just C errors that once fixed
>>>> will work.
>>>>
>>>>
>>>> > Anyway I'm away this week end, i will look into it next week.
>>>>
>>>> It's better if you can engage for couple of days into that. It will
>>>> require lots of work and compilation rounds, not just the code
>>>> changed, but the build system as well. Maybe some modules will have to
>>>> be disabled, etc.
>>>>
>>> after ifdef correction (patch attached) i get a new kind of error, and i
>>> fear it's related with all include you removed.
>>>
>>> Making all in src
>>> make  all-recursive
>>>   CC
>>> modules/ecore_evas/engines/win32/modules_ecore_evas_engines_win32_module_la-ecore_evas_win32.lo
>>> In file included from
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:12:0:
>>> ../src/lib/ecore_win32/Ecore_Win32.h:14:4: warning: #warning "You are using
>>> a work in progress API. This API is not stable" [-Wcpp]
>>> ../src/lib/ecore_win32/Ecore_Win32.h:15:4: warning: #warning "and is
>>> subject to change. You use this at your own risk." [-Wcpp]
>>
>> ugh, this is annoying... what about if you patch Ecore_Win32.h to do
>> like other libraries:
>>
>> #ifndef _ECORE_WIN32_API_IS_UNSTABLE_AND_I_KNOW_IT
>> #warning "...""
>> #endif
>>
>> then define that for every user file in EFL. It does pollute too much
>> and maybe that caused the confusion. See below.
>>
>>
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: In function
>>> '_ecore_evas_win32_event_window_damage':
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:258:3: warning:
>>> #warning [ECORE] [WIN32] No Region code [-Wcpp]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: In function
>>> '_ecore_evas_win32_rotation_set':
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:630:66: warning: unused
>>> parameter 'resize' [-Wunused-parameter]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: In function
>>> '_ecore_evas_win32_cursor_set':
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:814:42: warning: unused
>>> parameter 'ee' [-Wunused-parameter]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:814:59: warning: unused
>>> parameter 'obj' [-Wunused-parameter]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:814:68: warning: unused
>>> parameter 'layer' [-Wunused-parameter]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:814:79: warning: unused
>>> parameter 'hot_x' [-Wunused-parameter]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:814:90: warning: unused
>>> parameter 'hot_y' [-Wunused-parameter]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: In function
>>> '_ecore_evas_win32_alpha_set':
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:979:34: warning: unused
>>> variable 'wdata' [-Wunused-variable]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: In function
>>> '_ecore_evas_win32_screen_dpi_get':
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1056:52: warning:
>>> unused parameter 'ee' [-Wunused-parameter]
>>
>> please also fix these with EINA_UNUSED after the unused parameter name.
>>
>>
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: In function
>>> '_ecore_evas_win32_new_internal':
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1324:38: warning:
>>> declaration of '_ecore_evas_engine_init' shadows a global declaration
>>> [-Wshadow]
>>> ../src/lib/ecore_evas/ecore_evas_private.h:403:6: warning: shadowed
>>> declaration is here [-Wshadow]
>>
>> this is a regular programming mistake. Just fix the declaration of
>> _ecore_evas_engine_init :-)
>>
>>
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1354:4: error: 'iface'
>>> undeclared (first use in this function)
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1354:4: note: each
>>> undeclared identifier is reported only once for each function it appears in
>>
>> also another programming mistake. Please look at other ecore_evas
>> engines and see their handling of "iface". It's nothing related to
>> platform dependent stuff, just need to declare the variable with the
>> proper ecore evas engine type.
>>
>>
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: At top level:
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1412:1: warning:
>>> 'ecore_evas_software_gdi_new' already declared with dllexport attribute:
>>> dllimport ignored [-Wattributes]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1443:1: warning:
>>> 'ecore_evas_software_ddraw_new' already declared with dllexport attribute:
>>> dllimport ignored [-Wattributes]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1473:1: warning:
>>> 'ecore_evas_software_16_ddraw_new' already declared with dllexport
>>> attribute: dllimport ignored [-Wattributes]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1502:1: warning:
>>> 'ecore_evas_direct3d_new' already declared with dllexport attribute:
>>> dllimport ignored [-Wattributes]
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1534:1: warning:
>>> 'ecore_evas_gl_glew_new' already declared with dllexport attribute:
>>> dllimport ignored [-Wattributes]
>>
>> These are a mix of programming errors when splitting the modules.
>>
>> 1 - names: these functions (_new()) should be called (_new_internal)
>> and that's what src/lib/ecore_evas/ecore_evas.c should be looking. See
>> http://trac.enlightenment.org/e/browser/trunk/efl/src/lib/ecore_evas/ecore_evas.c#L3287
>> that's the symbol you must have.
>>
>> 2 - attribute: these EAPI in there should be defined to always be
>> export, that in the C file. At the beginning of ecore_evas_win32.c and
>> AFTER all the #include, add:
>>
>> #undef EAPI
>> #ifdef DLL_EXPORT
>> # define EAPI __declspec(dllexport)
>> #else
>> # define EAPI
>> #endif
>>
>> that's similar to what is in Ecore_Evas.h, but simplified as 1) we're
>> always doing the dllexport; 2) we're always building this for windows;
>> 3) we know EAPI was already defined by other #include :-)
>>
>>
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c: In function
>>> '_ecore_evas_win32_interface_new':
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1559:23: error:
>>> 'interface_win32_name' undeclared (first use in this function)
>>> modules/ecore_evas/engines/win32/ecore_evas_win32.c:1560:26: error:
>>> 'interface_win32_version' undeclared (first use in this function)
>>
>> regular programming errors. Missing variables, see what other
>> ecore_evas engine does.
>>
>>
>>> I think that the best choice we have here is to revert all the commits with
>>> headers remove, Lucas did last week :/
>>
>> relax and take a deep breath, the port won't be easy, but we're still
>> not any close to Lucas' changes yet ;-)
>>
>> These are all regular programming errors, nothing related to includes.
>> No need to remove his work. And my advice is to read the error
>> messages rather than assume/imagine a problem.
>
> I think that this answer is really rude. Someone break something that
> was working and you just say now spend your time fixing it.

Sorry if I sound rude, it was not my intention. But it was not working
before Lucas' changes -- IOW: was not Lucas' fault, it could be mine
or someone's else. In fact the problematic code right now is from
Flávio, and he asked Vincent to review that before commit (without a
reply, so likely Vincent couldn't test as well, which is fine on both
sides).


> That's not
> fair. Nicolas is giving you some hint at how bad the breakage his. Now
> the correct things to do is that either you or lucas setup a windows
> cross compilation environment, or we revert the change. At this point
> you are basically breaking the work of Vincent, expect someone to step
> up and do the fix again... I don't see us getting that lucky again !

Again, read the errors pointed out and they have nothing to do with
Lucas. Bashing him for something he didn't do is bad, and rude.

As for setting up a windows cross compilation. I could do it, however
the way things are going I'm quite demotivated to do this. Revert all
that you want, be happy.

--
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: bar...@gm...
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202