Bug 111504 - build a DumpRenderTree.pak even on windows
Summary: build a DumpRenderTree.pak even on windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks: 111508
  Show dependency treegraph
 
Reported: 2013-03-05 17:44 PST by Dirk Pranke
Modified: 2013-03-05 22:19 PST (History)
4 users (show)

See Also:


Attachments
Patch (6.56 KB, patch)
2013-03-05 17:48 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2013-03-05 17:44:43 PST
build a DumpRenderTree.pak even on windows
Comment 1 Dirk Pranke 2013-03-05 17:48:35 PST
Created attachment 191620 [details]
Patch
Comment 2 Dirk Pranke 2013-03-05 17:49:52 PST
Comment on attachment 191620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191620&action=review

> Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:-345
> -                    }],

I don't know why we were previously building the .pak file in <(INTERMEDIATE_DIR) and then copying it into <(PRODUCT_DIR). Is there a reason not to just build it into <(PRODUCT_DIR) by default?
Comment 3 Tony Chang 2013-03-05 18:12:53 PST
Comment on attachment 191620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191620&action=review

>> Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:-345
>> -                    }],
> 
> I don't know why we were previously building the .pak file in <(INTERMEDIATE_DIR) and then copying it into <(PRODUCT_DIR). Is there a reason not to just build it into <(PRODUCT_DIR) by default?

Not sure, but it seems fine to write the final result to <(PRODUCT_DIR).
Comment 4 Dirk Pranke 2013-03-05 18:39:14 PST
Comment on attachment 191620 [details]
Patch

Clearing flags on attachment: 191620

Committed r144863: <http://trac.webkit.org/changeset/144863>
Comment 5 Dirk Pranke 2013-03-05 18:39:16 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Dirk Pranke 2013-03-05 19:05:19 PST
hm, this appears to break xcode ... making DumpRenderTree_resources be a separate target and still having process_outputs_as_mac_bundle_resources set to true doesn't look like it works right (since DumpRenderTree_resources isn't actually a bundle).

Temporarily, I'm going to set that flag off, and we'll see what else breaks.
Comment 7 Dirk Pranke 2013-03-05 19:10:18 PST
Okay, I've attempted to patch this in bug 111506 / r144866 . It looks like that might work, but I have to wait for the xcode builders to cycle to be sure, and even then I'm not sure if this is the right fix.
Comment 8 Dirk Pranke 2013-03-05 19:10:55 PST
Er, that's bug 111509, not bug 111506.
Comment 9 Nico Weber 2013-03-05 22:15:35 PST
(In reply to comment #7)
> Okay, I've attempted to patch this in bug 111506 / r144866 . It looks like that might work, but I have to wait for the xcode builders to cycle to be sure, and even then I'm not sure if this is the right fix.

It fairly likely isn't. I commented on bug 111509.
Comment 10 Nico Weber 2013-03-05 22:19:21 PST
Comment on attachment 191620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191620&action=review

> Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:235
> +                'DumpRenderTree_resources',

What you can do to fix the problem I described in the other bug:

Add `'mac_bundle_resources': [ '<(PRODUCT_DIR)/DumpRenderTree.pak' ]` to this target, and remove process_outputs_as_mac_bundle_resources from your drt_resources target. (If this target here already has a mac_bundle_resources section, add the pak file to that.)

With that, everything should work.

>>> Tools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp:-345
>>> -                    }],
>> 
>> I don't know why we were previously building the .pak file in <(INTERMEDIATE_DIR) and then copying it into <(PRODUCT_DIR). Is there a reason not to just build it into <(PRODUCT_DIR) by default?
> 
> Not sure, but it seems fine to write the final result to <(PRODUCT_DIR).

Maybe repack.py doesn't write the output file atomically?