Bug 145837 – [EFL] Fix the openwebrtc and gst-plugins-openwebrtc build with clang

• Component: New Bugs
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

Building openwebrtc jhbuild module with clang fails:
-----------------------------------------------------

/home/webkit/WebKit/WebKitBuild/DependenciesEFL/Root/include/gstreamer-1.0/gst/gstcompat.h:54:38: warning: equality comparison with extraneous parentheses [-Wparentheses-equality]
  if (((((GstPad*)(pad))->direction) == GST_PAD_SRC))
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
/home/webkit/WebKit/WebKitBuild/DependenciesEFL/Root/include/gstreamer-1.0/gst/gstcompat.h:54:38: note: remove extraneous parentheses around the comparison to silence this warning
  if (((((GstPad*)(pad))->direction) == GST_PAD_SRC))
      ~                              ^             ~
/home/webkit/WebKit/WebKitBuild/DependenciesEFL/Root/include/gstreamer-1.0/gst/gstcompat.h:54:38: note: use '=' to turn this equality comparison into an assignment
  if (((((GstPad*)(pad))->direction) == GST_PAD_SRC))
                                     ^~
                                     =
1 warning generated.

-----

gstcompat.h:54 -->  if (GST_PAD_IS_SRC (pad))

Removing the extra parentheses would be very ugly here,
we should simply disable parentheses-equality warning.

[Csaba Osztrogonác]

We have same error in gst-plugins-openwebrtc module.

[Csaba Osztrogonác]

It can be fixed easily on EFL, because we download tarballs, which
can be patched by jhbuild. But unfortunately GTK download git repositories,
which can't be patched by jhbuild.

[Csaba Osztrogonác]

Created attachment 254643 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 254643 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254643&action=review

> Tools/efl/patches/openwebrtc-gst-plugins-clang-warning-fix.patch:10
> +-libgstvideorepair_la_CFLAGS = $(GST_CFLAGS) $(GST_PLUGINS_BASE_CFLAGS) -Wall -Wextra -Werror
> ++libgstvideorepair_la_CFLAGS = $(GST_CFLAGS) $(GST_PLUGINS_BASE_CFLAGS) -Wall -Wextra

Adding -Wno-parentheses-equality here doesn't work, because 
the parent Makefile adds -Wall to CFLAGS which comes after 
libgstvideorepair_la_CFLAGS and overrides the -Wno option.

[Rohit]

I have been facing this issue on GTK port for some time now. Is there any workaround to build WebKit for GTK port?

[Csaba Osztrogonác]

(In reply to comment #5)
> I have been facing this issue on GTK port for some time now. Is there any
> workaround to build WebKit for GTK port?

You can build the dependencies with GCC without any problem.
And then you can build WebKit with GCC or Clang too.

[Csaba Osztrogonác]

cc-ing Philip, maybe you are interested in fixing it for GTK
and/or fixing it in upstream openwebrtc / gst-plugins-openwebrtc.

[Philippe Normand]

Would you like to submit the patch upstream? It's yours after all :)

[Csaba Osztrogonác]

(In reply to comment #8)
> Would you like to submit the patch upstream? It's yours after all :)

upstream issues:
https://github.com/EricssonResearch/openwebrtc-gst-plugins/issues/44
https://github.com/EricssonResearch/openwebrtc/issues/420

[Csaba Osztrogonác]

I reported the bug upstream. Can we push this patch
to WebKit to be able to build dependencies with clang?

[Gyuyoung Kim]

Comment on attachment 254643 [details]
Patch

rs=me.

[Philippe Normand]

Comment on attachment 254643 [details]
Patch

Please bump the version now that your patch was merged upstream :)

[Csaba Osztrogonác]

(In reply to comment #12)
> Comment on attachment 254643 [details]
> Patch
> 
> Please bump the version now that your patch was merged upstream :)

Only the openwebrtc patch was merged, but bumping isn't as trivial as you think. Not at all. The openwebrtc-gst-plugin fix was rejected.

Bumping to trunk openwebrtc is impossible, because https://github.com/EricssonResearch/openwebrtc/commit/49635a9eba2d3e081cab7ba631bc32a5521975dd
needs newer gst-plugins-base - https://github.com/gstreamer-mirror/gst-plugins-base/commit/bfc13c8e51862662804dcc9d14bf3a7f9f0027bd . 

Or bumping to the exact openwebrtc revision which merged my fix 
isn't as easy, because we would need many unrelated fixes too:
- https://github.com/EricssonResearch/openwebrtc/commit
605fbcdc734444a936dde73ddc19b300f59c7503 commit in openwebrtc
uses a feature added to libnice by https://github.com/libnice/libnice/commit/4e4af4be3890f12e300fe71104564171091b7a17 , so we will need to bump libnice too.
This commit is already included in 0.1.11.
- Additionally we would have to fix FindOpenWebRTC.cmake too, because the
package version was bumped by https://github.com/EricssonResearch/openwebrtc/commit/437679ffb777265f6bcfdea2be0d0aec8bc6a474
- We would need to add -Wno-parentheses-equality to the brand
new tests/Makefile.am

----

I don't think if we should fix the whole world and bump absouletely
unrelated modules to unstable revisions (with possible new bugs, etc.)
just to suppress some compiler warnings.

[Csaba Osztrogonác]

Comment on attachment 254643 [details]
Patch

r? again, I still think it is the easieast fix for this issue.

[Philippe Normand]

Bumping openwebrtc* only makes sense after we migrated to GStreamer 1.6.x.

OpenWebRTC barely works anyway with older versions of GStreamer.

[Csaba Osztrogonác]

So can we land this patch as is?

[Csaba Osztrogonác]

ping?

[WebKit Commit Bot]

Comment on attachment 254643 [details]
Patch

Clearing flags on attachment: 254643

Committed r190409: <http://trac.webkit.org/changeset/190409>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.