- Set the bundle name based on the executable name (since they both are the b...

- Set the bundle name based on the executable name (since they both are the branding).
- Stop using CPP on the info.plist since we have to run a script anyways.
- Set the bundleid into a build setting for simple info.plist processing to pick up.
- Add a script for the chrome/app target to manually tweak the info.plist:
  - collect the version number from the build tree
  - collect the svn revision from the build tree
  - update the info.plist keys that need the version number based on collected data.
  - record the svn revision into the info.plist for trouble shooting.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=15778

[Mark Mentovai, 2009-05-11 18:46:58]

http://codereview.chromium.org/115187/diff/1/2
File build/mac/tweak_app_infoplist (right):

http://codereview.chromium.org/115187/diff/1/2#newcode38
Line 38: SVN_REVISION=$(svnversion "${SRCROOT}" | sed -E -e
's/^([0-9]*)(:*)([0-9MS]*)/\1/')
This pattern could use some love.

sed -En -e 's/^([0-9]+)(:[0-9MS]+)?$/\1/p'

Actually, now that we're just using the svn version as "informational," I don't
think we should process it at all.  Just take the output of svnversion.  If it's
a build where the WC is composed of multiple revisions or has modifications,
then there's nothing wrong with carrying that into the SVNRevision.

http://codereview.chromium.org/115187/diff/1/2#newcode54
Line 54: LONG_COPYRIGHT="Chromium ${FULL_VERSION}, Copyright 2009 The Chromium
Authors."
2009 should be in a variable.

http://codereview.chromium.org/115187/diff/1/2#newcode69
Line 69: # Save of the svn version number incase we need it
Save off

in case

http://codereview.chromium.org/115187/diff/1/2#newcode70
Line 70: defaults write "${TMP_INFO_PLIST_DEFAULTS}" SVNRevision
"${SVN_REVISION}"
If we're eliminating the svnversion post-processing, I'd add -string to this to
ensure that it's not sometimes an integer and sometimes a string.

http://codereview.chromium.org/115187/diff/1/2#newcode75
Line 75: # Honor the 429496.99.99 limit
Since this confused you earlier today, maybe you should lift a bit of the text
from my e-mail explaining what this limit is all about.

[TVL, 2009-05-11 19:02:13]

cl updated

[Mark Mentovai, 2009-05-11 19:06:44]

LGTM, I like.

http://codereview.chromium.org/115187/diff/1004/8
File build/mac/tweak_app_infoplist (right):

http://codereview.chromium.org/115187/diff/1004/8#newcode18
Line 18: #    this means if we were to generated a .h file for
INFOPLIST_PREFIX_HEADER
if we were to generate

http://codereview.chromium.org/115187/diff/1004/8#newcode53
Line 53: LONG_COPYRIGHT="Chromium ${FULL_VERSION}, Copyright 2009 The Chromium
Authors."
You don't want to put the year in a variable?

http://codereview.chromium.org/115187/diff/1004/8#newcode55
Line 55: LONG_COPYRIGHT="Chrome ${FULL_VERSION}, Copyright 2009 Google Inc."
since it's used here too?

Also, for both, I'd do

LONG_COPYRIGHT="${BUILD_BRANDING} ${FULL_VERSION}, Copyright ${YEAR} custom
text"

http://codereview.chromium.org/115187/diff/1004/8#newcode72
Line 72: defaults write "${TMP_INFO_PLIST_DEFAULTS}" CFBundleGetInfoString
"${LONG_COPYRIGHT}"
While we're here, we might as well add -string to all of the "defaults write"s,
I guess.

[Mark Mentovai, 2009-05-11 19:09:52]

http://codereview.chromium.org/115187/diff/1004/8
File build/mac/tweak_app_infoplist (right):

http://codereview.chromium.org/115187/diff/1004/8#newcode75
Line 75: # 6, 2, 2 digits. ÊThe limitation was present in Tiger, but it could
have been
Also, I see a funny character on this line.

[Mark Mentovai, 2009-05-11 20:12:23]

LGTM

Maybe file a bug to put YEAR into chrome/VERSION?

Maybe wrap some of those longish lines with \s?