A utility script to manipulate and extract resources from Windows programs.

A utility script to manipulate and extract resources from Windows programs.

This might be considered a first step in making our release signing process a little less opaque and brittle.

R=bradnelson@chromium.org,robertshield@chromium.org,grt@chromium.org
BUG=36234
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112707

[robertshield, 2011-12-02 00:35:42]

Awesome, lgtm w/ a couple of microscopic nits

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit.py
File chrome/tools/build/win/resedit.py (right):

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:26: # one to remove resources due to
overzeolus parameter verification.
overzealous

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:27: # For that case we're foced to go straight
to the native API implementation.
forced

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:88: input_file, None,
win32con.LOAD_LIBRARY_AS_DATAFILE)
does this raise an exception with a readable error message on failure?

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:277: help='On success, OUTPUT_FILE will
written with a copy of the '
will be written

[grt (UTC plus 2), 2011-12-02 02:38:11]

pretty damn cool, amigo.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit.py
File chrome/tools/build/win/resedit.py (right):

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:25: # The win32api-supplied UpdateResource
wrapper unfortunately does not allow
is there an appropriate place to file a bug for this?

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:101: file_copy = os.path.join(self.temp_dir,
how about keeping this value in the instance so you don't have to recompute it
in Commit?

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:161:
f.write(win32api.LoadResource(self.module, res_type, res_name))
This appears to be using FindResource rather than FindResourceEx, so your guess
is as good as mine as to how multiple languages will be handled.  If win32api
doesn't provide a variant that takes a langid, I suggest you roll your own.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:175:
f.write(win32api.LoadResource(self.module, res_type, res_name))
See previous comment.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:242: # Extract from mini_installer.exe, the
resource type "B7", langid 1033 and
Either remove references to langid and 1033, or fix ExtractResource to take a
langid.

[Sigurður Ásgeirsson, 2011-12-02 13:57:44]

Thanks! PTAL

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit.py
File chrome/tools/build/win/resedit.py (right):

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:25: # The win32api-supplied UpdateResource
wrapper unfortunately does not allow
On 2011/12/02 02:38:11, grt wrote:
> is there an appropriate place to file a bug for this?

I don't know. This'd be in the win32api module implementation.  I don't know who
maintains it.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:26: # one to remove resources due to
overzeolus parameter verification.
On 2011/12/02 00:35:42, robertshield wrote:
> overzealous

Done.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:27: # For that case we're foced to go straight
to the native API implementation.
On 2011/12/02 00:35:42, robertshield wrote:
> forced

Done.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:88: input_file, None,
win32con.LOAD_LIBRARY_AS_DATAFILE)
On 2011/12/02 00:35:42, robertshield wrote:
> does this raise an exception with a readable error message on failure?

Yups, you get the API call, the error code and the stack trace to output. That's
how all of win32api works.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:101: file_copy = os.path.join(self.temp_dir,
On 2011/12/02 02:38:11, grt wrote:
> how about keeping this value in the instance so you don't have to recompute it
> in Commit? 

Done.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:161:
f.write(win32api.LoadResource(self.module, res_type, res_name))
On 2011/12/02 02:38:11, grt wrote:
> This appears to be using FindResource rather than FindResourceEx, so your
guess
> is as good as mine as to how multiple languages will be handled.  If win32api
> doesn't provide a variant that takes a langid, I suggest you roll your own.

It does take a langid as an optional param, my oversight. Good catch!

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:175:
f.write(win32api.LoadResource(self.module, res_type, res_name))
On 2011/12/02 02:38:11, grt wrote:
> See previous comment.

Done.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:242: # Extract from mini_installer.exe, the
resource type "B7", langid 1033 and
On 2011/12/02 02:38:11, grt wrote:
> Either remove references to langid and 1033, or fix ExtractResource to take a
> langid.

Done.

http://codereview.chromium.org/8775026/diff/1/chrome/tools/build/win/resedit....
chrome/tools/build/win/resedit.py:277: help='On success, OUTPUT_FILE will
written with a copy of the '
On 2011/12/02 00:35:42, robertshield wrote:
> will be written

Done.

[grt (UTC plus 2), 2011-12-02 14:18:04]

LGTM with one tweak.

http://codereview.chromium.org/8775026/diff/5001/chrome/tools/build/win/resed...
File chrome/tools/build/win/resedit.py (right):

http://codereview.chromium.org/8775026/diff/5001/chrome/tools/build/win/resed...
chrome/tools/build/win/resedit.py:272: metavar='type langid name')
metavar='type langid name file_path'?

[Sigurður Ásgeirsson, 2011-12-02 14:31:30]

Thanks, committing.

http://codereview.chromium.org/8775026/diff/5001/chrome/tools/build/win/resed...
File chrome/tools/build/win/resedit.py (right):

http://codereview.chromium.org/8775026/diff/5001/chrome/tools/build/win/resed...
chrome/tools/build/win/resedit.py:272: metavar='type langid name')
On 2011/12/02 14:18:04, grt wrote:
> metavar='type langid name file_path'?

Done.

[commit-bot: I haz the power, 2011-12-02 14:31:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siggi@chromium.org/8775026/7002

[commit-bot: I haz the power, 2011-12-02 15:52:20]

Change committed as 112707