Bug 120870 - GTest: add support for FreeBSD and Hurd
Summary: GTest: add support for FreeBSD and Hurd
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-06 11:33 PDT by Alberto Garcia
Modified: 2013-09-09 02:48 PDT (History)
4 users (show)

See Also:

Attachments
Patch (3.22 KB, patch)
2013-09-06 11:34 PDT, Alberto Garcia 		gustavo: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Garcia 2013-09-06 11:33:22 PDT 
This is necessary for the FreeBSD and Hurd builds.
Comment 1 Alberto Garcia 2013-09-06 11:34:38 PDT 
Created attachment 210775 [details]
Patch
Comment 2 WebKit Commit Bot 2013-09-06 11:37:18 PDT 
Attachment 210775 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/ThirdParty/ChangeLog', u'Source/ThirdParty/gtest/include/gtest/internal/gtest-port.h']" exit_code: 1
Source/ThirdParty/gtest/include/gtest/internal/gtest-port.h:499:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alberto Garcia 2013-09-06 11:39:43 PDT 
(In reply to comment #2)
> Attachment 210775 [details] did not pass style-queue:

That's the second line here:

-#if (GTEST_OS_LINUX || GTEST_OS_MAC || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || \
+#if (GTEST_OS_LINUX || GTEST_OS_FREEBSD || GTEST_OS_HURD || \
+     GTEST_OS_MAC || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || \
      (GTEST_OS_WINDOWS_DESKTOP && _MSC_VER >= 1400) || \
      GTEST_OS_WINDOWS_MINGW || GTEST_OS_AIX)

I decided to keep the same spacing as the rest of the expression, I
guess that's what makes sense here?
Comment 4 Emilio Pozuelo Monfort 2013-09-06 11:41:38 PDT 
(In reply to comment #3)
> (In reply to comment #2)
> > Attachment 210775 [details] [details] did not pass style-queue:
> 
> That's the second line here:
> 
> -#if (GTEST_OS_LINUX || GTEST_OS_MAC || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || \
> +#if (GTEST_OS_LINUX || GTEST_OS_FREEBSD || GTEST_OS_HURD || \
> +     GTEST_OS_MAC || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || \
>       (GTEST_OS_WINDOWS_DESKTOP && _MSC_VER >= 1400) || \
>       GTEST_OS_WINDOWS_MINGW || GTEST_OS_AIX)
> 
> I decided to keep the same spacing as the rest of the expression, I
> guess that's what makes sense here?

gtest is an external project so we shouldn't follow webkit's style but be consistent with the rest of the file AFAIK.

Thanks for submitting this upstream!
Comment 5 Zan Dobersek 2013-09-06 11:42:36 PDT 
(In reply to comment #3)
> (In reply to comment #2)
> > Attachment 210775 [details] [details] did not pass style-queue:
> 
> That's the second line here:
> 
> -#if (GTEST_OS_LINUX || GTEST_OS_MAC || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || \
> +#if (GTEST_OS_LINUX || GTEST_OS_FREEBSD || GTEST_OS_HURD || \
> +     GTEST_OS_MAC || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || \
>       (GTEST_OS_WINDOWS_DESKTOP && _MSC_VER >= 1400) || \
>       GTEST_OS_WINDOWS_MINGW || GTEST_OS_AIX)
> 
> I decided to keep the same spacing as the rest of the expression, I
> guess that's what makes sense here?

This is a third-party source code, so we shouldn't really enforce WebKit style guidelines in there.
Comment 6 Zan Dobersek 2013-09-06 11:46:46 PDT 
(In reply to comment #4)
> gtest is an external project so we shouldn't follow webkit's style but be consistent with the rest of the file AFAIK.

Didn't see that, sorry for the extra noise.
Comment 7 Gustavo Noronha (kov) 2013-09-06 13:19:10 PDT 
Comment on attachment 210775 [details]
Patch

Should probably submit that to Google, so it's not lost when it's synced from there?
Comment 8 Alberto Garcia 2013-09-06 13:30:26 PDT 
Committed r155210: <http://trac.webkit.org/changeset/155210>
Comment 9 Alberto Garcia 2013-09-09 02:33:23 PDT 
(In reply to comment #7)
> (From update of attachment 210775 [details])
> Should probably submit that to Google, so it's not lost when it's synced from there?

I guess so, what do you think Emilio?
Comment 10 Emilio Pozuelo Monfort 2013-09-09 02:48:01 PDT 
Absolutely. Upstream is at https://code.google.com/p/googletest/source/browse/trunk/include/gtest/internal/gtest-port.h

I can have a look at it later this week, but feel free to beat me to it.