Unify the version string to be displayed on "About Chromium" dialog. (6/6)

Unify the version string to be displayed on "About Chromium" dialog. (6/6)

- Update tweak_info_plist to use lastchange.py.

FYI: We decided to split the changes of issue 7104106 (http://codereview.chromium.org/7104106/) into the following steps. This is the sixth step of them:

(1) Add CreateVersionString() to chrome_version_info.*, update the GTK+ code to use it. (reviewer: erg, tony)
(2) Update the mac code to use CreateVersionString(). Update About.xib. (reviewer: mark)
(3) Update the views code to use CreateVersionString(). This can happen at the same time as (2). (reviewer: davemoore, tony)
(4) Update version.bat to use lastchange.py. (reviewer: evan)
(5) Make changes to lastchange.py and webkit_version.py. (reviewer: evan, mark)
(6) Update tweak_info_plist to use lastchange.py. (reviewer: mark)

BUG=37186
TEST=Observe that "About Chromium" dialog shows the version string like "14.0.787.0 (Developer Build 88242 Mac OS X" on Mac.


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

[haraken1, 2011-07-27 14:12:01]

mark: would you please take a look?
tony: FYI

[Mark Mentovai, 2011-07-27 19:12:03]

I don’t think you can actually make this change until lastchange.py is revised.

http://codereview.chromium.org/7514042/diff/1/chrome/tools/build/mac/tweak_in...
File chrome/tools/build/mac/tweak_info_plist (right):

http://codereview.chromium.org/7514042/diff/1/chrome/tools/build/mac/tweak_in...
chrome/tools/build/mac/tweak_info_plist:34: sys.path.insert(0, os.path.join(TOP,
"build/util/"))
Remove the trailing slash.

This bothers me because it was something I commented on earlier in the review
history of http://codereview.chromium.org/7104106/, and something you fixed
during the review process, in patch set 15. I would like to just give the LG to
this on the basis of having reviewed it in the past, but I can’t do that without
assurance that the version of the file you’re proposing here is identical to the
version that I gave the LG to on the original change, after all modifications
have been made.

It also calls into question the other changes you’ve made in this series, and
those you’re planning to make. I would hate to think that you’re basing them on
anything other than the final patch set 18 from the earlier series.

[haraken1, 2011-07-28 01:02:51]

>
http://codereview.chromium.org/7514042/diff/1/chrome/tools/build/mac/tweak_in...
> File chrome/tools/build/mac/tweak_info_plist (right):
> 
>
http://codereview.chromium.org/7514042/diff/1/chrome/tools/build/mac/tweak_in...
> chrome/tools/build/mac/tweak_info_plist:34: sys.path.insert(0,
os.path.join(TOP,
> "build/util/"))
> Remove the trailing slash.

Done. 

> It also calls into question the other changes you’ve made in this series, and
> those you’re planning to make. I would hate to think that you’re basing them
on
> anything other than the final patch set 18 from the earlier series.

Very sorry for the confusion. Actually, by my mistake on git local branches, I
found that some changes that were made at the patch set 18
(http://codereview.chromium.org/7104106/) may not be reflected to this series of
change. However, I checked them one by one and concluded that there were no
problems in the earlier series. In summary, the only change that I need to make
now is the change that I made in this patch (i.e. "build/util/" -> "build/util"
change). 


> I don’t think you can actually make this change until lastchange.py is
revised.

I agree. I would like to come back after the change to lastchange.py is landed.

[Mark Mentovai, 2011-07-28 01:44:46]

OK, good, it’s unanimous. Put this one on ice for the time being and request
review again once lastchange.py is checked in.

[haraken1, 2011-07-31 03:44:58]

The change of lastchange.py (http://codereview.chromium.org/7493073/) is landed
successfully. Is it OK to commit this patch now?

[Mark Mentovai, 2011-07-31 04:10:32]

LGTM to land now. Please watch very closely for regressions.

[commit-bot: I haz the power, 2011-07-31 08:51:22]

Change committed as 94855

[Mark Mentovai, 2011-07-31 13:42:05]

In a build with this change in effect, I now see the .app’s
Contents/Info.plist reflecting

        <key>SVNPath</key>
        <string>/trunk/src/chrome</string>
        <key>SVNRevision</key>
        <string>94855</string>

94855 is correct, but SVNPath should be /trunk/src, not /trunk/src/chrome.

[haraken1, 2011-07-31 14:52:14]

I checkout the latest revision (r94855), removed all the files in
src/xcodebuild/Debug, and then built it on my Mac environment. Then,
src/xcodebuild/Debug/Chromium.app/Contents/Info.plist says like this:

        <key>SVNPath</key>
        <string>/trunk/src</string>
        <key>SVNRevision</key>
        <string>94855</string>

...seems correct.

What kind of build are you looking at? Release build?

Sorry for the confusion.


On 2011/07/31 13:42:05, Mark Mentovai wrote:
> In a build with this change in effect, I now see the .app’s
> Contents/Info.plist reflecting
> 
>         <key>SVNPath</key>
>         <string>/trunk/src/chrome</string>
>         <key>SVNRevision</key>
>         <string>94855</string>
> 
> 94855 is correct, but SVNPath should be /trunk/src, not /trunk/src/chrome.

[Mark Mentovai, 2011-08-01 02:45:37]

http://codereview.chromium.org/7514042/diff/2002/chrome/tools/build/mac/tweak...
File chrome/tools/build/mac/tweak_info_plist (left):

http://codereview.chromium.org/7514042/diff/2002/chrome/tools/build/mac/tweak...
chrome/tools/build/mac/tweak_info_plist:108: (stdout, retval) =
_GetOutputNoError(['svn', 'info', TOP])
Note this. The “svn info” used to happen in directory TOP.

http://codereview.chromium.org/7514042/diff/2002/chrome/tools/build/mac/tweak...
File chrome/tools/build/mac/tweak_info_plist (right):

http://codereview.chromium.org/7514042/diff/2002/chrome/tools/build/mac/tweak...
chrome/tools/build/mac/tweak_info_plist:109: version_info =
lastchange.FetchVersionInfo(default_lastchange=None)
To be equivalent, this should have |directory=TOP| included as an argument.