Bug 153540

Created attachment 272849 [details] Patch

We have to be careful when uploading the patch because it requires to update the bots dependencies.

Comment on attachment 272849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272849&action=review > Tools/gtk/jhbuild-webrtc.modules:-29 > - For example openh264 and libsrtp is not part of the "main" Tools/gtk/jhbuild.modules. Should we keep jhbuild-webrtc.modules for such dependencies?

I failed to quote what I intended above. Here's the text I intended to comment. > 22 <autotools id="openh264" supports-non-srcdir-builds="no" autogen-sh="pseudo-configure"> > 23 <branch module="cisco/openh264/archive/v1.4.0.tar.gz" version="1.4.0" > 24 checkoutdir="openh264-1.4.0" > 25 repo="github-tarball"> > 26 <patch file="openh264-configure.patch" strip="0"/> > 27 </branch> > 28 </autotools>

(In reply to comment #3) > Comment on attachment 272849 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272849&action=review > > > Tools/gtk/jhbuild-webrtc.modules:-29 > > - > > For example openh264 and libsrtp is not part of the "main" > Tools/gtk/jhbuild.modules. Should we keep jhbuild-webrtc.modules for such > dependencies? I think , we can add any dependency to the main jhbuild modules file when required, having a different modules file was required because we needed different versions of the same libraries to compile some part.

(In reply to comment #5) > I think , we can add any dependency to the main jhbuild modules file when > required, having a different modules file was required because we needed > different versions of the same libraries to compile some part. If no one complains about adding those dependencies to the main modules file, then I'm fine with that. :)

Comment on attachment 272849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272849&action=review >>> Tools/gtk/jhbuild-webrtc.modules:-29 >>> - >> >> For example openh264 and libsrtp is not part of the "main" Tools/gtk/jhbuild.modules. Should we keep jhbuild-webrtc.modules for such dependencies? > > I think , we can add any dependency to the main jhbuild modules file when required, having a different modules file was required because we needed different versions of the same libraries to compile some part. If they are needed to run the tests and can't be installed using the gtk/install-dependencies script then yes, I think those should be added in the main moduleset.

(In reply to comment #7) > Comment on attachment 272849 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272849&action=review > > >>> Tools/gtk/jhbuild-webrtc.modules:-29 > >>> - > >> > >> For example openh264 and libsrtp is not part of the "main" Tools/gtk/jhbuild.modules. Should we keep jhbuild-webrtc.modules for such dependencies? > > > > I think , we can add any dependency to the main jhbuild modules file when required, having a different modules file was required because we needed different versions of the same libraries to compile some part. > > If they are needed to run the tests and can't be installed using the > gtk/install-dependencies script then yes, I think those should be added in > the main moduleset. They are not needed, I've run the tests without them.

Comment on attachment 272849 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=272849&action=review >>>>> Tools/gtk/jhbuild-webrtc.modules:-29 >>>>> - >>>> >>>> For example openh264 and libsrtp is not part of the "main" Tools/gtk/jhbuild.modules. Should we keep jhbuild-webrtc.modules for such dependencies? >>> >>> I think , we can add any dependency to the main jhbuild modules file when required, having a different modules file was required because we needed different versions of the same libraries to compile some part. >> >> If they are needed to run the tests and can't be installed using the gtk/install-dependencies script then yes, I think those should be added in the main moduleset. > > They are not needed, I've run the tests without them. I think we should add them when we need them to the main jhbuild.modules, it is usually what we are doing for every other dependency. We added the extra modules file because we wanted to be able to have different versions of the modules. So we can add those dependencies when required in my opinion.

Comment on attachment 272849 [details] Patch Rejecting attachment 272849 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 272849, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: brtc.modules' patching file Tools/gtk/jhbuild.modules Hunk #1 succeeded at 438 (offset -16 lines). Hunk #2 succeeded at 446 with fuzz 1 (offset -19 lines). patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/platform/efl/TestExpectations patching file LayoutTests/platform/gtk/TestExpectations Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Philippe Normand']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/940836

Committed r197752: <http://trac.webkit.org/changeset/197752>

(In reply to comment #11) > Committed r197752: <http://trac.webkit.org/changeset/197752> This seems to have caused lot of timeouts on the layout tests of the release bot: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29?numbuilds=100 r197752 Exiting early after 0 crashes and 50 timeouts. 30409 tests run. 208 failures 5 new passes -- https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/14296 r197751 was a commit for the branch WebKitGTK/webkit-2.12 (not for trunk) r197750 242 failures 21 new passes 11 flakes -- https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/14295

(In reply to comment #12) > (In reply to comment #11) > > Committed r197752: <http://trac.webkit.org/changeset/197752> > > This seems to have caused lot of timeouts on the layout tests of the release > bot: > > > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20%28Tests%29?numbuilds=100 > > r197752 Exiting early after 0 crashes and 50 timeouts. 30409 tests run. > 208 failures 5 new passes -- > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20%28Tests%29/builds/14296 > r197751 was a commit for the branch WebKitGTK/webkit-2.12 (not for > trunk) > r197750 242 failures 21 new passes 11 flakes -- > https://build.webkit.org/builders/GTK%20Linux%2064- > bit%20Release%20%28Tests%29/builds/14295 I see, thanks for the comment, I'll review what is the problem, probably it was before there because this patch is not adding code.

I believe this should be fixed now with bug 155236