RESOLVED FIXED 145837
[EFL] Fix the openwebrtc and gst-plugins-openwebrtc build with clang 
https://bugs.webkit.org/show_bug.cgi?id=145837
Summary [EFL] Fix the openwebrtc and gst-plugins-openwebrtc build with clang
Csaba Osztrogonác
Reported 2015-06-10 03:22:12 PDT
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.
Attachments
Patch (4.53 KB, patch)
2015-06-10 03:48 PDT, Csaba Osztrogonác 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2015-06-10 03:23:10 PDT
We have same error in gst-plugins-openwebrtc module.
Csaba Osztrogonác
Comment 2 2015-06-10 03:28:38 PDT
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
Comment 3 2015-06-10 03:48:52 PDT
Created attachment 254643 [details] Patch
Csaba Osztrogonác
Comment 4 2015-06-10 03:51:47 PDT
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
Comment 5 2015-06-10 06:04:02 PDT
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
Comment 6 2015-06-10 06:16:10 PDT
(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
Comment 7 2015-06-12 03:37:46 PDT
cc-ing Philip, maybe you are interested in fixing it for GTK and/or fixing it in upstream openwebrtc / gst-plugins-openwebrtc.
Philippe Normand
Comment 8 2015-06-12 06:08:40 PDT
Would you like to submit the patch upstream? It's yours after all :)
Csaba Osztrogonác
Comment 9 2015-06-26 07:41:29 PDT
(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
Comment 10 2015-07-30 11:45:44 PDT
I reported the bug upstream. Can we push this patch to WebKit to be able to build dependencies with clang?
Gyuyoung Kim
Comment 11 2015-07-30 21:12:29 PDT
Comment on attachment 254643 [details] Patch rs=me.
Philippe Normand
Comment 12 2015-07-31 02:01:52 PDT
Comment on attachment 254643 [details] Patch Please bump the version now that your patch was merged upstream :)
Csaba Osztrogonác
Comment 13 2015-09-24 03:46:55 PDT
(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 14 2015-09-24 03:55:17 PDT
Comment on attachment 254643 [details] Patch r? again, I still think it is the easieast fix for this issue.
Philippe Normand
Comment 15 2015-09-24 04:13:35 PDT
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
Comment 16 2015-09-28 04:28:09 PDT
So can we land this patch as is?
Csaba Osztrogonác
Comment 17 2015-10-01 03:00:05 PDT
ping?
WebKit Commit Bot
Comment 18 2015-10-01 08:39:29 PDT
Comment on attachment 254643 [details] Patch Clearing flags on attachment: 254643 Committed r190409: <http://trac.webkit.org/changeset/190409>
WebKit Commit Bot
Comment 19 2015-10-01 08:39:34 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.