Switching deployment target to 10.8 causes compile error in installer. |
||
Issue description
/Users/erikchen/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF obj/chrome/installer/mac/app/mac_installer_base/Unpacker.o.d -DV8_DEPRECATION_WARNINGS -DENABLE_NOTIFICATIONS -DENABLE_PEPPER_CDMS -DENABLE_PLUGINS=1 -DENABLE_PDF=1 -DENABLE_PRINTING=1 -DENABLE_BASIC_PRINTING=1 -DENABLE_PRINT_PREVIEW=1 -DENABLE_SPELLCHECK=1 -DUSE_BROWSER_SPELLCHECKER=1 -DNO_TCMALLOC -DUSE_EXTERNAL_POPUP_MENU=1 -DENABLE_WEBRTC=1 -DENABLE_EXTENSIONS=1 -DENABLE_TASK_MANAGER=1 -DENABLE_THEMES=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_SUPERVISED_USERS=1 -DENABLE_SERVICE_DISCOVERY=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=282097-1 -DCR_XCODE_VERSION=0720 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../.. -Igen -fno-strict-aliasing -fstack-protector-strong -fcolor-diagnostics -arch x86_64 -Wall -Werror -Wextra -Wpartial-availability -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-deprecated-register -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-shift-negative-value -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -O0 -g1 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk -mmacosx-version-min=10.8 -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene -Wstring-conversion -std=c99 -Wobjc-missing-property-synthesis -fobjc-arc -c ../../chrome/installer/mac/app/Unpacker.m -o obj/chrome/installer/mac/app/mac_installer_base/Unpacker.o
../../chrome/installer/mac/app/Unpacker.m:149:3: error: 'release' is unavailable: not available in automatic reference counting mode
dispatch_release(unpack_dq_);
^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/usr/include/dispatch/object.h:194:38: note: expanded from macro 'dispatch_release'
_dispatch_object_validate(_o); [_o release]; })
^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/usr/include/objc/NSObject.h:37:1: note: 'release' has been explicitly marked unavailable here
- (oneway void)release OBJC_ARC_UNAVAILABLE;
^
../../chrome/installer/mac/app/Unpacker.m:149:3: error: ARC forbids explicit message send of 'release'
dispatch_release(unpack_dq_);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/usr/include/dispatch/object.h:194:38: note: expanded from macro 'dispatch_release'
_dispatch_object_validate(_o); [_o release]; })
,
Sep 27 2016
I made a CL for this, but it looks like the behavior in the SDK is switched on deployment target — as in, removing dispatch_release creates a leak with a target <10.8. It doesn't matter for the installer, but in case there are others, is there a good way to lump changes in with the bump so that they'll be applied together? https://codereview.chromium.org/2370273003
,
Sep 27 2016
In cases like that, you should switch on the SDK or the DT (careful, be sure you use the right one!) with #ifdefs. Once we’ve switched over and are committed to not rolling back, it’s easy to cruise on through the codebase and get rid of all of the dead branches.
,
Sep 27 2016
,
Sep 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0e482836bff9bb98a972cbc3c2651669d69e7254 commit 0e482836bff9bb98a972cbc3c2651669d69e7254 Author: sdy <sdy@chromium.org> Date: Tue Sep 27 21:45:16 2016 Drop dispatch_release(): ARC manages dispatch queues with a deployment target >= 10.8 Note: this is a leak with our current deployment target (but not a big deal for the short-lived installer). BUG= 650807 Review-Url: https://codereview.chromium.org/2370273003 Cr-Commit-Position: refs/heads/master@{#421345} [modify] https://crrev.com/0e482836bff9bb98a972cbc3c2651669d69e7254/chrome/installer/mac/app/Unpacker.m |
||
►
Sign in to add a comment |
||
Comment 1 by sdy@chromium.org
, Sep 27 2016Owner: sdy@chromium.org
Status: Started (was: Untriaged)