Reland r57812 (Add the new Mac manifest to the application bundle) and remove dummy translations

Reland r57812 (Add the new Mac manifest to the application bundle) and remove dummy translations

Add the policy templates generated from chrome/app/policy_templates.json to the Mac application bundle. (The template generator for Mac is implemented in CL 3116027) Also remove the old manifest file and references to translations that are not there yet.

The problem with the previous landing attempt was that in Release mode build, the following script was looking for binary files that were not there: build/mac/strip_from_xcode

BUG=49316
TEST=add Chromium to the managed applications' list in the OSX Workgroup Manager, and see if it shows the manageable preferences with English descriptions

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

[gfeher, 2010-08-30 13:40:32]

Hi, I came up with a solution to the Release mode build failure of
http://codereview.chromium.org/3143036/show.
Please one of you review it!

This was the quickest (and not the nicest) solution. I might also be able to fix
the problem in a couple of hours by overriding specific parts of
'target_defaults' in my target.

The removal of translations is temporary, until we really need and have them.

[TVL, 2010-08-30 14:45:46]

http://codereview.chromium.org/3287003/diff/1/3
File chrome/chrome_exe.gypi (right):

http://codereview.chromium.org/3287003/diff/1/3#newcode506
chrome/chrome_exe.gypi:506: 'mac_real_dsym': 1,
i'll defer to mark on if doing this is ok.

[gfeher, 2010-09-01 20:13:51]

Mark?

On 2010/08/30 14:45:46, TVL wrote:
> http://codereview.chromium.org/3287003/diff/1/3
> File chrome/chrome_exe.gypi (right):
> 
> http://codereview.chromium.org/3287003/diff/1/3#newcode506
> chrome/chrome_exe.gypi:506: 'mac_real_dsym': 1,
> i'll defer to mark on if doing this is ok.

[Mark Mentovai, 2010-09-01 20:31:56]

http://codereview.chromium.org/3287003/diff/1/3
File chrome/chrome_exe.gypi (right):

http://codereview.chromium.org/3287003/diff/1/3#newcode506
chrome/chrome_exe.gypi:506: 'mac_real_dsym': 1,
Instead of doing this, I’d prefer to add another configuration variable like
mac_real_dsym. In build/common.gypi, we should have (next to 'mac_real_dsym%')
'mac_strip%': 1. Then, the condition for '_type=="executable" or
_type=="shared_library" or _type=="loadable_module"' should be extended to only
be effective if mac_strip!=0. Finally, you should change this line here to
'mac_strip': 0. The comment should be adjusted to be more generic. We don’t want
any of that strip stuff to be added to the target. This has nothing to do with
mac_real_dsym. mac_real_dsym only controls who’s supposed to do the stripping.
(I understand that it works for your purposes, but it’s hackish and a better
solution is available.)

[gfeher, 2010-09-02 13:58:24]

http://codereview.chromium.org/3287003/diff/1/3
File chrome/chrome_exe.gypi (right):

http://codereview.chromium.org/3287003/diff/1/3#newcode506
chrome/chrome_exe.gypi:506: 'mac_real_dsym': 1,
I also didn't like it. Thanks for the help!

On 2010/09/01 20:31:56, Mark Mentovai wrote:
> Instead of doing this, I’d prefer to add another configuration variable like
> mac_real_dsym. In build/common.gypi, we should have (next to 'mac_real_dsym%')
> 'mac_strip%': 1. Then, the condition for '_type=="executable" or
> _type=="shared_library" or _type=="loadable_module"' should be extended to
only
> be effective if mac_strip!=0. Finally, you should change this line here to
> 'mac_strip': 0. The comment should be adjusted to be more generic. We don’t
want
> any of that strip stuff to be added to the target. This has nothing to do with
> mac_real_dsym. mac_real_dsym only controls who’s supposed to do the stripping.
> (I understand that it works for your purposes, but it’s hackish and a better
> solution is available.)

[Mark Mentovai, 2010-09-02 14:09:21]

LGTM on the strip part.

You’re deleting app-Manifest.plist and modifying policy_templates.grd, which you
weren’t doing in the patch set 1. Why did this change? Is that related? I was
only expecting to see a difference in how you avoided the strip. I haven’t
reviewed the extra bits you’re deleting relative to patch set 1.

http://codereview.chromium.org/3287003/diff/21001/22001
File build/common.gypi (right):

http://codereview.chromium.org/3287003/diff/21001/22001#newcode1210
build/common.gypi:1210: _type=="loadable_module") and (mac_strip != 0)', {
Remove the () around mac_strip.

There’s no space around the existing == operators. You should put spaces around
them to be consistent with the spaces you’re putting around the != for
mac_strip.

http://codereview.chromium.org/3287003/diff/21001/22001#newcode1255
build/common.gypi:1255: #  _type=="loadable_module") and (mac_strip != 0)
Adjust this style-wise to match whatever you do above.

[gfeher, 2010-09-02 14:48:53]

Sorry about the new changes. I realized that they are closely related to this
CL.
-I actually should have removed app-Manifest.plist in the previous CL. (The CL
that I couldn't land.) app-Manifest.plist was only used by copy_mcx_manifest.sh
to be copied into Chromium.app. Now it is replaced by the bundle assembled here.
-The changes in policy_templates.grd sine Patch Set 1 are only because of a
rebase. (The only new thing in that file in this CL is removing the non-English
translations. That does not strictly belong here, but it came along with a
modification to 'app_manifest_bundle' in 'chrome_exe.gypi': introducing and
using the variable 'available_locales')

Whould you like me to ask someone else to review these changes?

http://codereview.chromium.org/3287003/diff/21001/22001
File build/common.gypi (right):

http://codereview.chromium.org/3287003/diff/21001/22001#newcode1210
build/common.gypi:1210: _type=="loadable_module") and (mac_strip != 0)', {
On 2010/09/02 14:09:21, Mark Mentovai wrote:
> Remove the () around mac_strip.
> 
> There’s no space around the existing == operators. You should put spaces
around
> them to be consistent with the spaces you’re putting around the != for
> mac_strip.

It looks to me that having no spaces is the more common in this file. So I'd
rather remove the spaces around the != operator.

http://codereview.chromium.org/3287003/diff/21001/22001#newcode1255
build/common.gypi:1255: #  _type=="loadable_module") and (mac_strip != 0)
On 2010/09/02 14:09:21, Mark Mentovai wrote:
> Adjust this style-wise to match whatever you do above.

Done.

[Mark Mentovai, 2010-09-02 15:03:00]

LGTM. Thanks for the explanation of the other changes.