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

Issue 1040643003: Fix java_cpp_enum.gypi so that source_file changes will rebuild dependencies. (Closed)

Created:
5 years, 9 months ago by mlamouri (slow - plz ping)
Modified:
5 years, 8 months ago
Reviewers:
*cjhopman, Nico
CC:
chromium-reviews, jbudorick+watch_chromium.org, klundberg+watch_chromium.org, yfriedman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix java_cpp_enum.gypi so that source_file changes will rebuild dependencies. As a result, if a generated .java class had to be added to a .jar file, the java class would be rebuilt but the jar file would not be updated. This has caused landmines in the past: https://codereview.chromium.org/899403002 It is fixing the underlying issue blocking this: https://codereview.chromium.org/1003143002 BUG=457038 Committed: https://crrev.com/86c3b5b8bf1aa48335f8213bcaa85ce45748a8a4 Cr-Commit-Position: refs/heads/master@{#322717}

Patch Set 1 #

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

Messages

Total messages: 12 (5 generated)
mlamouri (slow - plz ping)
cjhopman@, could you review this change as a de-facto owner of this file? klundberg@, could ...
5 years, 9 months ago (2015-03-28 20:24:54 UTC) #3
mlamouri (slow - plz ping)
On 2015/03/28 at 20:24:54, Mounir Lamouri wrote: > cjhopman@, could you review this change as ...
5 years, 9 months ago (2015-03-28 20:25:44 UTC) #4
cjhopman
lgtm
5 years, 9 months ago (2015-03-29 01:46:02 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1040643003/1
5 years, 8 months ago (2015-03-29 11:14:12 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 8 months ago (2015-03-29 11:16:56 UTC) #10
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/86c3b5b8bf1aa48335f8213bcaa85ce45748a8a4 Cr-Commit-Position: refs/heads/master@{#322717}
5 years, 8 months ago (2015-03-29 11:18:35 UTC) #11
Nico
5 years, 8 months ago (2015-03-30 19:03:22 UTC) #12
Message was sent while issue was closed.
lgtm, thanks for the fix! 

(I think this still isn't quite right if an enum is removed from a header. In
that case, the old generated .java file will stay around in the build dir and
still end up in the jar. This would probably be easier to get right if the
outputs were explicitly listed in the gyp file, then they could be explicitly
passed to the step that compiles .java files to .jar files – kind of like the gn
build does this.)

Powered by Google App Engine
This is Rietveld 408576698