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

Issue 578363002: Fix generation of java enums (Closed)

Created:
6 years, 3 months ago by cjhopman
Modified:
6 years, 3 months ago
CC:
chromium-reviews, klundberg+watch_chromium.org, yfriedman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Fix generation of java enums java_cpp_enum.gypi was creating all enums in the same root folder and then adding that folder to its dependents generated_src_dirs. This has two issues: first, incremental builds will include stale files when things are moved/renamed/etc. second, all libraries that depend on such an enum target will actually compile and include all the enum targets (and in fact may even include different versions of one enum in different libraries). This change just makes each such target use its own unique directory (this will still have the stale enum issue when renaming an enum if the target name doesn't change, but that is very rare). The GN version already used unique directories. TBR=mkosiba Committed: https://crrev.com/e98bcec0ee81f43526ad06a2e27a1cdc3ef7f190 Cr-Commit-Position: refs/heads/master@{#295595}

Patch Set 1 #

Patch Set 2 : Fix aosp :/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M android_webview/java_library_common.mk View 1 1 chunk +2 lines, -2 lines 0 comments Download
M build/android/java_cpp_enum.gypi View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 14 (5 generated)
cjhopman
tedchoc: *
6 years, 3 months ago (2014-09-18 17:41:15 UTC) #2
Ted C
lgtm
6 years, 3 months ago (2014-09-18 17:44:41 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/578363002/1
6 years, 3 months ago (2014-09-18 17:58:43 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: android_aosp on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/builds/16789)
6 years, 3 months ago (2014-09-18 18:34:21 UTC) #7
cjhopman
mkosiba TBRed for android_webview/
6 years, 3 months ago (2014-09-18 20:33:27 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/578363002/20001
6 years, 3 months ago (2014-09-18 20:35:16 UTC) #11
commit-bot: I haz the power
Committed patchset #2 (id:20001) as 964bbc7f4f845d3928039e92378586a50b25338d
6 years, 3 months ago (2014-09-18 23:20:43 UTC) #12
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/e98bcec0ee81f43526ad06a2e27a1cdc3ef7f190 Cr-Commit-Position: refs/heads/master@{#295595}
6 years, 3 months ago (2014-09-18 23:21:24 UTC) #13
mkosiba (inactive)
6 years, 3 months ago (2014-09-19 12:46:52 UTC) #14
Message was sent while issue was closed.
thanks!

Powered by Google App Engine
This is Rietveld 408576698