Bug 42911 - Update ruby tools to work with shallow framework bundles
Summary: Update ruby tools to work with shallow framework bundles
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
Depends on:
Reported: 2010-07-23 14:27 PDT by David Kilzer (:ddkilzer)
Modified: 2010-07-23 14:59 PDT (History)
1 user (show)

See Also:

Patch (4.03 KB, patch)
2010-07-23 14:27 PDT, David Kilzer (:ddkilzer)
mrowe: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2010-07-23 14:27:02 PDT
Created attachment 62460 [details]

Reviewed by NOBODY (OOPS!).

* Scripts/check-for-inappropriate-files-in-framework: Added
check for the SHALLOW_BUNDLE environment variable so that the
script will work with iOS WebKit builds.
* Scripts/check-for-webkit-framework-include-consistency: Ditto.
 3 files changed, 23 insertions(+), 8 deletions(-)
Comment 1 Mark Rowe (bdash) 2010-07-23 14:41:07 PDT
Comment on attachment 62460 [details]

A few minor comments, primarily stylistic

I think that:
has_shallow_bundle = (ENV['SHALLOW_BUNDLE'] || "NO").upcase == “YES”
would be clearer than:
has_shallow_bundle = !ENV['SHALLOW_BUNDLE'].nil? && ENV['SHALLOW_BUNDLE'].upcase == “YES”

You’re using has_shallow_bundle and is_shallow to mean the same thing.  I’d pick one name and stick with it.  I think “is_shallow_bundle” would be a good compromise.

 49   all_headers = is_shallow ? `find WebKit.framework/{,Private}Headers -type f -name '*.h'`.split \
 50                            : `find WebKit.framework/Versions/A/{,Private}Headers -type f -name '*.h'`.split

Duplicating the entire command here is unfortunate.  I’d suggest something like:

current_version_path = is_shallow_bundle ? “” : “Versions/A/”
all_headers = `find WebKit.framework/#{current_version_path}{,Private}Headers -type f -name '*.h'`.split

(Excuse the curly quotes :-/)

Other than that, r=me!
Comment 2 David Kilzer (:ddkilzer) 2010-07-23 14:59:58 PDT
Committed r63997: <http://trac.webkit.org/changeset/63997>