Tracking bug for making builds fail on unbound plist substitutions |
|||
Issue descriptionRight now, gen_plist.py silently drops plist keys whose values have substitutions which aren't defined in the build. So, a plist like this: <?xml version="1.0" encoding="UTF-8"?> <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> <plist version="1.0"> <dict> <key>Foo</key> <true/> <key>Bar</key> <string>This is an ${UNBOUND_SUBSTITUTION}</string> </dict> </plist> …will no longer have a "Bar" key after running through the build system. As a result, it's easy to forget an entry in "extra_substitutions" in a BUILD.gn file and not notice. I want to change this script to fail the build when it can't match a substitution.
,
Sep 26 2016
My hope was to make the default/only behavior safer. How would you feel about handling the iOS build by conditionally including template plists, so that it's explicit about which keys it intends to fill in?
,
Sep 26 2016
I understand that we want a safe default behaviour (and I agree it is a good goal). For iOS downstream build, it would probably be worthwhile to split the Info.plist used, but it will take some time to identify all configurations that need to be fixed. What about having a strict default with a flag to revert to the current behaviour? Then we file a separate bug to cleanup downstream iOS build, and once cleanup is complete, we can remove the old behaviour. That way, we can have the sane strict default on Mac without waiting for iOS to be fixed first. WDYT?
,
Sep 26 2016
,
Sep 26 2016
Works for me, CL coming shortly. I filed bug 650345 to track cleanup, but feel free to close it if it should live in a different place.
,
Sep 27 2016
That's perfect, thank you.
,
Oct 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8be2f0889cd2d57cfd9910279e4b2e697aba76f8 commit 8be2f0889cd2d57cfd9910279e4b2e697aba76f8 Author: sdy <sdy@chromium.org> Date: Fri Oct 07 17:17:01 2016 Remove LSMinimumSystemVersion from content/shell's Info.plists, hardcode 10.9 in the installer's Info.plist. - The ones in content/shell were being stripped out by gen_plist.py because nothing is bound to the MACOSX_DEPLOYMENT_TARGET substitution. - The main app already has this key hard-coded in its Info.plist. From talking to rsesek@, using the same pattern for the installer will be less confusing than inventing a new build variable, expecially since our actual minimum supported version is only temporarily out of sync with the "real" build-time deployment target. BUG= 649854 Review-Url: https://codereview.chromium.org/2364203002 Cr-Commit-Position: refs/heads/master@{#423894} [modify] https://crrev.com/8be2f0889cd2d57cfd9910279e4b2e697aba76f8/chrome/installer/mac/app/BUILD.gn [modify] https://crrev.com/8be2f0889cd2d57cfd9910279e4b2e697aba76f8/chrome/installer/mac/app/Info.plist [modify] https://crrev.com/8be2f0889cd2d57cfd9910279e4b2e697aba76f8/content/shell/app/app-Info.plist [modify] https://crrev.com/8be2f0889cd2d57cfd9910279e4b2e697aba76f8/content/shell/app/helper-Info.plist
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/faba7ddfc2b78b1741e578817d7cbbaed0030a93 commit faba7ddfc2b78b1741e578817d7cbbaed0030a93 Author: sdy <sdy@chromium.org> Date: Wed Jul 05 21:04:22 2017 Fail to build with unbound Info.plist substitutions instead of silently dropping them. - In plist_util.py, fail if any plist substitutions aren't bound to values. - Remove a few keys in BuildInfo.plist which had no substitutions. They're added later by tweak_info_plist.py. BUG= 649854 , 650345 Review-Url: https://codereview.chromium.org/2367923002 Cr-Commit-Position: refs/heads/master@{#484348} [modify] https://crrev.com/faba7ddfc2b78b1741e578817d7cbbaed0030a93/build/config/mac/BuildInfo.plist [modify] https://crrev.com/faba7ddfc2b78b1741e578817d7cbbaed0030a93/build/config/mac/plist_util.py [modify] https://crrev.com/faba7ddfc2b78b1741e578817d7cbbaed0030a93/ui/base/BUILD.gn [modify] https://crrev.com/faba7ddfc2b78b1741e578817d7cbbaed0030a93/ui/base/test/framework-Info.plist
,
Jul 6 2017
This is fixed, right?
,
Jul 6 2017
👍
,
Jul 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2d33e2ab9e70b899a75b91cae1f2915841e87924 commit 2d33e2ab9e70b899a75b91cae1f2915841e87924 Author: Sylvain Defresne <sdefresne@chromium.org> Date: Mon Jul 10 09:46:52 2017 Fix compilation of ios_clean_skeleton_perf_egtests. Add a substitution for ENCRYPTION_EXPORT_COMPLIANCE_CODE if ios_encryption_export_compliance_code is non-empty to fix compilation. Bug: 649854 Change-Id: I55ab7e3467b89bc2970ab5a7fc11fc5baed09518 Reviewed-on: https://chromium-review.googlesource.com/563666 Reviewed-by: Eric Noyau <noyau@chromium.org> Commit-Queue: Eric Noyau <noyau@chromium.org> Cr-Commit-Position: refs/heads/master@{#485217} [modify] https://crrev.com/2d33e2ab9e70b899a75b91cae1f2915841e87924/ios/clean/chrome/test/perf/BUILD.gn |
|||
►
Sign in to add a comment |
|||
Comment 1 by sdefresne@chromium.org
, Sep 26 2016