Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
12701012:
Removed unused ant stuff (Closed)
|
Created:
7 years, 9 months ago by cjhopman Modified:
7 years, 9 months ago CC:
chromium-reviews, klundberg+watch_chromium.org, frankf+watch_chromium.org, bulach+watch_chromium.org, yfriedman+watch_chromium.org, ilevy+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@sdk_targets Visibility:
Public. |
DescriptionRemoved unused ant stuff
This removes a bunch of the ant properties and targets that are never
used in our builds. These are related to: installing apks, running
tests, emma instrumentation, lint, help, clean, building library
dependencies, building main apk for test apks, renderscript, and asking
for keystore password.
BUG=158821
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=187735
Patch Set 1 : #
Total comments: 20
Patch Set 2 : #Patch Set 3 : Rebase + Fix compile failure #Messages
Total messages: 11 (0 generated)
https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... File build/android/ant/apk-build.xml (left): https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:3: Copyright (C) 2005-2008 The Android Open Source Project Copyright. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:141: <property name="dex.force.jumbo" value="false" /> can remove this. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:154: <property name="manifestmerger.enabled" value="false" /> Can remove this? https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:168: <property name="lint.out.html" value="bin/lint.html" /> remove lint.out.* and there comments/usages. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:233: <condition property="exe" value=".exe" else=""><os family="windows" /></condition> we can remove the windows development as well. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:1664: <echo> 'ant nodeps debug'</echo> We should only remove documentation for targets which are getting deleted.
lgtm https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... File build/android/ant/apk-build.xml (left): https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:657: <not><isset property="dont.do.deps" /></not> Can you get rid of this property now? I set it in build/android/ant/common.xml to quite a subant warning but I'm guessing it becomes unused https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:1496: <!-- ********** Run Lint on the project ********* --> Keep this around? Or you'd rather manually invoke? https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... File build/android/ant/apk-build.xml (right): https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:222: <property name="lint" location="${android.tools.dir}/lint${bat}" /> Would be interesting to get lint running in the future. I wonder if it overlaps with findbugs at all.
https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... File build/android/ant/apk-build.xml (left): https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:3: Copyright (C) 2005-2008 The Android Open Source Project On 2013/03/11 23:35:03, shashi wrote: > Copyright. ? https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:141: <property name="dex.force.jumbo" value="false" /> On 2013/03/11 23:35:03, shashi wrote: > can remove this. This is still used (it could be inlined below, but I wanted to keep this as a more straightforward deletion of unused stuff). https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:154: <property name="manifestmerger.enabled" value="false" /> On 2013/03/11 23:35:03, shashi wrote: > Can remove this? Same. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:168: <property name="lint.out.html" value="bin/lint.html" /> On 2013/03/11 23:35:03, shashi wrote: > remove lint.out.* and there comments/usages. Done. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:233: <condition property="exe" value=".exe" else=""><os family="windows" /></condition> On 2013/03/11 23:35:03, shashi wrote: > we can remove the windows development as well. Done. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:657: <not><isset property="dont.do.deps" /></not> On 2013/03/11 23:38:06, Yaron wrote: > Can you get rid of this property now? I set it in build/android/ant/common.xml > to quite a subant warning but I'm guessing it becomes unused It's likely still needed for native_test_apk.xml. I'll probably do one more delete/clean up pass on these files (that'll get some of those more non-obvious unneeded things) and I'll check with that change. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:1496: <!-- ********** Run Lint on the project ********* --> On 2013/03/11 23:38:06, Yaron wrote: > Keep this around? Or you'd rather manually invoke? I think we should have this, but it can be it's own gyp action, it's just calling an executable with the source directories and class file directories (also can be used for both apks and libraries). https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:1664: <echo> 'ant nodeps debug'</echo> On 2013/03/11 23:35:03, shashi wrote: > We should only remove documentation for targets which are getting deleted. Well, we have no way to see this help message (except read this file) and for the two targets we use (debug/release) this documentation isn't even correct anyway. https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... File build/android/ant/apk-build.xml (right): https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:222: <property name="lint" location="${android.tools.dir}/lint${bat}" /> On 2013/03/11 23:38:06, Yaron wrote: > Would be interesting to get lint running in the future. I wonder if it overlaps > with findbugs at all. Agreed. See below.
lgtm https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... File build/android/ant/apk-build.xml (left): https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:3: Copyright (C) 2005-2008 The Android Open Source Project On 2013/03/11 23:54:21, cjhopman wrote: > On 2013/03/11 23:35:03, shashi wrote: > > Copyright. > > ? I meant should we mention we have changed original? https://codereview.chromium.org/12701012/diff/4002/build/android/ant/apk-buil... build/android/ant/apk-build.xml:1664: <echo> 'ant nodeps debug'</echo> Alright, since this is just a temporary step and we are not going to depend on ant in future, my concern was if we remove some targets and later do not remove all dependencies on Ant. On 2013/03/11 23:54:21, cjhopman wrote: > On 2013/03/11 23:35:03, shashi wrote: > > We should only remove documentation for targets which are getting deleted. > > Well, we have no way to see this help message (except read this file) and for > the two targets we use (debug/release) this documentation isn't even correct > anyway.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12701012/3
Failed to apply patch for build/android/ant/apk-build.xml:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file build/android/ant/apk-build.xml
Hunk #1 FAILED at 74.
Hunk #2 FAILED at 146.
Hunk #3 FAILED at 182.
Hunk #4 FAILED at 230.
Hunk #5 FAILED at 254.
Hunk #6 FAILED at 407.
Hunk #7 FAILED at 420.
Hunk #8 FAILED at 649.
Hunk #9 FAILED at 723.
Hunk #10 FAILED at 801.
Hunk #11 FAILED at 833.
Hunk #12 FAILED at 1013.
Hunk #13 FAILED at 1081.
Hunk #14 FAILED at 1121.
Hunk #15 FAILED at 1209.
Hunk #16 FAILED at 1268.
Hunk #17 FAILED at 1321.
17 out of 17 hunks FAILED -- saving rejects to file
build/android/ant/apk-build.xml.rej
Patch: build/android/ant/apk-build.xml
Index: build/android/ant/apk-build.xml
diff --git a/build/android/ant/apk-build.xml b/build/android/ant/apk-build.xml
index
1aef20fa20e38feb67dfe7918b676c9b3db18824..a50998615f7d58c5208d2ff2e81cec8b40100110
100644
--- a/build/android/ant/apk-build.xml
+++ b/build/android/ant/apk-build.xml
@@ -74,7 +74,6 @@
<condition property="proguard.config" value="${PROGUARD_FLAGS}">
<istrue value="${PROGUARD_ENABLED}"/>
</condition>
- <!-- TODO(shashishekhar): Enable emma and code-coverage filters. -->
<!-- Set the output directory for the final apk to the ${apks.dir}. -->
<property-location name="out.final.file"
@@ -146,30 +145,12 @@
<property name="java.source" value="1.5" />
<property name="java.compilerargs" value="" />
- <!-- Renderscript options -->
- <property name="renderscript.debug.opt.level" value="O0" />
- <property name="renderscript.release.opt.level" value="O3" />
-
<!-- manifest merger default value -->
<property name="manifestmerger.enabled" value="false" />
- <!-- instrumentation options -->
- <property name="emma.filter" value="" />
-
<!-- Verbosity -->
<property name="verbose" value="false" />
- <!-- Output location of the HTML report for the "lint" target.
- Ideally this would be specified as
- value="${out.dir}/lint.html"
- but we can't make a forward reference to the definition for
- ${out.dir}, and it is not a configurable property (yet).
- -->
- <property name="lint.out.html" value="bin/lint.html" />
-
- <!-- Output location of the XML report for the "lint" target -->
- <property name="lint.out.xml" value="bin/lint.xml" />
-
<!-- ******************************************************* -->
<!-- ********************* Custom Tasks ******************** -->
<!-- ******************************************************* -->
@@ -182,16 +163,6 @@
<!-- Custom tasks -->
<taskdef resource="anttasks.properties" classpathref="android.antlibs" />
- <!-- Emma configuration -->
- <property name="emma.dir" value="${sdk.dir}/tools/lib" />
- <path id="emma.lib">
- <pathelement location="${emma.dir}/emma.jar" />
- <pathelement location="${emma.dir}/emma_ant.jar" />
- </path>
- <taskdef resource="emma_ant.properties" classpathref="emma.lib" />
- <!-- End of emma configuration -->
-
-
<!-- ******************************************************* -->
<!-- ******************* Other Properties ****************** -->
<!-- ******************************************************* -->
@@ -230,21 +201,11 @@
<!-- tools location -->
<property name="android.tools.dir" location="${sdk.dir}/tools" />
<property name="android.platform.tools.dir"
location="${sdk.dir}/platform-tools" />
- <condition property="exe" value=".exe" else=""><os family="windows"
/></condition>
- <condition property="bat" value=".bat" else=""><os family="windows"
/></condition>
<property name="adb" location="${android.platform.tools.dir}/adb${exe}" />
<property name="zipalign" location="${android.tools.dir}/zipalign${exe}" />
<property name="aidl" location="${android.platform.tools.dir}/aidl${exe}"
/>
<property name="aapt" location="${android.platform.tools.dir}/aapt${exe}"
/>
<property name="dx" location="${android.platform.tools.dir}/dx${bat}" />
- <property name="renderscript"
location="${android.platform.tools.dir}/llvm-rs-cc${exe}"/>
- <property name="lint" location="${android.tools.dir}/lint${bat}" />
-
- <!-- Renderscript include Path -->
- <path id="android.renderscript.include.path">
- <pathelement
location="${android.platform.tools.dir}/renderscript/include" />
- <pathelement
location="${android.platform.tools.dir}/renderscript/clang-include" />
- </path>
<!-- Intermediate files -->
<property name="dex.file.name" value="classes.dex" />
@@ -254,14 +215,6 @@
<!-- Build property file -->
<property name="out.build.prop.file"
location="${out.absolute.dir}/build.prop" />
-
- <!-- This is needed by emma as it uses multilevel verbosity instead of
simple 'true' or 'false'
- The property 'verbosity' is not user configurable and depends
exclusively on 'verbose'
- value.-->
- <condition property="verbosity" value="verbose" else="quiet">
- <istrue value="${verbose}" />
- </condition>
-
<!-- properties for signing in release mode -->
<condition property="has.keystore">
<and>
@@ -407,7 +360,7 @@
</macrodef>
<!-- This is macro which zipaligns in.package and outputs it to
out.package. Used by targets
- debug, -debug-with-emma and release.-->
+ debug and release.-->
<macrodef name="zipalign-helper">
<attribute name="in.package" />
<attribute name="out.package" />
@@ -420,26 +373,6 @@
</sequential>
</macrodef>
- <macrodef name="run-tests-helper">
- <attribute name="emma.enabled" default="false" />
- <element name="extra-instrument-args" optional="yes" />
- <sequential>
- <echo level="info">Running tests ...</echo>
- <exec executable="${adb}" failonerror="true">
- <arg line="${adb.device.arg}" />
- <arg value="shell" />
- <arg value="am" />
- <arg value="instrument" />
- <arg value="-w" />
- <arg value="-e" />
- <arg value="coverage" />
- <arg value="@{emma.enabled}" />
- <extra-instrument-args />
- <arg value="${project.app.package}/${test.runner}" />
- </exec>
- </sequential>
- </macrodef>
-
<macrodef name="record-build-key">
<attribute name="key" default="false" />
<attribute name="value" default="false" />
@@ -649,70 +582,16 @@
targetApi="${project.target.apilevel}"
verbose="${verbose}" />
- <!-- compile the libraries if any -->
- <if>
- <condition>
- <and>
- <isreference refid="project.library.folder.path" />
- <not><isset property="dont.do.deps" /></not>
- </and>
- </condition>
- <then>
- <!-- figure out which target must be used to build the library
projects.
- If emma is enabled, then use 'instrument' otherwise, use
'debug' -->
- <condition property="project.libraries.target"
value="instrument" else="${build.target}">
- <istrue value="${build.is.instrumented}" />
- </condition>
-
- <echo level="info">----------</echo>
- <echo level="info">Building Libraries with
'${project.libraries.target}'...</echo>
-
- <!-- no need to build the deps as we have already
- the full list of libraries -->
- <subant failonerror="true"
- buildpathref="project.library.folder.path"
- antfile="build.xml">
- <target name="nodeps" />
- <target name="${project.libraries.target}" />
- <property name="emma.coverage.absolute.file"
location="${out.absolute.dir}/coverage.em" />
- </subant>
- </then>
- </if>
-
- <!-- compile the main project if this is a test project -->
- <if condition="${project.is.test}">
- <then>
- <!-- figure out which target must be used to build the tested
project.
- If emma is enabled, then use 'instrument' otherwise, use
'debug' -->
- <condition property="tested.project.target" value="instrument"
else="debug">
- <isset property="emma.enabled" />
- </condition>
-
- <echo level="info">----------</echo>
- <echo level="info">Building tested project at
${tested.project.absolute.dir} with '${tested.project.target}'...</echo>
- <subant target="${tested.project.target}" failonerror="true">
- <fileset dir="${tested.project.absolute.dir}"
includes="build.xml" />
- </subant>
-
- <!-- get the tested project full classpath to be able to build
- the test project -->
- <testedprojectclasspath
- projectLocation="${tested.project.absolute.dir}"
- projectClassPathOut="tested.project.classpath"/>
- </then>
- <else>
- <!-- no tested project, make an empty Path object so that javac
doesn't
- complain -->
- <path id="tested.project.classpath" />
- </else>
- </if>
+ <!-- no tested project, make an empty Path object so that javac doesn't
+ complain -->
+ <path id="tested.project.classpath" />
</target>
<!-- empty default pre-build target. Create a similar target in
your build.xml and it'll be called instead of this one. -->
<ta…
(message too large)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12701012/3
Sorry for I got bad news for ya. Compile failed with a clobber build on android_dbg. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12701012/24002
Message was sent while issue was closed.
Change committed as 187735 |
