Bug 120870 – GTest: add support for FreeBSD and Hurd

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Alberto Garcia]

This is necessary for the FreeBSD and Hurd builds.

[Alberto Garcia]

Created attachment 210775 [details]
Patch

[WebKit Commit Bot]

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.

[Alberto Garcia]

(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?

[Emilio Pozuelo Monfort]

(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!

[Zan Dobersek]

(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.

[Zan Dobersek]

(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.

[Gustavo Noronha (kov)]

Comment on attachment 210775 [details]
Patch

Should probably submit that to Google, so it's not lost when it's synced from there?

[Alberto Garcia]

Committed r155210: <http://trac.webkit.org/changeset/155210>

[Alberto Garcia]

(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?

[Emilio Pozuelo Monfort]

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.