Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2)

Issue 2110943002: Revert of 🎊 Have build_config targets depend only on other build_config targets (Closed)

Created:
4 years, 5 months ago by Pete Williamson
Modified:
4 years, 5 months ago
Reviewers:
brettw, agrieve
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, Peter Beverloo, jam, darin-cc_chromium.org, mikecase+watch_chromium.org, mkwst+moarreviews-shell_chromium.org, jbudorick+watch_chromium.org, jochen+watch_chromium.org, pkotwicz
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of 🎊 Have build_config targets depend only on other build_config targets (patchset #3 id:40001 of https://codereview.chromium.org/2095913003/ ) Reason for revert: Sheriff: reverting on suspicion of breaking the android build. It looks like the apk packaging step fails when a required component is not found. This change looked like a likely culprit since it changes the naming conventions, and the problem was file not found. Link to failed build here: https://build.chromium.org/p/chromium/builders/Android/builds/58342 Relevant section of build logs here: FAILED: gen/android_webview/system_webview_apk__create_incremental__package.d gen/android_webview/system_webview_apk/system_webview_apk.apk_intermediates_incremental.unfinished.apk python ../../build/android/gyp/apkbuilder.py --depfile gen/android_webview/system_webview_apk__create_incremental__package.d --resource-apk=gen/android_webview/system_webview_apk/system_webview_apk.apk_intermediates_incremental.ap_ --output-apk=gen/android_webview/system_webview_apk/system_webview_apk.apk_intermediates_incremental.unfinished.apk --assets=@FileArg\(gen/android_webview/system_webview_apk.build_config:assets\) --uncompressed-assets=@FileArg\(gen/android_webview/system_webview_apk.build_config:uncompressed_assets\) --dex-file=gen/build/android/incremental_install/bootstrap.dex --android-abi=armeabi-v7a --native-lib-placeholders=\[\"libfix.crbug.384638.so\"\] Traceback (most recent call last): File "../../build/android/gyp/apkbuilder.py", line 311, in <module> main(sys.argv[1:]) File "../../build/android/gyp/apkbuilder.py", line 307, in main depfile_deps=depfile_deps) File "/b/build/slave/Android/build/src/build/android/gyp/util/build_utils.py", line 527, in CallAndWriteDepfileIfStale pass_changes=True) File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 63, in CallAndRecordIfStale new_metadata.AddFile(path, _Md5ForPath(path)) File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 381, in _Md5ForPath _UpdateMd5ForFile(md5, path) File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 362, in _UpdateMd5ForFile with open(path, 'rb') as infile: IOError: [Errno 2] No such file or directory: u'snapshot_blob.bin' [34308/48394] ACTION //android_webview/test:android_webview_apk__create_incremental__package(//build/toolchain/android:arm) FAILED: gen/android_webview/test/android_webview_apk__create_incremental__package.d gen/android_webview/test/android_webview_apk/android_webview_apk.apk_intermediates_incremental.unfinished.apk python ../../build/android/gyp/apkbuilder.py --depfile gen/android_webview/test/android_webview_apk__create_incremental__package.d --resource-apk=gen/android_webview/test/android_webview_apk/android_webview_apk.apk_intermediates_incremental.ap_ --output-apk=gen/android_webview/test/android_webview_apk/android_webview_apk.apk_intermediates_incremental.unfinished.apk --assets=@FileArg\(gen/android_webview/test/android_webview_apk.build_config:assets\) --uncompressed-assets=@FileArg\(gen/android_webview/test/android_webview_apk.build_config:uncompressed_assets\) --dex-file=gen/build/android/incremental_install/bootstrap.dex --android-abi=armeabi-v7a --native-lib-placeholders=\[\"libfix.crbug.384638.so\"\] Traceback (most recent call last): File "../../build/android/gyp/apkbuilder.py", line 311, in <module> main(sys.argv[1:]) File "../../build/android/gyp/apkbuilder.py", line 307, in main depfile_deps=depfile_deps) File "/b/build/slave/Android/build/src/build/android/gyp/util/build_utils.py", line 527, in CallAndWriteDepfileIfStale pass_changes=True) File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 63, in CallAndRecordIfStale new_metadata.AddFile(path, _Md5ForPath(path)) File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 381, in _Md5ForPath _UpdateMd5ForFile(md5, path) File "/b/build/slave/Android/build/src/build/android/gyp/util/md5_check.py", line 362, in _UpdateMd5ForFile with open(path, 'rb') as infile: IOError: [Errno 2] No such file or directory: u'snapshot_blob.bin' Original issue's description: > Have build_config targets depend only on other build_config targets > > This works by having android target names follow a naming convention. > If a target name matches, then it is assumed to have a .build_config file. > gn gen will raise an error if it doesn't. Likewise, if a target name > does not match the filters, but does create a .build_config file, gn gen > will fail on an assert. > > This change adds a bunch of naming-convention exceptions, which will > be addressed in subsequent commits. > > Finally, why make this change? This allows all .build_config targets to be > generated in just a few seconds, which will allow fast generation of > build.gradle files for Android Studio integration. > > BUG=620034 > > Committed: https://crrev.com/1029ca88161324eff7a62b1e2eecc84d5acf0f6c > Cr-Commit-Position: refs/heads/master@{#402934} TBR=brettw@chromium.org,agrieve@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=620034 Committed: https://crrev.com/70586184529bdc819367bcdcb4dbaf41d8325559 Cr-Commit-Position: refs/heads/master@{#402949}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -155 lines) Patch
M build/android/gyp/write_build_config.py View 2 chunks +12 lines, -3 lines 0 comments Download
M build/config/android/internal_rules.gni View 9 chunks +20 lines, -115 lines 0 comments Download
M build/config/android/rules.gni View 10 chunks +17 lines, -34 lines 0 comments Download
M chromecast/browser/android/BUILD.gn View 1 chunk +3 lines, -1 line 0 comments Download
M content/shell/android/BUILD.gn View 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
Pete Williamson
Created Revert of 🎊 Have build_config targets depend only on other build_config targets
4 years, 5 months ago (2016-06-29 22:09:17 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2110943002/1
4 years, 5 months ago (2016-06-29 22:15:46 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 5 months ago (2016-06-29 22:20:26 UTC) #5
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/70586184529bdc819367bcdcb4dbaf41d8325559 Cr-Commit-Position: refs/heads/master@{#402949}
4 years, 5 months ago (2016-06-29 22:22:06 UTC) #7
agrieve
4 years, 5 months ago (2016-06-30 00:08:13 UTC) #8
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2109593005/ by agrieve@chromium.org.

The reason for reverting is: Will fix....

Powered by Google App Engine
This is Rietveld 408576698