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

Issue 194293002: Revert 256097 "android: Pass (non-generated) .java files via a g..." (Closed)

Created:
6 years, 9 months ago by Nico
Modified:
6 years, 9 months ago
Reviewers:
Nico
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 256097 "android: Pass (non-generated) .java files via a g..." Breaks target device_extras_apk on the upstream bot, due to the src/ issue mentioned on the review: A java target contains another java target in a subdirectory, and due to the missing src/ the java files in the subtarget get picked up twice. > android: Pass (non-generated) .java files via a gyp filelist to javac.py > > Before this CL, the javac action rules called `find javadir -name "*.java"` to > compute the action's inputs, and passed javadir to javac.py. The script then > again looked for all .java files in that directory. One issue with that approach > is that if a java file gets removed, the javac.py action didn't get executed again > since actions are only run if their commandline changes or an input file is newer > than an output file – a disappearing input doesn't mark an action dirty. (As > workaround, the md5 hash of all .java files is currently passed in an > --ignore parameter, so removal of a java file will lead to a changing > commandline, which _will_ cause a rebuild.) > > After this CL, the result of `find javadir -name "*.java"` is written to a gyp > filelist (see https://codereview.chromium.org/27418008/), and the filelist > is passed to javac.py. gyp writes filelists exactly if their contents would > change, so removal of java file will change the mtime of the filelist, which will > cause a rebuild. Another advantage is that the list of java files is computed in > only one place. > > This CL doesn't yet remove the md5 hack, but it makes it possible to do so > in a follow-up change. > > (This approach doesn't work for generated java files, because these might > not yet exist at gyp time. Rename --src-dirs to --src-gendirs and keep it in > use for generated files. The dependency handling of generated java files is > done through stamp files, so the md5 hack isn't needed for them.) > > No intended behavior change. > > BUG=177552 > R=cjhopman@chromium.org > > Review URL: https://codereview.chromium.org/193693002 TBR=thakis@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256127

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -18 lines) Patch
M trunk/src/build/android/gyp/javac.py View 2 chunks +3 lines, -6 lines 0 comments Download
M trunk/src/build/java.gypi View 2 chunks +7 lines, -4 lines 0 comments Download
M trunk/src/build/java_apk.gypi View 2 chunks +7 lines, -8 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Nico
6 years, 9 months ago (2014-03-11 04:29:46 UTC) #1
Nico
6 years, 9 months ago (2014-03-11 04:29:53 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 manually as r256127 (tree was closed).

Powered by Google App Engine
This is Rietveld 408576698