New issue
Advanced search Search tips

Issue 711670 link

Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

macOS build fails when including openssl/opensslconf.h

Project Member Reported by ellyjo...@chromium.org, Apr 14 2017

Issue description

Filing a bug to track the issue described here: https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/Bj3hmbrS02k

FAILED: obj/chrome/browser/ui/views/views/tab_renderer_data.o 
/Users/ellyjones/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/chrome/browser/ui/views/views/tab_renderer_data.o.d -DCHROME_VIEWS_IMPLEMENTATION -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DDISABLE_NACL -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DENABLE_MEDIA_ROUTER=1 -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"299960-1\" -DCR_XCODE_VERSION=0810 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DCOMPONENT_BUILD -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSKIA_DLL -DGR_GL_IGNORE_ES3_MSAA=0 -DSK_SUPPORT_GPU=1 -DSK_BUILD_FOR_MAC -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -I../.. -Igen -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/skia/include/gpu -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -fno-strict-aliasing -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -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-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -O2 -fno-omit-frame-pointer -gdwarf-2 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9 -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-auto-raw-pointer -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -fvisibility-inlines-hidden -std=c++11 -stdlib=libc++ -fno-rtti -fno-exceptions -c ../../chrome/browser/ui/views/tabs/tab_renderer_data.cc -o obj/chrome/browser/ui/views/views/tab_renderer_data.o
In file included from ../../chrome/browser/ui/views/tabs/tab_renderer_data.cc:5:
In file included from ../../chrome/browser/ui/views/tabs/tab_renderer_data.h:9:
In file included from ../../chrome/browser/ui/tabs/tab_utils.h:13:
In file included from ../../content/public/browser/web_contents_user_data.h:10:
In file included from ../../content/public/browser/web_contents.h:21:
In file included from ../../content/public/browser/navigation_controller.h:24:
In file included from ../../content/public/common/referrer.h:10:
In file included from ../../net/url_request/url_request.h:26:
In file included from ../../net/base/net_error_details.h:9:
In file included from ../../net/http/http_response_info.h:14:
In file included from ../../net/ssl/ssl_info.h:20:
In file included from ../../net/ssl/ssl_config.h:12:
In file included from ../../net/cert/x509_certificate.h:25:
../../third_party/boringssl/src/include/openssl/base.h:68:10: fatal error: 'openssl/opensslconf.h' file not found
#include <openssl/opensslconf.h>
         ^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

The linked thread mentions patching https://codereview.chromium.org/2812013003/ and https://codereview.chromium.org/2806293004/ to fix this.
 

Comment 1 by thakis@chromium.org, Apr 14 2017

Components: Build
Probably an issue with the 10.12 SDK?
Building on my desktop with the 10.12 SDK failed in this manner, but building on my laptop with the 10.11 SDK succeeds, so that seems like a good guess.

Comment 3 by thakis@chromium.org, Apr 14 2017

Cc: davidben@chromium.org
Ah, we probably want that include to pick up third_party/boringssl/src/include/openssl/opensslconf.h but we don't set up include dirs correcty, so on 10.11 we get "lucky" and it falls through to the system version instead, which probably doesn't exist in 10.12?

Should we include boringssl headers with <> at all (instead of "")?
mattm@'s CLs fix it so I imagine there is already a fix in flight, just not landed yet.
thakis: The header pulling in <openssl/opensslconf.h> is within BoringSSL itself which can't assume it lives at "third_party/boringssl/...". It's all a mess. Our monorepo-relative #include preference doesn't really mesh well with external projects. :-/

This CL, once I do another DEPS roll, will avoid failures here from being platform-specific, which should avoid masking problems due to us getting "lucky".
https://boringssl.googlesource.com/boringssl/+/d403be92a436a45bd9be2e016cb250e7157dfeb3
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2dbc3ed538ce6f3470af81dab163ca6e3c018bbc

commit 2dbc3ed538ce6f3470af81dab163ca6e3c018bbc
Author: mattm <mattm@chromium.org>
Date: Fri Apr 14 21:43:12 2017

Fix some build deps for //extensions/browser/api/guest_view, //extensions/common, //extensions/renderer.

//extensions/browser/api/guest_view: guest_view_internal_api.h includes extensions/browser/extension_function.h. The same problem exists for other subdirs of e/b/api/guest_view/. Cannot add //extensions/browser as a dep to //extensions/browser/api/guest_view because that would create a circular dependency.
Merge all those subdir files into //extensions/browser/api and specify all the public_deps of extension_function.h there. (This doesn't fix the underlying circular includes issue, but does clean things up a little bit.)

