Bug 168335 – Make libwebrtc.dylib

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Make libwebrtc.dylib

[Alex Christensen]

Created attachment 301548 [details]
Patch

[WebKit Commit Bot]

Attachment 301548 [details] did not pass style-queue:


ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/common_video/libyuv/include/webrtc_libyuv.h:21:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/base/location.h:16:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/RTCLogging.mm:41:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/RTCLogging.mm:44:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/p2p/client/basicportallocator.h:19:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 5 in 29 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 301548 [details]
Patch

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

> Source/ThirdParty/libwebrtc/Source/webrtc/common_types.h:32
> +#if defined(WEBRTC_EXPORT) && defined(WIN32)
>  #define WEBRTC_DLLEXPORT _declspec(dllexport)
>  #elif defined(WEBRTC_DLL)

A little strange to change the condition for the elif clause like this. Maybe wrap the whole block starting on line 30 in #if defined WIN32?

> Source/ThirdParty/libwebrtc/Source/webrtc/base/export.h:9
> + *  Copyright (c) 2017 Apple Inc. All Rights Reserved.
> + *
> + *  Use of this source code is governed by a BSD-style license
> + *  that can be found in the LICENSE file in the root of the source
> + *  tree. An additional intellectual property rights grant can be found
> + *  in the file PATENTS.  All contributing project authors may
> + *  be found in the AUTHORS file in the root of the source tree.
> + */

Is this the right license to use for Apple-contriubuted code in this directory?

> Source/ThirdParty/libwebrtc/Source/webrtc/base/export.h:10
> +#ifndef WEBRTC_BASE_EXPORT_H_

Need a newline before this. Why not #pragma once?

> Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj:12290
> +				MACH_O_TYPE = mh_dylib;

This should be the default for the product type so you shouldn’t specify it here (nor in the xcconfig).

> Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj:12314
> +				MACH_O_TYPE = mh_dylib;

Ditto.

> Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj:12322
> +				MACH_O_TYPE = mh_dylib;

Ditto.

Comment on attachment 301548 [details]
Patch

OK assuming you address the comments.

[Alex Christensen]

(In reply to comment #3)
> > Source/ThirdParty/libwebrtc/Source/webrtc/base/export.h:9
> > + *  Copyright (c) 2017 Apple Inc. All Rights Reserved.
> > + *
> > + *  Use of this source code is governed by a BSD-style license
> > + *  that can be found in the LICENSE file in the root of the source
> > + *  tree. An additional intellectual property rights grant can be found
> > + *  in the file PATENTS.  All contributing project authors may
> > + *  be found in the AUTHORS file in the root of the source tree.
> > + */
> 
> Is this the right license to use for Apple-contriubuted code in this
> directory?
We did the same in https://bugs.webkit.org/show_bug.cgi?id=167257
> 
> > Source/ThirdParty/libwebrtc/Source/webrtc/base/export.h:10
> > +#ifndef WEBRTC_BASE_EXPORT_H_
> 
> Need a newline before this. Why not #pragma once?
This matches libwebrtc code.

[Alex Christensen]

http://trac.webkit.org/r212326

[Ryan Haddad]

Reverted r212326 for reason:

This change broke certain build configurations.

Committed r212333: <http://trac.webkit.org/changeset/212333>

[Ryan Haddad]

(In reply to comment #7)
> Reverted r212326 for reason:
> 
> This change broke certain build configurations.
> 
> Committed r212333: <http://trac.webkit.org/changeset/212333>

rdar://problem/30523442

[Radar WebKit Bug Importer]

<rdar://problem/30535569>

[youenn fablet]

I also had to twaek Source/ThirdParty/libwebrtc/Source/webrtc/base/checks.h file for the Debug build to work:

=====
 #define WEBRTC_BASE_CHECKS_H_

+#include "webrtc/base/export.h"
 #include "webrtc/typedefs.h"
=====

and

=====
-class FatalMessage {
+class WEBRTC_EXPORT FatalMessage {
=====

[Alex Christensen]

Created attachment 301664 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/r212401

[youenn fablet]

Comment on attachment 301664 [details]
Patch

Let's go!

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

> Source/ThirdParty/libwebrtc/Configurations/libwebrtc.xcconfig:24
> +EXCLUDED_SOURCE_FILE_NAMES[sdk=macosx*] = audio_device_ios.mm voice_processing_audio_unit.mm audio_device_not_implemented_ios.mm RTCAudioSessionConfiguration.m RTCAudioSessionDelegateAdapter.mm RTCAudioSession.mm RTCAudioSession+Configuration.mm RTCUIApplication.mm;

All RTCXX are now gone so we should not need to have them here anymore.

> Source/ThirdParty/libwebrtc/Source/webrtc/base/checks.h:214
> +class WEBRTC_EXPORT FatalMessageVoidify {

I think it is FatalMessage not FatalMessageVoidify that needs to be exported.