From f47bf0fecfb13443b88c1026f28727b5377a9577 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 21 Mar 2018 23:28:51 -0700 Subject: [PATCH] releasetools: Fix an issue in GetMinSdkVersion. The following is a buggy pattern that won't capture anything into err. The issue is benign, since a failed run would be eventually captured by a subsequent check. p = Run(["aapt", ...], stdout=subprocess.PIPE) output, err = p.communicate() if err: raise ... This CL changes the error detection to be based on the return code from aapt. It also adds some sanity test to ensure the call to aapt works. The test app is built from AOSP com.android.cts.ctsshim (chosen mostly because of its small size). Test: python -m unittest test_common Change-Id: I337f141bd0fc5f0801dfc628c601b88b7640789c --- tools/releasetools/common.py | 55 +++++++++++++++++------- tools/releasetools/test_common.py | 17 ++++++++ tools/releasetools/testdata/TestApp.apk | Bin 0 -> 4817 bytes 3 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 tools/releasetools/testdata/TestApp.apk diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index 787de98222..c048e0fc8a 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -718,18 +718,31 @@ def GetKeyPasswords(keylist): def GetMinSdkVersion(apk_name): - """Get the minSdkVersion delared in the APK. This can be both a decimal number - (API Level) or a codename. + """Gets the minSdkVersion declared in the APK. + + It calls 'aapt' to query the embedded minSdkVersion from the given APK file. + This can be both a decimal number (API Level) or a codename. + + Args: + apk_name: The APK filename. + + Returns: + The parsed SDK version string. + + Raises: + ExternalError: On failing to obtain the min SDK version. """ + proc = Run( + ["aapt", "dump", "badging", apk_name], stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + stdoutdata, stderrdata = proc.communicate() + if proc.returncode != 0: + raise ExternalError( + "Failed to obtain minSdkVersion: aapt return code {}:\n{}\n{}".format( + proc.returncode, stdoutdata, stderrdata)) - p = Run(["aapt", "dump", "badging", apk_name], stdout=subprocess.PIPE) - output, err = p.communicate() - if err: - raise ExternalError("Failed to obtain minSdkVersion: aapt return code %s" - % (p.returncode,)) - - for line in output.split("\n"): - # Looking for lines such as sdkVersion:'23' or sdkVersion:'M' + for line in stdoutdata.split("\n"): + # Looking for lines such as sdkVersion:'23' or sdkVersion:'M'. m = re.match(r'sdkVersion:\'([^\']*)\'', line) if m: return m.group(1) @@ -737,11 +750,20 @@ def GetMinSdkVersion(apk_name): def GetMinSdkVersionInt(apk_name, codename_to_api_level_map): - """Get the minSdkVersion declared in the APK as a number (API Level). If - minSdkVersion is set to a codename, it is translated to a number using the - provided map. - """ + """Returns the minSdkVersion declared in the APK as a number (API Level). + If minSdkVersion is set to a codename, it is translated to a number using the + provided map. + + Args: + apk_name: The APK filename. + + Returns: + The parsed SDK version number. + + Raises: + ExternalError: On failing to get the min SDK version number. + """ version = GetMinSdkVersion(apk_name) try: return int(version) @@ -750,8 +772,9 @@ def GetMinSdkVersionInt(apk_name, codename_to_api_level_map): if version in codename_to_api_level_map: return codename_to_api_level_map[version] else: - raise ExternalError("Unknown minSdkVersion: '%s'. Known codenames: %s" - % (version, codename_to_api_level_map)) + raise ExternalError( + "Unknown minSdkVersion: '{}'. Known codenames: {}".format( + version, codename_to_api_level_map)) def SignFile(input_name, output_name, key, password, min_api_level=None, diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index fb26b6660a..f211b03b25 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -504,6 +504,23 @@ class CommonApkUtilsTest(unittest.TestCase): actual = common.ParseCertificate(cert_fp.read()) self.assertEqual(expected, actual) + def test_GetMinSdkVersion(self): + test_app = os.path.join(self.testdata_dir, 'TestApp.apk') + self.assertEqual('24', common.GetMinSdkVersion(test_app)) + + def test_GetMinSdkVersion_invalidInput(self): + self.assertRaises( + common.ExternalError, common.GetMinSdkVersion, 'does-not-exist.apk') + + def test_GetMinSdkVersionInt(self): + test_app = os.path.join(self.testdata_dir, 'TestApp.apk') + self.assertEqual(24, common.GetMinSdkVersionInt(test_app, {})) + + def test_GetMinSdkVersionInt_invalidInput(self): + self.assertRaises( + common.ExternalError, common.GetMinSdkVersionInt, 'does-not-exist.apk', + {}) + class CommonUtilsTest(unittest.TestCase): diff --git a/tools/releasetools/testdata/TestApp.apk b/tools/releasetools/testdata/TestApp.apk new file mode 100644 index 0000000000000000000000000000000000000000..a91160368cb8249aba90770620b0595c283ec065 GIT binary patch literal 4817 zcmeHLXIxWV(oZ0eP!dW)6G1?VA|;`tAWfQxbfgGK7X%^!1?gQ71nIr^7Me;Ekg6aA zq$(}+A|OQt1c@8o_u18b_I=*{x*vA#Z_c@Q&YgSDnLG2pGe=v46oddkAP@imz6Nt`qQPi zv%Cj=zu0|JrDYlj2!-qc?;w`(Op8r1DLmuhq6@OSfrmGI`26Te8Ig+p^x!VO1=ij4 z02&Cr*|h{U_;z?q*R2Jn-N%O-+$R})LJ{Y>D#_L3|37D*Ik zaazs`L*E{aQf+uy=A-Lzy^?l2bTK|B zmo3_22P@qU!PwS}%Zm^8ujnYXkxt?R=B+#O@pqvouune0Dy=1|Z8Wk+zWVlUHD`|4 zh6!QR=Ug&7-^l*p`SexNIJnJn3;kgSwLXvT>%_Zf66_pzB=b(5GGDcm`7zMsBlRO= zqs8O!(S%3Wd^{Fix4e*v{_d}>0fs#Ms>JF*3IGI90RY;68LhO$xYbnFR}fUcsUobT ztgA1ir(*Wh6di;FgyPe`%6)aED*ed6Fm6MWi6l373@f!k517>CCXfMz+sw_TBhh@T zOFkBMlzdaz_eN`W_KvFWzsyktsps34Gx*677}f%%$3|6hH!*kGmd}Ge@^#uE=7tbn z6AO**c5W01W09=QL>?SIu59JqggdK2Iqn+Sdj1agO3jX{Ld7-q7KLYvtY_+$5f-~9 zI@dS??r>hxeD6$HrGX{3B)`k*UJE0?!{~`s0M9G{_Anm|9qGo~)|^qjYAUh(6t|)!NR?EOS(1k7 zoP0LIKXY44PI*n<96Ry`5^?-9g>`V}wrgXe+rbVj5u#4hGf;N1gK)e(AXTdK+4cCI zmmvgg{WfBz_=Cq)42?LWOq}e7H=#w+{zxP{+sHk)N>S7D$w_~n32TzSvZ$^7Y<3II zs-1WDsJr&P@lW$UQIv?V%Ma_J!x4E#=B8R70V@G6_E>7mm!B@XY=EYWk`vUh^D<-S z?+$s=b652bdk*EeT96uwQ?c3$I%Hf`(^G`&MlS*%DW`;v1*FFA%9#+X$qS#9eH-yI z2cLDdfhIKEzxU0nXAN6>;oEXvB+PmD7CqgG|HfHS`%wZ<6aJ(06AVHmn;qBO935(z zT~rQxe30AZOtmrIKf{-sKUa@3+R<2pCOTWzXHV;3Qc6rG_p7YS1Qwt8n@%W8^MKwE zkSH4i9n1#GyC9$8z^zaG;|960)=j^3{+7d>){26+GuC^$>hP@+G-E~Ab2}@ce7HMuBv=5`S}AvUYB`7((2;WkNaqSR@DJ} z{~5_AF}Jmr@1kkF$1|pB@2`iqH!nY~wUH4{c2%yxCOcjhZJg>H>M*tyrdkx{M3o2J zZ>6^A!*g3(9@5{nT`_;qd^QIW`q{p4Kum=g}6fPNPO9 zhS6+UZoZ+%L37Uvr?LKfEV`THrE053MBHyyQ%KA2RCjA9<(utc) zmPom?vZO}Tbw~Gy_wsvSty_4!?R3VGXiDLB#}>AQ?qK3Xvc*f6r+&4oDHO)vT`XL; zx-BDTtr=F^JXm$?_B=i$Ib`O#z^xeHww|J`aL1(7T~=i(dq2C*6haIfcnV_=Zi5TM zK>$D#`TqxFh~HsMQ{kq%in5-*kfw@Yza|wC)&u67tAAEov+?qO4;*U0of|@aJ+3&V z$x5s~<@|_6w&DbMR`Pt1>{^bF^4RMFNUpQV>u?61w1;cRO4^ih-&HPNF@vD{4UHFu zF_hM&v$}LG?xuGD=lNuzmfV+ljFV3WSL#<^Ux{Eg%~@^dRrFP_(bhBuh0m# zCQ129Kaeh3?Tw~92kzd|S3fE}*f;{8uFmoxkt>5Z_mWeP1```1A^+&lJf?8HKs@Ir z@*2?s0>rTkL?#6S035^~6ipTG)(~~9j=uQ?w#hottrP#QvAP1LC% zGBOBMf(#;kmP1McBq5;y#Jg0qg7<)UB$S+jKTIv`015<>kpYl_2`DNg1v#k!h#XF0 zpofAZ=}s>oa7rahCkHz;#>K%B#fU_l3c+xwCi;PkyQPB*hoOV52Z|O6JC#$y!GGkz zf0t1Gi-h!-gbT^>k2z2P8R3@yn?uY0(F<9lossNJP^1)6L_`{i5)~DbFkym<5+NNW z`N!pd>a>G^9KRbFNCqGc0z!!&0|+4r0s;Z!@hqU(9xCpdtv6P4A3yM1y*l8u!M5;| zJgQ{vRms4;5l@$5%3`M#STjwkxbe@AS3OFVR@Y=TY#Npbc_S-^y9@RutZlW(7oHs; zrPdo5<_48-oD5+*Qn@!HI$EXtqgw}K`Z_1s7m>Z6;)jYmr>}p}=7MTNz#?-2$xNRe z$wZA;iV`*j>h?l%3hdr`C49JoqPT1?DI{z||Jub_ao30$PaqBKryjjF=`_>Xz-`2$ z2CU<;forwRru1I$P@DQ@@2C$A)PLLD2X0(YH4vrix|eor)hG3jH z9F{v>X0RYY>qTV&h(?Fs%Ur*T%vNRrj+-@lbYBfVaKAL1TAJpPj}?3nMhH)21C|h8 zU`-V7=$Rdgp~$$oRH7sn1o+LrCycXAARU)__Y+z3+lAR!5HmgJ&R9(f9`fyz5(@8J z5zvRW_y98`nrhP&UE7)Px-wJUUjn_<=eCQeTm0Rb{JgYSWR<>hfiZS2H#CFiTTH?r zK3}v1H#rxS`AwL&)IG$A>c}Qtg?tdnzKR!|oC8fmO0s9r#hs zT}->a+Bdh!=xiS_57tII!Y|DM#GR$#xVi8!Bs)+W|te^&oJ iYdFn(eo-LBZ|cA0LE0K*#QcevGn^hof2#kVxBd+jo8qPb literal 0 HcmV?d00001