Example failure:
In file included from gen/extensions/browser/api/generated_api_registration.cc:9:
In file included from ../../extensions/browser/api/guest_view/web_view/web_view_internal_api.h:15:
In file included from ../../extensions/browser/guest_view/web_view/web_view_guest.h:14:
In file included from ../../components/guest_view/browser/guest_view.h:9:
In file included from ../../components/guest_view/browser/guest_view_base.h:15:
In file included from ../../components/zoom/zoom_observer.h:8:
In file included from ../../components/zoom/zoom_controller.h:16:
In file included from ../../content/public/browser/web_contents_observer.h:15:
In file included from ../../content/public/browser/navigation_controller.h:24:
In file included from ../../content/public/common/referrer.h:10:
In file included from ../../net/url_request/url_request.h:26:
In file included from ../../net/base/net_error_details.h:9:
In file included from ../../net/http/http_response_info.h:14:
In file included from ../../net/ssl/ssl_info.h:20:
In file included from ../../net/ssl/ssl_config.h:12:
In file included from ../../net/cert/x509_certificate.h:25:
../../third_party/boringssl/src/include/openssl/base.h:68:10: fatal error: 'openssl/opensslconf.h' file not found

//extensions/common: move //content/public/common to public_deps, as extensions/common/extension_messages.h includes files from there.
//extensions/renderer: add dep on //extensions/common, as many files in extensions/renderer include extensions/common/extension_messages.h.

Example failure for both of those:
In file included from ../../extensions/renderer/service_worker_request_sender.cc:9:
In file included from ../../extensions/common/extension_messages.h:16:
In file included from ../../content/public/common/common_param_traits.h:24:
In file included from ../../content/public/common/common_param_traits_macros.h:13:
In file included from ../../content/public/common/referrer.h:10:
In file included from ../../net/url_request/url_request.h:26:
In file included from ../../net/base/net_error_details.h:9:
In file included from ../../net/http/http_response_info.h:14:
In file included from ../../net/ssl/ssl_info.h:20:
In file included from ../../net/ssl/ssl_config.h:12:
In file included from ../../net/cert/x509_certificate.h:25:
../../third_party/boringssl/src/include/openssl/base.h:68:10: fatal error: 'openssl/opensslconf.h' file not found

BUG= 711670 

Review-Url: https://codereview.chromium.org/2812013003
Cr-Commit-Position: refs/heads/master@{#464802}

[modify] https://crrev.com/2dbc3ed538ce6f3470af81dab163ca6e3c018bbc/extensions/browser/api/BUILD.gn
[delete] https://crrev.com/18281c41a2b809619b3f533c76033e1a14729944/extensions/browser/api/guest_view/BUILD.gn
[delete] https://crrev.com/18281c41a2b809619b3f533c76033e1a14729944/extensions/browser/api/guest_view/app_view/BUILD.gn
[delete] https://crrev.com/18281c41a2b809619b3f533c76033e1a14729944/extensions/browser/api/guest_view/extension_view/BUILD.gn
[delete] https://crrev.com/18281c41a2b809619b3f533c76033e1a14729944/extensions/browser/api/guest_view/web_view/BUILD.gn
[modify] https://crrev.com/2dbc3ed538ce6f3470af81dab163ca6e3c018bbc/extensions/common/BUILD.gn
[modify] https://crrev.com/2dbc3ed538ce6f3470af81dab163ca6e3c018bbc/extensions/renderer/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/487d548c3639002dbee792c672e0460da257bb17

commit 487d548c3639002dbee792c672e0460da257bb17
Author: mattm <mattm@chromium.org>
Date: Mon Apr 17 22:56:34 2017

Add //content/public/browser deps to //chrome/browser/ui/views

chrome/browser/ui/views/tabs/tab_renderer_data.h includes chrome/browser/ui/tabs/tab_utils.h which includes content/public/browser/web_contents_user_data.h.

chrome/browser/ui/views cannot have a dep on chrome/browser/ui because that would result in a circular dependency.

Including the content/public/browser dep here fixes the issue, though it's a bit ugly of a solution.

BUG= 711670 

Review-Url: https://codereview.chromium.org/2806293004
Cr-Commit-Position: refs/heads/master@{#465051}

[modify] https://crrev.com/487d548c3639002dbee792c672e0460da257bb17/chrome/browser/ui/views/BUILD.gn

https://boringssl.googlesource.com/boringssl/+/d403be92a436a45bd9be2e016cb250e7157dfeb3 should now have rolled into Chromium so, unless we have SDK-specific #includes of BoringSSL headers anywhere, hopefully any future variants of this problem should happen uniformly across old and new SDKs and get caught at the CQ.

(I was going to say uniformly across platforms, but we definitely have platform-specific #includes, so I can't claim that one.)

Comment 9 by mattm@chromium.org, Apr 18 2017

Status: Fixed (was: Started)
 Issue 731816  has been merged into this issue.

Sign in to add a comment