[Webkit-unassigned] [Bug 42286] [EFL] Add support for using libcurl network backend.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42286





--- Comment #5 from Rafael Antognolli <antognolli at profusion.mobi>  2010-07-16 14:14:26 PST ---
(In reply to comment #4)
> (From update of attachment 61722 [details])
> Looks good to me. I am saying r- now to fix the typos and get the questions answered, specially the ones related to gthread.
> 
> > +        Instead of just libsoup, the EFL port now can use libcurl backend.
> > +        This is a step in the direction of removing dependency on glib. Just
> > +        need to pass the option -DNETWORK_BACKEND=curl to camke in order to
> > +        enable it.
> 
> typo: camke (in the changelog too)

Ok, fixing that.

> > @@ -133,9 +157,20 @@ LIST(APPEND WebCore_INCLUDE_DIRECTORIES
> >    ${Glib_INCLUDE_DIRS}
> >    ${GTK_INCLUDE_DIRS}
> >    ${ICU_INCLUDE_DIRS}
> > -  ${LIBSOUP24_INCLUDE_DIRS}
> > -  ${LIBXML2_INCLUDE_DIRS}
> > +  ${LIBXML2_INCLUDE_DIR}
> 
> You are changing from DIRS to DIR. Is that intentional?

Yes, I don't know why but the FindXml defines DIR instead of DIRS (which is the default). This was wrong but somehow worked before trying the curl backend.

> > @@ -80,7 +88,7 @@ SET(EWebLauncher_LIBRARIES
> >      ${EVAS_LIBRARIES}
> >      ${Gdk_LIBRARIES}
> >      ${Glib_LIBRARIES}
> > -    ${LIBSOUP24_LIBRARIES}
> > +    ${GTHREAD_LIBRARIES}
> 
> If you are trying to make glib optional, adding gthread unconditionally is what you want?

Same here, I think libsoup was adding the -lgthread flag automatically and that's how it was working before. But now in order to keep it compiling, it's necessary to add this gthread flag, since we still didn't remove glib completely. This is done in the next patch (I've just sent it).

> > diff --git a/WebKit/ChangeLog b/WebKit/ChangeLog
> > index 38c36fc..daa9e1d 100644
> > --- a/WebKit/ChangeLog
> > +++ b/WebKit/ChangeLog
> 
> Please start a new ChangeLog in WebKit/efl the soonish.

Sure :-) 

> > diff --git a/cmake/FindGthread.cmake b/cmake/FindGthread.cmake
> > new file mode 100644
> > index 0000000..7dabbee
> > --- /dev/null
> > +++ b/cmake/FindGthread.cmake
> > @@ -0,0 +1,21 @@
> > +# Find include and libraries for GTHREAD library
> > +# GTHREAD_INCLUDE     Directories to include to use GTHREAD
> > +# GTHREAD_INCLUDE-I   Directories to include to use GTHREAD (with -I)
> > +# GTHREAD_LIBRARIES   Libraries to link against to use GTHREAD
> > +# GTHREAD_FOUND       GTHREAD was found
> > +
> > +IF (UNIX)
> > +    INCLUDE (UsePkgConfig)
> > +    PKGCONFIG (gthread-2.0 GTHREAD_include_dir GTHREAD_link_dir GTHREAD_libraries GTHREAD_include)
> > +    IF (GTHREAD_include AND GTHREAD_libraries)
> > +        SET (GTHREAD_FOUND TRUE)
> > +        EXEC_PROGRAM ("echo"
> > +            ARGS "${GTHREAD_include} | sed 's/[[:blank:]]*-I/;/g'"
> > +            OUTPUT_VARIABLE GTHREAD_INCLUDE
> > +        )
> > +        SET (GTHREAD_INCLUDE-I ${GTHREAD_include})
> > +        SET (GTHREAD_LIBRARIES ${GTHREAD_libraries})
> > +    ELSE (GTHREAD_include AND GTHREAD_libraries)
> > +        SET (GTHREAD_FOUND FALSE)
> > +    ENDIF (GTHREAD_include AND GTHREAD_libraries)
> > +ENDIF (UNIX)
> > diff --git a/cmake/OptionsEfl.cmake b/cmake/OptionsEfl.cmake
> > index 3408ea7..65bf4f7 100644
> > --- a/cmake/OptionsEfl.cmake
> > +++ b/cmake/OptionsEfl.cmake
> > @@ -8,14 +8,18 @@ ADD_DEFINITIONS(-DDATA_DIR="${DATA_DIR}")
> >  ADD_DEFINITIONS(-DWTF_PLATFORM_EFL=1)
> >  SET(WTF_PLATFORM_EFL 1)
> >  
> > -SET(LIBSOUP24_MIN_VERSION 2.28.2)
> > +# -----------------------------------------------------------------------------
> > +# Determine which network backend will be used
> > +# -----------------------------------------------------------------------------
> > +SET(ALL_NETWORK_BACKENDS soup curl)
> > +OPTION(NETWORK_BACKEND "choose which network backend to use (one of ${ALL_NETWORK_BACKENDS})" "soup")
> >  
> >  FIND_PACKAGE(Cairo 1.6 REQUIRED)
> >  FIND_PACKAGE(EFL REQUIRED)
> >  FIND_PACKAGE(Freetype 9.0 REQUIRED)
> >  FIND_PACKAGE(GDK 2.10 REQUIRED)
> >  FIND_PACKAGE(Glib REQUIRED)
> > -FIND_PACKAGE(LibSoup2 2.28.2 REQUIRED)
> > +FIND_PACKAGE(Gthread REQUIRED)
> >  FIND_PACKAGE(Sqlite REQUIRED)
> >  FIND_PACKAGE(LibXml2 2.6 REQUIRED)
> >  FIND_PACKAGE(LibXslt 1.1.7 REQUIRED)
> > @@ -32,14 +36,6 @@ LIST(APPEND WTF_INCLUDE_DIRECTORIES ${ICU_INCLUDE_DIRS})
> >  SET(WTF_PLATFORM_CAIRO 1)
> >  ADD_DEFINITIONS(-DWTF_PLATFORM_CAIRO=1)
> 
> 
> Are these gthread changes related to this patch? should not it go in a followup?

As I explained above, this was necessary because libsoup was getting this dependency automatically, but now that I removed it our call for g_thread_init in our ewk_main.cpp was unresolved.

Should I send the changes from bug 42480 in the same patch, keep them splitted, or split even more? (I'm just trying make sure that both changes keep it compiling fine).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.