search

flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
164.79k stars 27.16k forks source link

Flutter Mac IOS Engine bot fails to compile new revision of zlib #106352

Open aam opened 2 years ago

aam commented 2 years ago

Fltuter engine Mac iOS builder fails on new revision of zlib:

From logs of a dart->engine roll

[1617/4268] CC obj/third_party/zlib/chrome_zlib.adler32.o
FAILED: obj/third_party/zlib/chrome_zlib.adler32.o 
clang -MD -MF obj/third_party/zlib/chrome_zlib.adler32.o.d -DARM_OS_IOS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DZLIB_IMPLEMENTATION -DADLER32_SIMD_NEON -DINFLATE_CHUNK_SIMD_NEON -DINFLATE_CHUNK_READ_64LE -DDEFLATE_SLIDE_HASH_NEON -I../.. -Igen -I../../third_party/zlib -isysroot /opt/s/w/ir/cache/osx_sdk/XCode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator15.0.sdk -mios-simulator-version-min=11.0 -fno-strict-aliasing -arch arm64 -fcolor-diagnostics -fvisibility=hidden -stdlib=libc++ -Wstring-conversion -Wnewline-eof -Os -fno-ident -fdata-sections -ffunction-sections -g2 -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wunguarded-availability -Wno-deprecated-declarations -Wno-deprecated-non-prototype -Wno-incompatible-pointer-types -Wunused-variable -std=c99  -c ../../third_party/zlib/adler32.c -o obj/third_party/zlib/chrome_zlib.adler32.o
error: unknown warning option '-Wno-deprecated-non-prototype' [-Werror,-Wunknown-warning-option]
[1618/4268] CXX obj/third_party/skia/modules/skunicode/src/skunicode.SkUnicode_icu_builtin.o
[1619/4268] CC obj/third_party/zlib/chrome_zlib.compress.o
FAILED: obj/third_party/zlib/chrome_zlib.compress.o 
clang -MD -MF obj/third_party/zlib/chrome_zlib.compress.o.d -DARM_OS_IOS -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DZLIB_IMPLEMENTATION -DADLER32_SIMD_NEON -DINFLATE_CHUNK_SIMD_NEON -DINFLATE_CHUNK_READ_64LE -DDEFLATE_SLIDE_HASH_NEON -I../.. -Igen -I../../third_party/zlib -isysroot /opt/s/w/ir/cache/osx_sdk/XCode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator15.0.sdk -mios-simulator-version-min=11.0 -fno-strict-aliasing -arch arm64 -fcolor-diagnostics -fvisibility=hidden -stdlib=libc++ -Wstring-conversion -Wnewline-eof -Os -fno-ident -fdata-sections -ffunction-sections -g2 -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wunguarded-availability -Wno-deprecated-declarations -Wno-deprecated-non-prototype -Wno-incompatible-pointer-types -Wunused-variable -std=c99  -c ../../third_party/zlib/compress.c -o obj/third_party/zlib/chrome_zlib.compress.o
error: unknown warning option '-Wno-deprecated-non-prototype' [-Werror,-Wunknown-warning-option]

This seems to indicate that clang toolchain on that bot is older compared to other Flutter engine mac bots.

@iskakaushik

iskakaushik commented 2 years ago

cc: @zanderso for triage

zanderso commented 2 years ago

It's intentional that that build is using the version of clang coming from the version of Xcode that we use in CI. The build flags will need to be adjusted.

zanderso commented 2 years ago

@aam @iskakaushik Could either of you link to the BUILD.gn file that introduces the problematic flag?

aam commented 2 years ago

https://chromium.googlesource.com/chromium/src/third_party/zlib/+/a41a88a1b9153717ceb3940e0da7af7cb70f757e/BUILD.gn, is that what you mean ?

aam commented 2 years ago

introduced in https://chromium-review.googlesource.com/c/chromium/src/+/3646604

zanderso commented 2 years ago

https://chromium-review.googlesource.com/c/chromium/src/+/3646604

I don't think that's where -Wno-deprecated-non-prototype was introduced. It looks like the flag predates that CL.

But, taking a look at how this is done there, I don't think we'll be able to role zlib forward since we have to build with a version of clang that doesn't know about that flag. An alternative might be to introduce a secondary BUILD.gn file, but then that becomes tech-debt, which I think we should avoid. If there's no reason to role zlib forward right now, I think we should just stay where we are. Dropping this to P4 assuming the revert is going to role through without issue.

aam commented 2 years ago

I don't think that's where -Wno-deprecated-non-prototype was introduced. It looks like the flag predates that CL.

Ah, right: https://chromium-review.googlesource.com/c/chromium/src/+/3615937 is the one.

I don't think we'll be able to role zlib forward since we have to build with a version of clang that doesn't know about that flag

How much of a problem is the fact that we use different versions of clang on different bots?

If there's no reason to role zlib forward right now, I think we should just stay where we are.

Reason the zlib was rolled forward was to make sure it compiles with latest clang.

@rmacnak-google

rmacnak-google commented 2 years ago

This toolchain discrepancy is probably also responsible for https://github.com/flutter/engine/pull/35310 failing.