Bug 59297 - patch for inclusive Unix support on Chromium
Summary: patch for inclusive Unix support on Chromium
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Other
: P2 Normal
Assignee: Nobody
URL: http://chromium.hybridsource.org
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-24 00:06 PDT by chromium
Modified: 2011-04-26 10:09 PDT (History)
2 users (show)

See Also:


Attachments
Unix ifdefs (20.90 KB, patch)
2011-04-24 00:19 PDT, chromium
jamesr: review-
Details | Formatted Diff | Diff
updated Changelog (21.02 KB, patch)
2011-04-25 17:51 PDT, chromium
tony: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chromium 2011-04-24 00:06:27 PDT
I recently built Chromium 10 on OpenBSD and Solaris, I'm submitting the ifdef changes that came out of that work.  Most of these diffs simply change 

OS(LINUX) || OS(FREEBSD) 

to 

OS(UNIX) && !OS(DARWIN)

rather than stringing along something like 

OS(LINUX) || OS(FREEBSD) || OS(OPENBSD) || OS(SOLARIS) || OS(NETBSD) || OS(DRAGONFLY) 

every time support is added for a new Unix.  This patch from WebKit trunk was tested on FreeBSD, an earlier variant was tested with Chromium 10 on OpenBSD and Solaris.  All ifdef changes are only on files either built for Chromium or that are safe-guarded with a PLATFORM(CHROMIUM), so this patch shouldn't affect non-Chromium platforms.  Some of the Skia files aren't built on Darwin so the check for Darwin is left off there.  I added OpenBSD and Solaris to the Inspector bindings too.
Comment 1 chromium 2011-04-24 00:19:47 PDT
Created attachment 90876 [details]
Unix ifdefs
Comment 2 Tony Chang 2011-04-25 10:00:30 PDT
Resetting r? flag since you're asking for review.
Comment 3 James Robinson 2011-04-25 10:04:08 PDT
Comment on attachment 90876 [details]
Unix ifdefs

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

> WebKit/ChangeLog:7
> +        patch for inclusive Unix support on Chromium
> +        https://bugs.webkit.org/show_bug.cgi?id=59297
> +

This should go in WebKit/chromium/ChangeLog.  The prepare-ChangeLog and webkit-patch scripts will place changelogs in the proper locations.

"inclusive Unix support" is pretty vague - can you state what you actually mean?
Comment 4 chromium 2011-04-25 17:51:58 PDT
Created attachment 91035 [details]
updated Changelog

Sorry about setting the wrong review flag, I didn't read the instructions properly.  I didn't see the ChangeLog in WebKit/chromium, fixed that and updated the message with more detail.  What directory is one supposed to run prepare-ChangeLog from in a Chromium checkout of WebKit?  I tried running from third_party/WebKit/Source but it wouldn't work, which is why I found the ChangeLog directories myself and ran it from each one separately.  As for what I meant by inclusive support, I described it in the first comment here and have updated the comment in the ChangeLog.  Let me know if it isn't clear.
Comment 5 Tony Chang 2011-04-26 10:00:58 PDT
Comment on attachment 91035 [details]
updated Changelog

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

> WebCore/ChangeLog:9
> +        No new tests. (OOPS!)

Nit: This should be replaced with a line that says why there are no new tests (e.g., No new tests since this is just a compile change.).  I will fix this when I check it in.
Comment 6 Tony Chang 2011-04-26 10:09:51 PDT
http://trac.webkit.org/changeset/84921